Fix an issue on type casting of default string function/procedure arguments in debugger tool. #9036 #9397
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,7 +49,24 @@ SELECT | |
| pg_catalog.pg_get_function_identity_arguments(p.oid) AS signature, | ||
|
|
||
| {% if hasFeatureFunctionDefaults %} | ||
| pg_catalog.array_to_string( | ||
| ARRAY( | ||
| SELECT | ||
| -- Removing type cast, then removing surrounding single quotes if present | ||
| CASE | ||
| WHEN regexp_replace(val, E'::[a-zA-Z_]+$', '') ~ E'^''.*''$' | ||
| THEN substring(regexp_replace(val, E'::[a-zA-Z_]+$', '') FROM E'^''(.*)''$') | ||
| ELSE regexp_replace(val, E'::[a-zA-Z_]+$', '') | ||
| END | ||
| FROM unnest( | ||
| regexp_split_to_array( | ||
| pg_catalog.pg_get_expr(p.proargdefaults, 'pg_catalog.pg_class'::regclass, false), | ||
| ',\s*' | ||
| ) | ||
|
Comment on lines
+61
to
+65
There was a problem hiding this comment. 🧩 Analysis chainCritical: The comma split doesn't respect string literal boundaries. The regex pattern For example, a function argument with default
This produces mangled output. A robust solution would need context-aware parsing that understands string literal boundaries, or an alternative approach (e.g., using Can you verify whether the original issue (#9036) anticipated comma-containing defaults? If this is a realistic scenario for your debugger users, this needs a fix before merging. I'll verify the concerns raised by examining the actual file and gathering context about the issue being fixed. Length of output: 2150 🏁 Script executed: #!/bin/bash
# Search for any related tests or documentation about this fix
fd -t f -e sql -e py | xargs grep -l "proargdefaults\|get_function_debug_info" | head -20Length of output: 2001 🏁 Script executed: #!/bin/bash
# Check git history for recent changes to this file
git log --oneline -20 "web/pgadmin/tools/debugger/templates/debugger/sql/get_function_debug_info.sql"Length of output: 195 🏁 Script executed: #!/bin/bash
# Look for issue #9036 references in the codebase
rg -n "9036|proargdefaults" --type=md --type=py --type=sql -C3 | head -50Length of output: 4581 Let me examine the Length of output: 1554 🏁 Script executed: #!/bin/bash
# Check git commit details for #9036
git show 9bf97110 --statLength of output: 370 🏁 Script executed: #!/bin/bash
# Test what PostgreSQL pg_get_expr actually returns
# Create a test to understand the format
python3 << 'EOF'
import re
# Test the regex patterns with realistic PostgreSQL function defaults
test_cases = [
"1", # Simple integer
"'hello'::text", # String with type cast
"'hello, world'::text", # String with comma inside (CRITICAL TEST)
"123::numeric", # Numeric with type cast
"'a'::numeric(10,2)", # Parameterized type (MAY FAIL)
"'{1,2,3}'::int[]", # Array type (MAY FAIL)
"'one'::text, 'two, three'::text", # Multiple with comma inside second
]
# The regex patterns from the SQL
type_cast_pattern = r'::[a-zA-Z_]+$'
quote_pattern_check = r"^'.*'$"
quote_extraction = r"^'(.*)'$"
split_pattern = r',\s*'
print("Testing SPLIT PATTERN: ', \\s*'")
print("-" * 60)
for case in test_cases:
parts = re.split(split_pattern, case)
print(f"Input: {case!r}")
print(f"Split result: {parts}")
print()
print("\nTesting TYPE CAST REMOVAL: '::[a-zA-Z_]+$'")
print("-" * 60)
for case in test_cases:
cleaned = re.sub(type_cast_pattern, '', case)
print(f"Input: {case!r} -> {cleaned!r}")
print("\nTesting REALISTIC SCENARIO - comma in string:")
print("-" * 60)
problematic = "'hello, world'::text"
parts = re.split(split_pattern, problematic)
print(f"Input: {problematic!r}")
print(f"Split into {len(parts)} parts: {parts}")
print(f"Expected: 1 part, got {len(parts)}")
if len(parts) > 1:
print("❌ ISSUE CONFIRMED: Comma inside string literal is split!")
EOFLength of output: 1216 🏁 Script executed: #!/bin/bash
# Check what pg_get_expr with false (last param) returns
# Look for PostgreSQL documentation or examples
rg -n "pg_get_expr.*proargdefaults" --type=sql -C5Length of output: 61951 Critical issue confirmed: The comma split doesn't respect string literal boundaries. Testing reveals that Major issue confirmed: The type cast regex is incomplete. The pattern
These type casts remain in output, defeating the fix's intent. Recommendation: Consider using a state-machine approach (like the existing 🤖 Prompt for AI Agents |
||
| ) AS val | ||
| ), | ||
| ',' | ||
| ) AS proargdefaults, | ||
| p.pronargdefaults | ||
| {%else%} | ||
| '' AS proargdefaults, 0 AS pronargdefaults | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Major: The type-cast removal regex is incomplete for complex PostgreSQL types.
The pattern
E'::[a-zA-Z_]+$'only matches simple type names. It fails for parameterized and array types:::numeric(10,2)→ cast remains (parentheses not matched)::varchar(255)→ cast remains (parentheses not matched)::int[]→ cast remains (brackets not matched)::character varying→ cast remains (space not matched)This means type casts are not actually removed for many real-world defaults, defeating the fix's purpose.
Can you confirm: what default types does the debugger tool need to support? If complex types like
numeric(10,2)or array types are used, the regex needs to be extended. A more complete pattern might look likeE'::[^,)]+$'or similar, but ideally verify what PostgreSQL'spg_get_expractually outputs for your test cases.I'll verify this review comment by examining the codebase context and researching PostgreSQL type formats.
🏁 Script executed:
Length of output: 859
🏁 Script executed:
Length of output: 0
🏁 Script executed:
Length of output: 7967
🏁 Script executed:
Length of output: 995
🏁 Script executed:
Length of output: 227
🌐 Web query:
💡 Result:
Short answer: pg_get_expr decompiles an expression (e.g. a column default) but it does not itself "pretty-print" raw typmods — when reconstructing type names it relies on format_type (or the catalog machinery that calls it). That means parameterized types appear as their SQL names with their modifiers when available (e.g. numeric(10,2), character varying(255)), and array types appear with the usual [] suffix (e.g. integer[], text[]). See examples below.
Why/how
Examples
Behavior notes
Sources
🌐 Web query:
💡 Result:
Short answer — PostgreSQL type names follow the general identifier rules. Unquoted type names: start with a letter (or underscore for general identifiers) and may contain letters, digits and underscores; they are subject to the identifier length limit (NAMEDATALEN, 63 by default). Quoted type names (double-quoted) may contain spaces, parentheses, and virtually any character except the zero byte; double quotes inside the name are written as two double quotes. Note: type names beginning with an underscore are reserved for internally-created array types, and historically a type name may have a slightly shorter usable length (see CREATE TYPE notes). [1][2][3]
References
Sources:
[1] PostgreSQL: SQL Syntax — Lexical Structure (quoted identifiers).
[2] PostgreSQL: CREATE TYPE — notes about type-name underscore reservation and length.
[3] PostgreSQL docs (current) — delimited identifier quoting and allowed characters.
Fix regex pattern to handle parameterized, array, and spaced type names.
The regex
E'::[a-zA-Z_]+$'is incomplete. PostgreSQL's pg_get_expr outputs parameterized types as their SQL names with modifiers (e.g., numeric(10,2), character varying(255)) and array types with brackets (e.g., integer[], text[]). The current pattern only matches simple type names and will fail to remove casts for:::numeric(10,2)→ parentheses not matched::varchar(255)→ parentheses not matched::int[]→ brackets not matched::character varying→ space not matchedSince the debugger handles these types (numeric, varchar, double precision), defaults with complex types will retain their casts. Replace the regex with a pattern that matches to the end of the type name, such as
E'::[^,)]*$'(matches::through end, excluding commas/closing parens that separate expressions).Apply this fix to lines 57-59 in
web/pgadmin/tools/debugger/templates/debugger/sql/get_function_debug_info.sql.🤖 Prompt for AI Agents