Skip to content

Allow users to customize ofType columns during table creation #223 #9580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mzabuawala wants to merge 1 commit into
base: master
Choose a base branch
from
+102 −6
Open

Allow users to customize ofType columns during table creation #223 #9580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,20 @@ def get_oftype(self, gid, sid, did, scid, tid=None):
if not status:
return internal_server_error(errormsg=type_cols)

# Mark columns as inherited from type (not table)
# so the UI allows editing defaults/constraints
# Also store original values for comparison later
for col in type_cols['rows']:
if 'inheritedfrom' in col:
col['inheritedfromtype'] = col['inheritedfrom']
# Keep inheritedfrom for backward compatibility
# but UI will check inheritedfromtype first

# Store original values from the composite type
# This allows backend to detect actual modifications
col['original_defval'] = col.get('defval')
col['original_attnotnull'] = col.get('attnotnull', False)

res.append({
'label': row['typname'],
'value': row['typname'],
Expand Down
28 changes: 28 additions & 0 deletions ...min/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ export default class ColumnSchema extends BaseUISchema {
return { cell: this.attlenRange(state) ? 'int' : '' };
}

isInheritedFromType(state) {
return !isEmptyString(state.inheritedfromtype);
}
Comment on lines +163 to +165
Copy link

@coderabbitai coderabbitai bot Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Wire isInheritedFromType into inherited-column edit checks.

Right now the edit/readonly gates still key off inheritedfrom, and get_oftype keeps that field for backward compatibility. That means OF TYPE columns can remain read‑only, undermining the goal of making them editable. Please distinguish table inheritance from type inheritance in the checks (and in the direct inheritedfrom checks for length/precision/default).

🔧 Suggested fix 
   isInheritedFromType(state) {
     return !isEmptyString(state.inheritedfromtype);
   }
+
+  isInheritedFromTable(state) {
+    return !isEmptyString(state.inheritedfrom) && !this.isInheritedFromType(state);
+  }
...
-      if (!isEmptyString(state.inheritedfrom)){
+      if (this.isInheritedFromTable(state)){
         return true;
       }
...
-        if (!isEmptyString(state.inheritedfrom)) {
+        if (obj.isInheritedFromTable(state)) {
           return false;
         }
...
-        if (!isEmptyString(state.inheritedfrom)) {
+        if (obj.isInheritedFromTable(state)) {
           return false;
         }
...
-        return !(!isEmptyString(state.inheritedfrom) || !this.editableCheckForTable(state));
+        return !(obj.isInheritedFromTable(state) || !this.editableCheckForTable(state));
🤖 Prompt for AI Agents 
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js`
around lines 163 - 165, The edit/readonly logic currently treats any
inheritedfrom as table inheritance; update those gates to distinguish type
inheritance by using isInheritedFromType(state) so only true table inheritance
makes fields read-only. Specifically, replace direct checks of
state.inheritedfrom in the edit/readonly guards (and the
length/precision/default validation paths) with a check that inheritedfrom is
present AND isInheritedFromType(state) is false (e.g., only treat as
table-inherited when !isInheritedFromType(state) &&
!isEmptyString(state.inheritedfrom)). Also ensure get_oftype still preserves
compatibility but that any logic deciding editability consults
isInheritedFromType(state) to allow OF TYPE columns to be editable.


get baseFields() {
let obj = this;

Expand Down Expand Up @@ -284,6 +288,30 @@ export default class ColumnSchema extends BaseUISchema {
}
return false;
},
},{
/* This field is used to track inheritance from composite types */
id: 'inheritedfromtype',
label: gettext('Inherited from type'),
type: 'text',
visible: false,
},{
/* This field is used to track inheritance from parent tables */
id: 'inheritedfromtable',
label: gettext('Inherited from table'),
type: 'text',
visible: false,
},{
/* Store original DEFAULT from composite type to detect modifications */
id: 'original_defval',
label: gettext('Original Default'),
type: 'text',
visible: false,
},{
/* Store original NOT NULL from composite type to detect modifications */
id: 'original_attnotnull',
label: gettext('Original NOT NULL'),
type: 'boolean',
visible: false,
},{
id:'geometry', label: gettext('Geometry Type'), deps: ['cltype'],
group: gettext('Definition'), type: 'select', options: this.geometryTypes,
Expand Down
54 changes: 54 additions & 0 deletions web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,9 +364,63 @@ def parse_format_columns(data, mode=None):
# tables 'CREATE' mode
final_columns = []

# Get list of columns in primary key constraint
pk_columns = set()
if 'primary_key' in data and len(data['primary_key']) > 0:
for pk in data['primary_key']:
if 'columns' in pk:
for col in pk['columns']:
if 'column' in col:
pk_columns.add(col['column'])

for c in columns:
# Include non-inherited columns
if c.get('inheritedfrom', None) is None:
final_columns.append(c)
# Also include columns inherited from composite types (OF TYPE)
# that have modifications (WITH OPTIONS clause)
elif c.get('inheritedfromtype', None) is not None:
# Check if column has been modified or is in a constraint
has_modifications = False

# Check if column is in PRIMARY KEY constraint
# Note: We don't include the column for PRIMARY KEY because
# it's added as a separate table-level constraint
# Uncomment this if you want column-level PRIMARY KEY:
# if c.get('name') in pk_columns:
# has_modifications = True

# Check for PRIMARY KEY (marked via is_primary_key checkbox)
if c.get('is_primary_key'):
has_modifications = True

# Check if DEFAULT value was actually modified
# (different from type)
original_defval = c.get('original_defval')
current_defval = c.get('defval')
# Compare as strings, treating None and empty string
# as equivalent
orig_val = str(original_defval) \
if original_defval is not None else ''
curr_val = str(current_defval) \
if current_defval is not None else ''
if orig_val != curr_val:
has_modifications = True

# Check if NOT NULL was actually modified
# (different from type)
original_attnotnull = c.get('original_attnotnull', False)
current_attnotnull = c.get('attnotnull', False)
if original_attnotnull != current_attnotnull:
has_modifications = True

if has_modifications:
# Mark this column to use WITH OPTIONS syntax in template
# Skip identity columns as WITH OPTIONS
# cannot be combined with GENERATED ALWAYS AS IDENTITY
if c.get('colconstype') != 'i':
c['has_with_options'] = True
final_columns.append(c)
Comment on lines +386 to +423
Copy link

@coderabbitai coderabbitai bot Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's understand the file structure and locate the template that uses has_with_options
find web -name "*.sql" -o -name "*.j2" -o -name "*.jinja*" | head -20

Repository: pgadmin-org/pgadmin4

Length of output: 2188

🏁 Script executed:

# Search for has_with_options usage in the codebase
rg "has_with_options" --type py --type sql -B 2 -A 2

Repository: pgadmin-org/pgadmin4

Length of output: 11388

🏁 Script executed:

# Get more context around the code in question - show the full function
cd web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns && \
wc -l utils.py && \
head -n 430 utils.py | tail -n 100

Repository: pgadmin-org/pgadmin4

Length of output: 4144

🌐 Web query:

PostgreSQL ALTER TABLE column WITH OPTIONS syntax requirements - is at least one option required

💡 Result:

Yes. In PostgreSQL the ALTER TABLE ... ALTER COLUMN ... SET ( ... ) form requires at least one option inside the parentheses—the grammar is a comma-separated list, not an optional/empty group:

  • ALTER [ COLUMN ] column_name SET ( attribute_option = value [, ... ] )
  • ALTER [ COLUMN ] column_name RESET ( attribute_option [, ... ] ) (postgresql.org)

So SET () (or RESET ()) is invalid syntax. If you have nothing to set, omit that subcommand; if you want to remove options, use RESET and name one or more options (e.g. RESET (n_distinct)). (postgresql.org)

Citations:

Remove the is_primary_key check to avoid generating invalid WITH OPTIONS syntax.

The code marks columns for WITH OPTIONS when only is_primary_key is true, but the template outputs WITH OPTIONS with no options if DEFAULT and NOT NULL haven't changed. PostgreSQL requires at least one option in WITH OPTIONS (e.g. NOT NULL or DEFAULT), so this generates invalid SQL. Additionally, the comment already notes that PRIMARY KEY is handled as a table-level constraint, making this check redundant.

Suggested change 
-                # Check for PRIMARY KEY (marked via is_primary_key checkbox)
-                if c.get('is_primary_key'):
-                    has_modifications = True
🤖 Prompt for AI Agents 
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/utils.py`
around lines 386 - 423, Remove the redundant is_primary_key-driven modification
flag: delete the check that sets has_modifications when c.get('is_primary_key')
is true so primary keys are not flagged as column-level changes; rely only on
actual DEFAULT/NOT NULL differences (original_defval/defval and
original_attnotnull/attnotnull) to set has_modifications, and keep the existing
identity check and has_with_options assignment for final_columns as-is (refer to
variables/keys c, is_primary_key, has_modifications, original_defval, defval,
original_attnotnull, attnotnull, has_with_options, final_columns).

Comment on lines +367 to +423
Copy link

@coderabbitai coderabbitai bot Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's read the complete function to understand the full context
wc -l web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/utils.py

Repository: pgadmin-org/pgadmin4

Length of output: 151

🏁 Script executed:

# Read the function around lines 367-423 with more context
sed -n '340,450p' web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/utils.py

Repository: pgadmin-org/pgadmin4

Length of output: 4313

🏁 Script executed:

# Search for how has_with_options is used in the codebase
rg 'has_with_options' --type py -B 2 -A 2

Repository: pgadmin-org/pgadmin4

Length of output: 708

🏁 Script executed:

# Search for WITH OPTIONS in template files to understand SQL generation
rg 'WITH OPTIONS' --type-list

Repository: pgadmin-org/pgadmin4

Length of output: 453

🏁 Script executed:

# Also search for any SQL templates that use this flag
find . -name '*.sql' -o -name '*.jinja2' | head -20

Repository: pgadmin-org/pgadmin4

Length of output: 2228

🏁 Script executed:

# Search for table creation templates in the tables directory
find web/pgadmin/browser/server_groups/servers/databases/schemas/tables -name '*.sql' -o -name '*.jinja2' | head -30

Repository: pgadmin-org/pgadmin4

Length of output: 3907

🏁 Script executed:

# Search for how has_with_options is referenced in templates
rg 'has_with_options' --type py --type sql --type jinja2 -B 3 -A 3

Repository: pgadmin-org/pgadmin4

Length of output: 94

🏁 Script executed:

# Look for CREATE TABLE template files
find web/pgadmin/browser/server_groups/servers/databases/schemas/tables -type f -name '*create*' | head -20

Repository: pgadmin-org/pgadmin4

Length of output: 2667

🏁 Script executed:

# Find template files for tables (look for .sql files in templates directories)
find web/pgadmin/browser/server_groups/servers/databases/schemas/tables -path '*/templates/*' -type f | head -30

Repository: pgadmin-org/pgadmin4

Length of output: 3542

🏁 Script executed:

# Search for WITH OPTIONS in template files
rg 'WITH OPTIONS' -B 2 -A 2

Repository: pgadmin-org/pgadmin4

Length of output: 12287

🏁 Script executed:

# Search for the create.sql template for tables
find web/pgadmin/browser/server_groups/servers/databases/schemas/tables -name 'create.sql'

Repository: pgadmin-org/pgadmin4

Length of output: 3765

🏁 Script executed:

# Let me read the actual template file to see the exact syntax
cat web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/create.sql | grep -A 5 "has_with_options"

Repository: pgadmin-org/pgadmin4

Length of output: 1592

🏁 Script executed:

# Also check if there are any test cases that test WITH OPTIONS without actual options
find web/pgadmin/browser/server_groups/servers/databases/schemas/tables -path '*/tests/*' -name '*.sql' | xargs grep -l "WITH OPTIONS" 2>/dev/null | head -5

Repository: pgadmin-org/pgadmin4

Length of output: 46

Remove is_primary_key from triggering WITH OPTIONS syntax

Lines 394–423: Setting has_modifications = True when is_primary_key is set independently causes has_with_options to be marked even when DEFAULT and NOT NULL remain unchanged. This results in SQL like column_name WITH OPTIONS with no options, which is invalid PostgreSQL syntax. The code comments correctly note that PRIMARY KEY is handled as a table-level constraint, so it should not trigger column-level WITH OPTIONS syntax. Remove the is_primary_key check (lines 405–407) to gate has_with_options only to actual option changes (DEFAULT or NOT NULL).

🤖 Prompt for AI Agents 
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/utils.py`
around lines 367 - 423, Remove the check that sets has_modifications when
c.get('is_primary_key') is true inside the columns loop so that PRIMARY KEY does
not cause has_with_options to be set; instead only detect modifications via
differences in defval (original_defval vs defval) and attnotnull
(original_attnotnull vs attnotnull) as currently implemented, and continue to
skip identity columns (colconstype == 'i') before marking c['has_with_options']
= True; locate this logic around the loop over columns and the variables
has_modifications, c.get('is_primary_key'), original_defval/current_defval, and
original_attnotnull/current_attnotnull.


# Now we have all lis of columns which we need
# to include in our create definition, Let's format them
Expand Down
2 changes: 1 addition & 1 deletion web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.ui.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ export class LikeSchema extends BaseUISchema {
}

resetVals(state) {
if(this.isRelationDisable(state) && this.top.isNew()) {
if(this.isRelationDisable(state) && this.top && this.top.isNew()) {
return {
like_default_value: false,
like_constraints: false,
Expand Down
2 changes: 1 addition & 1 deletion ...er/server_groups/servers/databases/schemas/tables/templates/tables/sql/11_plus/create.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ CREATE {% if data.relpersistence %}UNLOGGED {% endif %}TABLE{% if add_not_exists
{% if data.columns and data.columns|length > 0 %}
{% for c in data.columns %}
{% if c.name and c.cltype %}
{% if c.inheritedfromtable %}-- Inherited from table {{c.inheritedfromtable}}: {% elif 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.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 != '' %} DEFAULT {{c.defval}}{% 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.attnotnull %} NOT NULL{% endif %}{% if c.defval is defined and c.defval is not none and c.defval != '' %} 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.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 != '' %} DEFAULT {{c.defval}}{% endif %}{% endif %}
{% if c.colconstype == 'i' and c.attidentity and c.attidentity != '' %}
{% if c.attidentity == 'a' %} GENERATED ALWAYS AS IDENTITY{% elif c.attidentity == 'd' %} GENERATED BY DEFAULT AS IDENTITY{% endif %}
{% if c.seqincrement or c.seqcycle or c.seqincrement or c.seqstart or c.seqmin or c.seqmax or c.seqcache %} ( {% endif %}
Expand Down
2 changes: 1 addition & 1 deletion ...er/server_groups/servers/databases/schemas/tables/templates/tables/sql/12_plus/create.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ CREATE {% if data.relpersistence %}UNLOGGED {% endif %}TABLE{% if add_not_exists
{% if data.columns and data.columns|length > 0 %}
{% for c in data.columns %}
{% if c.name and c.cltype %}
{% if c.inheritedfromtable %}-- Inherited from table {{c.inheritedfromtable}}: {% elif 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.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 %}
{% 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.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.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.colconstype == 'i' and c.attidentity and c.attidentity != '' %}
{% if c.attidentity == 'a' %} GENERATED ALWAYS AS IDENTITY{% elif c.attidentity == 'd' %} GENERATED BY DEFAULT AS IDENTITY{% endif %}
{% if c.seqincrement or c.seqcycle or c.seqincrement or c.seqstart or c.seqmin or c.seqmax or c.seqcache %} ( {% endif %}
Expand Down
2 changes: 1 addition & 1 deletion ...er/server_groups/servers/databases/schemas/tables/templates/tables/sql/14_plus/create.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ CREATE {% if data.relpersistence %}UNLOGGED {% endif %}TABLE{% if add_not_exists
{% if data.columns and data.columns|length > 0 %}
{% for c in data.columns %}
{% if c.name and c.cltype %}
{% if c.inheritedfromtable %}-- Inherited from table {{c.inheritedfromtable}}: {% elif 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 %}
{% 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.colconstype == 'i' and c.attidentity and c.attidentity != '' %}
{% if c.attidentity == 'a' %} GENERATED ALWAYS AS IDENTITY{% elif c.attidentity == 'd' %} GENERATED BY DEFAULT AS IDENTITY{% endif %}
{% if c.seqincrement or c.seqcycle or c.seqincrement or c.seqstart or c.seqmin or c.seqmax or c.seqcache %} ( {% endif %}
Expand Down
2 changes: 1 addition & 1 deletion ...er/server_groups/servers/databases/schemas/tables/templates/tables/sql/16_plus/create.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ CREATE {% if data.relpersistence %}UNLOGGED {% endif %}TABLE{% if add_not_exists
{% if data.columns and data.columns|length > 0 %}
{% for c in data.columns %}
{% if c.name and c.cltype %}
{% if c.inheritedfromtable %}-- Inherited from table {{c.inheritedfromtable}}: {% elif 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.attstorage is defined and c.attstorage != c.defaultstorage%} STORAGE {%if c.attstorage == 'p' %}PLAIN{% elif c.attstorage == 'm'%}MAIN{% elif c.attstorage == 'e'%}EXTERNAL{% elif c.attstorage == 'x'%}EXTENDED{% elif c.attstorage == 'd'%}DEFAULT{% 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 %}
{% 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.attstorage is defined and c.attstorage != c.defaultstorage%} STORAGE {%if c.attstorage == 'p' %}PLAIN{% elif c.attstorage == 'm'%}MAIN{% elif c.attstorage == 'e'%}EXTERNAL{% elif c.attstorage == 'x'%}EXTENDED{% elif c.attstorage == 'd'%}DEFAULT{% endif %}{% endif %}{% 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.attstorage is defined and c.attstorage != c.defaultstorage%} STORAGE {%if c.attstorage == 'p' %}PLAIN{% elif c.attstorage == 'm'%}MAIN{% elif c.attstorage == 'e'%}EXTERNAL{% elif c.attstorage == 'x'%}EXTENDED{% elif c.attstorage == 'd'%}DEFAULT{% 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.colconstype == 'i' and c.attidentity and c.attidentity != '' %}
{% if c.attidentity == 'a' %} GENERATED ALWAYS AS IDENTITY{% elif c.attidentity == 'd' %} GENERATED BY DEFAULT AS IDENTITY{% endif %}
{% if c.seqincrement or c.seqcycle or c.seqincrement or c.seqstart or c.seqmin or c.seqmax or c.seqcache %} ( {% endif %}
Expand Down
2 changes: 1 addition & 1 deletion ...er/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/create.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ CREATE {% if data.relpersistence %}UNLOGGED {% endif %}TABLE{% if add_not_exists
{% if data.columns and data.columns|length > 0 %}
{% for c in data.columns %}
{% if c.name and c.cltype %}
{% if c.inheritedfromtable %}-- Inherited from table {{c.inheritedfromtable}}: {% elif 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.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 != '' %} DEFAULT {{c.defval}}{% 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.attnotnull %} NOT NULL{% endif %}{% if c.defval is defined and c.defval is not none and c.defval != '' %} 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)}}{{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.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 != '' %} DEFAULT {{c.defval}}{% endif %}{% endif %}
Copy link

@coderabbitai coderabbitai bot Jan 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Template syntax error: Missing {% if is_sql %} conditional around type rendering.

The template has a malformed conditional structure. Comparing to the other templates (11_plus, 12_plus, 14_plus), there's a missing {% if is_sql %} check before {{c.displaytypname}}.

Current (broken):

{{conn|qtIdent(c.name)}}{{c.displaytypname}}{% else %}{{ GET_TYPE.CREATE_TYPE_SQL(...) }}{% endif %}

The {% else %} has no corresponding {% if %} in scope, which will cause a Jinja2 syntax error.

Expected (from other templates):

{{conn|qtIdent(c.name)}} {% if is_sql %}{{c.displaytypname}}{% else %}{{ GET_TYPE.CREATE_TYPE_SQL(...) }}{% endif %}
🐛 Proposed fix 
-    {% 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.attnotnull %} NOT NULL{% endif %}{% if c.defval is defined and c.defval is not none and c.defval != '' %} 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)}}{{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.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 != '' %} 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.attnotnull %} NOT NULL{% endif %}{% if c.defval is defined and c.defval is not none and c.defval != '' %} 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.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 != '' %} DEFAULT {{c.defval}}{% endif %}{% endif %}
🤖 Prompt for AI Agents 
In
`@web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/tables/sql/default/create.sql`
at line 49, The template has a malformed conditional around rendering the column
type: wrap the direct type rendering ({{c.displaytypname}}) with an {% if is_sql
%} ... {% else %} ... {% endif %} so the existing {% else %} and {% endif %}
pair up correctly; specifically, after rendering the column name
({{conn|qtIdent(c.name)}}) insert {% if is_sql %}{{c.displaytypname}}{% else %}
and keep the existing GET_TYPE.CREATE_TYPE_SQL(conn, c.cltype, c.attlen,
c.attprecision, c.hasSqrBracket) branch as the else, ensuring the final {% endif
%} closes this new conditional.

✅ Addressed in commit be63f61

{% if c.colconstype == 'i' and c.attidentity and c.attidentity != '' %}
{% if c.attidentity == 'a' %} GENERATED ALWAYS AS IDENTITY{% elif c.attidentity == 'd' %} GENERATED BY DEFAULT AS IDENTITY{% endif %}
{% if c.seqincrement or c.seqcycle or c.seqincrement or c.seqstart or c.seqmin or c.seqmax or c.seqcache %} ( {% endif %}
Expand Down
Loading