Fixed the issue where the table inheritance missing alter ... default…… statements in created EDIT-script. #9595#9774
Open
khushboovashi wants to merge 1 commit intopgadmin-org:masterfrom
Open
Conversation
… statements in created EDIT-script. pgadmin-org#9595
WalkthroughUpdates to SQL generation templates across multiple PostgreSQL versions to properly handle DEFAULT values for columns inherited from parent tables. The templates now emit DEFAULT clauses for inherited columns during table creation and apply defaults via ALTER TABLE statements post-creation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/14_plus/create.sql`:
- Around line 55-57: The template treats items with c.inheritedfromtable as
comments but still uses data.columns and loop.last (and the same
comma/constraint guards) causing stray commas and invalid SQL; change the
separator logic to only consider columns that emit real DDL. Fix by deriving a
filtered list (e.g., columns_for_ddl = [c for c in data.columns if not
c.inheritedfromtable]) or by tracking a boolean "emitted" flag inside the loop
when you actually output a column (the branches that render
{{conn|qtIdent(c.name)}} and type/DEFAULT/NOT NULL); then use that filtered list
or the emitted flag instead of loop.last / data.columns when deciding to print
commas and when generating the ALTER COLUMN ... SET DEFAULT block so
inherited-table comment lines are excluded from comma handling and subsequent
ALTER blocks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7cc658ab-b280-4a81-8f7b-dc8825356530
📒 Files selected for processing (5)
web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/11_plus/create.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/12_plus/create.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/14_plus/create.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/16_plus/create.sqlweb/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/create.sql
Comment on lines
55
to
+57
| {% for c in data.columns %} | ||
| {% if c.name and c.cltype %} | ||
| {% if c.inheritedfromtype and c.has_with_options %}{# Use WITH OPTIONS syntax for modified OF TYPE columns #}{{conn|qtIdent(c.name)}} WITH OPTIONS{% if c.attcompression is defined and c.attcompression is not none and c.attcompression != '' %} COMPRESSION {{c.attcompression}}{% endif %}{% if c.attnotnull %} NOT NULL{% endif %}{% if c.defval is defined and c.defval is not none and c.defval != '' and c.colconstype != 'g' %} DEFAULT {{c.defval}}{% endif %}{% elif c.inheritedfromtable %}{# Inherited from parent table - keep as comment #}-- Inherited from table {{c.inheritedfromtable}}: {{conn|qtIdent(c.name)}}{% else %}{# Regular column or inherited without modifications #}{% if c.inheritedfromtype %}-- Inherited from type {{c.inheritedfromtype}}: {% endif %}{{conn|qtIdent(c.name)}} {% if is_sql %}{{c.displaytypname}}{% else %}{{ GET_TYPE.CREATE_TYPE_SQL(conn, c.cltype, c.attlen, c.attprecision, c.hasSqrBracket) }}{% endif %}{% if c.geometry and not is_sql %}({{c.geometry}}{% if c.srid %},{{c.srid}}{% endif %}){% endif %}{% if c.attcompression is defined and c.attcompression is not none and c.attcompression != '' %} COMPRESSION {{c.attcompression}}{% endif %}{% if c.collspcname %} COLLATE {{c.collspcname}}{% endif %}{% if c.attnotnull %} NOT NULL{% endif %}{% if c.defval is defined and c.defval is not none and c.defval != '' and c.colconstype != 'g' %} DEFAULT {{c.defval}}{% endif %}{% endif %} | ||
| {% if c.inheritedfromtype and c.has_with_options %}{# Use WITH OPTIONS syntax for modified OF TYPE columns #}{{conn|qtIdent(c.name)}} WITH OPTIONS{% if c.attcompression is defined and c.attcompression is not none and c.attcompression != '' %} COMPRESSION {{c.attcompression}}{% endif %}{% if c.attnotnull %} NOT NULL{% endif %}{% if c.defval is defined and c.defval is not none and c.defval != '' and c.colconstype != 'g' %} DEFAULT {{c.defval}}{% endif %}{% elif c.inheritedfromtable %}{# Inherited from parent table - keep as comment #}-- Inherited from table {{c.inheritedfromtable}}: {{conn|qtIdent(c.name)}}{% if c.defval is defined and c.defval is not none and c.defval != '' %} DEFAULT {{c.defval}}{% endif %}{% else %}{# Regular column or inherited without modifications #}{% if c.inheritedfromtype %}-- Inherited from type {{c.inheritedfromtype}}: {% endif %}{{conn|qtIdent(c.name)}} {% if is_sql %}{{c.displaytypname}}{% else %}{{ GET_TYPE.CREATE_TYPE_SQL(conn, c.cltype, c.attlen, c.attprecision, c.hasSqrBracket) }}{% endif %}{% if c.geometry and not is_sql %}({{c.geometry}}{% if c.srid %},{{c.srid}}{% endif %}){% endif %}{% if c.attcompression is defined and c.attcompression is not none and c.attcompression != '' %} COMPRESSION {{c.attcompression}}{% endif %}{% if c.collspcname %} COLLATE {{c.collspcname}}{% endif %}{% if c.attnotnull %} NOT NULL{% endif %}{% if c.defval is defined and c.defval is not none and c.defval != '' and c.colconstype != 'g' %} DEFAULT {{c.defval}}{% endif %}{% endif %} |
There was a problem hiding this comment.
Don't count inherited-table comments as real column definitions.
c.inheritedfromtable still emits only a -- ... comment here, but comma handling still uses the full data.columns list (loop.last in this loop and the constraint comma guards below). As soon as the table has any other rendered item, the output becomes (... -- inherited col\n, next_item ...) or leaves a trailing comma before ), which is invalid SQL. That also prevents the new post-create ALTER COLUMN ... SET DEFAULT block from ever running. Please base separator logic on only the entries that emit actual DDL, or move inherited-table comments outside the column list.
Also applies to: 71-71, 77-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/14_plus/create.sql`
around lines 55 - 57, The template treats items with c.inheritedfromtable as
comments but still uses data.columns and loop.last (and the same
comma/constraint guards) causing stray commas and invalid SQL; change the
separator logic to only consider columns that emit real DDL. Fix by deriving a
filtered list (e.g., columns_for_ddl = [c for c in data.columns if not
c.inheritedfromtable]) or by tracking a boolean "emitted" flag inside the loop
when you actually output a column (the branches that render
{{conn|qtIdent(c.name)}} and type/DEFAULT/NOT NULL); then use that filtered list
or the emitted flag instead of loop.last / data.columns when deciding to print
commas and when generating the ALTER COLUMN ... SET DEFAULT block so
inherited-table comment lines are excluded from comma handling and subsequent
ALTER blocks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit