Fix index/constraint column not shown when its name requires quoting (#6481) - #10039
Fix index/constraint column not shown when its name requires quoting (#6481)#10039dpage wants to merge 2 commits into
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan includes up to 8 reviews per rolling hour; 4 remain after this review. WalkthroughThe change adds ChangesQuoted Identifier Handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change corrects display and editing of index or constraint columns with quoted names, with focused coverage added; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ea60e15 to
464cc15
Compare
asheshv
left a comment
asheshv
left a comment
There was a problem hiding this comment.
The SQL fix (comparing against quote_ident(a.attname) instead of a.attname) is correct — is_exp will now classify properly. But the Python side that displays the column name is still broken for the exact category the PR claims to fix.
web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/utils.py:123 uses attdef.strip('"'). str.strip('"') strips outer quotes but doesn't unescape doubled inner quotes. For a column named col"x, pg_get_indexdef returns "col""x"; .strip('"') yields col""x, not col"x. So the properties panel still displays the wrong name for any identifier containing a literal ".
Pull this into a tiny helper:
def unquote_ident(s):
if s.startswith('"') and s.endswith('"'):
return s[1:-1].replace('""', '"')
return sTests gap: no scenarios for quoted-identifier column names (uppercase, spaces, reserved words, embedded "). The regression that caused #6481 wouldn't have been caught by existing tests, and this PR doesn't add coverage for it.
464cc15 to
9adc76c
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
…g#6481 The is_exp flag compared pg_get_indexdef() (which returns the SQL-quoted identifier) directly against a.attname (the raw name). For any column name needing quoting these differ, so a plain column was wrongly treated as an expression and not rendered, and validation reported it empty. Compare against pg_catalog.quote_ident(a.attname) instead, which matches pg_get_indexdef()'s output for both normal and quoted names; real expressions (attname NULL) still evaluate as expressions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The SQL fix classifies the column correctly, but the Python that displays
it still used str.strip('"'), which removes the outer quotes without
unescaping the doubled inner ones, so a column named col"x came back as
col""x and the properties panel showed the wrong name. Worse, the create
templates re-quote that value with qtIdent(), turning it into
"col""""x" in generated DDL.
unquote_ident() in pgadmin.utils reverses quote_ident() properly: it
unescapes doubled quotes, and because it only matches a string that is
entirely one quoted identifier it leaves expressions such as "a" || "b"
alone, which strip() mangled. The same pattern appeared in the index,
exclusion, index and foreign key constraint code, so all five call sites
now share the helper.
Tests cover the helper directly, and a new index test asserts the
Properties panel reports the right column for a mixed case name, a
reserved word and a name containing a literal double quote, none of which
had any coverage.
The 11_plus index template hunk from the original commit is dropped: that
bucket no longer exists on master.
9adc76c to
067830a
Compare
2 participants
Summary
Fixes #6481.
When an index (or exclusion constraint) column name requires quoting — e.g. it contains a double quote or other special characters — the column was not shown in the Properties panel, and editing reported it as empty.
Root cause: the
is_exp(expression) flag is computed aspg_get_indexdef(...) = a.attname.pg_get_indexdef()returns the SQL-quoted identifier (e.g."col""x") whilea.attnameis the raw name (col"x). For any name needing quoting these never match, so a plain column was misclassified as an expression and dropped.Fix: compare against
pg_catalog.quote_ident(a.attname), which yields the same quoted form aspg_get_indexdef()for both normal and special names. Real expression indexes (whereattnameis NULL) still evaluate as expressions.Changes
indexes/sql/11_plus/column_details.sql,indexes/sql/default/column_details.sql,exclusion_constraint/sql/default/get_constraint_cols.sql🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests