Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
SELECT DISTINCT att.attname as name, att.attnum as OID, pg_catalog.format_type(ty.oid,NULL) AS datatype,
pg_catalog.format_type(ty.oid,att.atttypmod) AS displaytypname,
att.attnotnull as not_null,
CASE WHEN att.atthasdef OR att.attidentity != '' OR ty.typdefault IS NOT NULL THEN True
ELSE False END as has_default_val, des.description, seq.seqtypid,
{# Detect generated columns to exclude from INSERT/UPDATE in View/Edit Data #}
CASE WHEN att.attgenerated = 's' THEN true ELSE false END as is_generated
FROM pg_catalog.pg_attribute att
JOIN pg_catalog.pg_type ty ON ty.oid=atttypid
JOIN pg_catalog.pg_namespace tn ON tn.oid=ty.typnamespace
JOIN pg_catalog.pg_class cl ON cl.oid=att.attrelid
JOIN pg_catalog.pg_namespace na ON na.oid=cl.relnamespace
LEFT OUTER JOIN pg_catalog.pg_type et ON et.oid=ty.typelem
LEFT OUTER JOIN pg_catalog.pg_attrdef def ON adrelid=att.attrelid AND adnum=att.attnum
LEFT OUTER JOIN (pg_catalog.pg_depend JOIN pg_catalog.pg_class cs ON classid='pg_class'::regclass AND objid=cs.oid AND cs.relkind='S') ON refobjid=att.attrelid AND refobjsubid=att.attnum
LEFT OUTER JOIN pg_catalog.pg_namespace ns ON ns.oid=cs.relnamespace
LEFT OUTER JOIN pg_catalog.pg_index pi ON pi.indrelid=att.attrelid AND indisprimary
LEFT OUTER JOIN pg_catalog.pg_description des ON (des.objoid=att.attrelid AND des.objsubid=att.attnum AND des.classoid='pg_class'::regclass)
LEFT OUTER JOIN pg_catalog.pg_sequence seq ON cs.oid=seq.seqrelid
WHERE

{% if tid %}
att.attrelid = {{ tid|qtLiteral(conn) }}::oid
{% endif %}
{% if table_name and table_nspname %}
cl.relname= {{table_name |qtLiteral(conn)}} and na.nspname={{table_nspname|qtLiteral(conn)}}
{% endif %}
{% if clid %}
AND att.attnum = {{ clid|qtLiteral(conn) }}
{% endif %}
{### To show system objects ###}
{% if not show_sys_objects and not has_oids %}
AND att.attnum > 0
{% endif %}
{### To show oids in view data ###}
{% if has_oids %}
AND (att.attnum > 0 OR (att.attname = 'oid' AND att.attnum < 0))
{% endif %}
AND att.attisdropped IS FALSE
ORDER BY att.attnum
Original file line number Diff line number Diff line change
Expand Up @@ -1300,8 +1300,10 @@ export function ResultSet() {
}

pageDataOutOfSync.current = true;
if(_.size(dataChangeStore.added)) {
// Update the rows in a grid after addition
// Update the rows in a grid after addition/update.
// row_added contains refetched row data with recalculated
// generated column values (for both INSERT and UPDATE).
if(_.size(dataChangeStore.added) || _.size(dataChangeStore.updated)) {
respData.data.query_results.forEach((qr)=>{
if(!_.isNull(qr.row_added)) {
let rowClientPK = Object.keys(qr.row_added)[0];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@ UPDATE {{ conn|qtIdent(nsp_name, object_name) | replace("%", "%%") }} SET
{% if not loop.first %}, {% endif %}{{ conn|qtIdent(col) | replace("%", "%%") }} = %({{ pgadmin_alias[col] }})s{% if type_cast_required[col] %}::{{ data_type[col] }}{% endif %}{% endfor %}
WHERE
{% for pk in primary_keys %}
{% if not loop.first %} AND {% endif %}{{ conn|qtIdent(pk) | replace("%", "%%") }} = {{ primary_keys[pk]|qtLiteral(conn) }}{% endfor %};
{% if not loop.first %} AND {% endif %}{{ conn|qtIdent(pk) | replace("%", "%%") }} = {{ primary_keys[pk]|qtLiteral(conn) }}{% endfor %}
{# Return primary keys to refetch row with recalculated generated column values #}
{% if pk_names and not has_oids %} RETURNING {{pk_names | replace("%", "%%")}}{% endif %}
{% if has_oids %} RETURNING oid{% endif %};
10 changes: 10 additions & 0 deletions web/pgadmin/tools/sqleditor/utils/get_column_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ def get_columns_types(is_query_tool, columns_info, table_oid, conn, has_oids,
col_type['seqtypid'] = col['seqtypid'] = \
rset['rows'][key]['seqtypid']

# Check if column is a generated column (PostgreSQL 12+).
# Generated columns must be excluded from INSERT/UPDATE.
col_type['is_generated'] = col['is_generated'] = \
rset['rows'][key].get('is_generated', False)

else:
for row in rset['rows']:
if row['oid'] == col['table_column']:
Expand All @@ -76,12 +81,17 @@ def get_columns_types(is_query_tool, columns_info, table_oid, conn, has_oids,

col_type['seqtypid'] = col['seqtypid'] = \
row['seqtypid']

# Check if column is a generated column (PG 12+).
col_type['is_generated'] = col['is_generated'] = \
row.get('is_generated', False)
break

else:
col_type['not_null'] = col['not_null'] = None
col_type['has_default_val'] = \
col['has_default_val'] = None
col_type['seqtypid'] = col['seqtypid'] = None
col_type['is_generated'] = col['is_generated'] = False

return column_types
67 changes: 60 additions & 7 deletions web/pgadmin/tools/sqleditor/utils/save_changed_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ def save_changed_data(changed_data, columns_info, conn, command_obj,
if command_obj.has_oids():
data.pop('oid', None)

# Remove generated columns (GENERATED ALWAYS AS) as they
# cannot be inserted - PostgreSQL auto-computes their values.
for col_name, col_info in columns_info.items():
if col_info.get('is_generated', False):
data.pop(col_name, None)

# Update columns value with columns having
# not_null=False and has no default value
column_data.update(data)
Expand Down Expand Up @@ -163,14 +169,38 @@ def save_changed_data(changed_data, columns_info, conn, command_obj,
# For updated rows
elif of_type == 'updated':
list_of_sql[of_type] = []

# Check if table has generated columns. If yes, we need to
# refetch row after UPDATE to get recalculated values for UI.
has_generated_cols = any(
col_info.get('is_generated', False)
for col_info in columns_info.values()
)

# Get primary keys info (same as INSERT) - needed for RETURNING
# clause and SELECT query to refetch updated row.
pk_names, primary_keys = command_obj.get_primary_keys()

Comment on lines +173 to +183
Copy link
Copy Markdown

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:

#!/bin/bash
set -euo pipefail

# 1) Inspect get_primary_keys implementations/returns used by sqleditor commands
rg -n -C3 '\bdef get_primary_keys\s*\(' web/pgadmin/tools/sqleditor web/pgadmin/browser
rg -n -C2 '\breturn\b.*(pk_names|primary_keys|keys)' web/pgadmin/tools/sqleditor web/pgadmin/browser

# 2) Trace update.sql/select.sql call sites and assumptions on returned rows
rg -n -C3 'render_template\(.+update\.sql|render_template\(.+select\.sql|res\[[\"\x27]rows[\"\x27]\]\[0\]' web/pgadmin/tools/sqleditor/utils

# 3) Check tests around generated columns + update/refetch + pk/oids edge cases
rg -n -C2 'generated|is_generated|save_changed_data|has_oids|get_primary_keys|update' web/pgadmin/tools/sqleditor/tests

Repository: pgadmin-org/pgadmin4

Length of output: 6571


🏁 Script executed:

cat -n web/pgadmin/tools/sqleditor/utils/save_changed_data.py | sed -n '160,250p'

Repository: pgadmin-org/pgadmin4

Length of output: 4654


🏁 Script executed:

rg -n 'def has_oids' web/pgadmin/tools/sqleditor

Repository: pgadmin-org/pgadmin4

Length of output: 216


🏁 Script executed:

rg -n 'res\[.rows.\]\[0\]' web/pgadmin/tools/sqleditor/utils -A 2 -B 2

Repository: pgadmin-org/pgadmin4

Length of output: 1129


🏁 Script executed:

find web/pgadmin/tools/sqleditor -name 'update.sql' -o -name '*.sql' | head -20

Repository: pgadmin-org/pgadmin4

Length of output: 1396


🏁 Script executed:

rg -n 'RETURNING' web/pgadmin/tools/sqleditor -B 3 -A 1 | head -80

Repository: pgadmin-org/pgadmin4

Length of output: 1801


🏁 Script executed:

cat -n web/pgadmin/tools/sqleditor/command.py | sed -n '553,570p'

Repository: pgadmin-org/pgadmin4

Length of output: 781


🏁 Script executed:

cat -n web/pgadmin/tools/sqleditor/command.py | sed -n '838,850p'

Repository: pgadmin-org/pgadmin4

Length of output: 541


🏁 Script executed:

cat -n web/pgadmin/tools/sqleditor/utils/save_changed_data.py | sed -n '330,365p'

Repository: pgadmin-org/pgadmin4

Length of output: 1924


🏁 Script executed:

cat -n web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/update.sql

Repository: pgadmin-org/pgadmin4

Length of output: 886


🏁 Script executed:

rg -n 'get_primary_keys' web/pgadmin/tools/sqleditor/command.py -A 10 | head -60

Repository: pgadmin-org/pgadmin4

Length of output: 1985


🏁 Script executed:

cat -n web/pgadmin/tools/sqleditor/command.py | sed -n '507,546p'

Repository: pgadmin-org/pgadmin4

Length of output: 1656


🏁 Script executed:

rg -n 'test.*generated|test.*refetch|test.*update' web/pgadmin/tools/sqleditor/tests/ -l

Repository: pgadmin-org/pgadmin4

Length of output: 46


🏁 Script executed:

# Check if there are tests for the UPDATE + generated columns scenario
rg -n 'has_generated' web/pgadmin/tools/sqleditor/tests/ -B 2 -A 2

Repository: pgadmin-org/pgadmin4

Length of output: 46


🏁 Script executed:

# Check for any error handling or guards around res['rows'][0]
rg -n "res\['rows'\]" web/pgadmin/tools/sqleditor/utils/save_changed_data.py -B 3 -A 3

Repository: pgadmin-org/pgadmin4

Length of output: 882


🏁 Script executed:

# Check if pk_names could be empty string (falsy) or explicitly None
cat -n web/pgadmin/tools/sqleditor/command.py | sed -n '518,542p'

Repository: pgadmin-org/pgadmin4

Length of output: 1049


🏁 Script executed:

# Look for any validation or tests of the refetch flow
fd test.*save_changed web/pgadmin/tools/sqleditor/tests/

Repository: pgadmin-org/pgadmin4

Length of output: 46


🏁 Script executed:

# Verify the logic: if pk_names is empty string, is it falsy in Jinja?
# Also check what execute_dict returns when there's no RETURNING clause
rg -n 'execute_dict|execute_void' web/pgadmin/tools/sqleditor/utils/save_changed_data.py -B 2 -A 2 | head -40

Repository: pgadmin-org/pgadmin4

Length of output: 1519


🏁 Script executed:

# Check if there's error handling for empty res['rows']
rg -n "rows.*\[\s*0\s*\]" web/pgadmin/tools/sqleditor/utils/save_changed_data.py -B 5 -A 2

Repository: pgadmin-org/pgadmin4

Length of output: 817


🏁 Script executed:

# Verify UserDefinedQueryCommand initialization with table_has_oids
rg -n 'table_has_oids|self.pk_names' web/pgadmin/tools/sqleditor/command.py | head -30

Repository: pgadmin-org/pgadmin4

Length of output: 493


🏁 Script executed:

# Verify what execute_dict returns when there's no RETURNING clause
rg -n 'def execute_dict' web/pgadmin/ -A 15 | head -50

Repository: pgadmin-org/pgadmin4

Length of output: 3713


🏁 Script executed:

# Check if there are integration tests for generated columns + update
find web/pgadmin/tools/sqleditor/tests -name '*.py' -exec grep -l 'generated\|refetch' {} \;

Repository: pgadmin-org/pgadmin4

Length of output: 46


🏁 Script executed:

# Trace the issue: confirm empty string pk_names is falsy in Jinja template
cat web/pgadmin/tools/sqleditor/templates/sqleditor/sql/default/update.sql

Repository: pgadmin-org/pgadmin4

Length of output: 816


🏁 Script executed:

# Verify what happens when execute_dict returns no rows (no RETURNING clause)
cat -n web/pgadmin/utils/driver/psycopg3/connection.py | sed -n '1286,1350p'

Repository: pgadmin-org/pgadmin4

Length of output: 2984


🏁 Script executed:

# Final verification: trace complete flow when UPDATE has no RETURNING clause
rg -n "res\['rows'\]" web/pgadmin/utils/driver/psycopg3/connection.py -A 3 -B 3 | head -60

Repository: pgadmin-org/pgadmin4

Length of output: 46


🏁 Script executed:

cat -n web/pgadmin/utils/driver/psycopg3/connection.py | sed -n '1350,1365p'

Repository: pgadmin-org/pgadmin4

Length of output: 756


Guard refetch enqueueing when UPDATE cannot return identifiers.

When a table has generated columns, refetch is enabled (line 220). However, the UPDATE statement only includes a RETURNING clause if pk_names is non-empty AND has_oids is false, OR if has_oids is true (update.sql lines 9–10). If both conditions fail—e.g., a table with no primary keys on PostgreSQL 12+ (where has_oids() returns False)—the UPDATE has no RETURNING clause and returns an empty row set.

Line 347 then unconditionally accesses res['rows'][0] to extract parameters for the refetch SELECT, causing an IndexError when the list is empty. This crash can occur on valid tables with generated columns but no primary keys.

Add a guard to enqueue refetch only when identifiers are returnable:

can_refetch_updated_row = command_obj.has_oids() or bool(pk_names)
if has_generated_cols and can_refetch_updated_row:
    # enqueue select_sql
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/tools/sqleditor/utils/save_changed_data.py` around lines 173 -
183, The refetch logic must be guarded so we only enqueue a refetch when the
UPDATE can actually return identifiers: compute a boolean like
can_refetch_updated_row using command_obj.has_oids() or bool(pk_names) (pk_names
/ primary_keys are from command_obj.get_primary_keys()), and change the
condition that currently uses has_generated_cols alone to require both
has_generated_cols and can_refetch_updated_row before enqueuing the SELECT (the
place that later reads res['rows'][0]); this prevents attempting to read
returned rows when the UPDATE had no RETURNING clause.

for each_row in changed_data[of_type]:
data = changed_data[of_type][each_row]['data']
row_primary_keys = changed_data[of_type][each_row][
'primary_keys']

# Remove generated columns (GENERATED ALWAYS AS) as they
# cannot be updated - PostgreSQL auto-computes their values.
for col_name, col_info in columns_info.items():
if col_info.get('is_generated', False):
data.pop(col_name, None)

pk_escaped = {
pk: pk_val.replace('%', '%%') if hasattr(
pk_val, 'replace') else pk_val
for pk, pk_val in
changed_data[of_type][each_row]['primary_keys'].items()
for pk, pk_val in row_primary_keys.items()
}

# Pass pk_names and has_oids for RETURNING clause in
# UPDATE statement.
# This will help to fetch the updated row's.
sql = render_template(
"/".join([command_obj.sql_path, 'update.sql']),
data_to_be_saved=data,
Expand All @@ -180,12 +210,35 @@ def save_changed_data(changed_data, columns_info, conn, command_obj,
nsp_name=command_obj.nsp_name,
data_type=column_type,
type_cast_required=type_cast_required,
pk_names=pk_names if has_generated_cols else None,
has_oids=command_obj.has_oids(),
conn=conn
)
list_of_sql[of_type].append({'sql': sql,
'data': data,
'row_id':
data.get(client_primary_key)})

# For tables with generated columns, add select_sql to
# refetch updated row.
if has_generated_cols:
select_sql = render_template(
"/".join([command_obj.sql_path, 'select.sql']),
object_name=command_obj.object_name,
nsp_name=command_obj.nsp_name,
pgadmin_alias=pgadmin_alias,
primary_keys=primary_keys,
has_oids=command_obj.has_oids()
)
list_of_sql[of_type].append({
'sql': sql,
'data': data,
'client_row': each_row,
'select_sql': select_sql,
'row_id': data.get(client_primary_key)
})
else:
list_of_sql[of_type].append({
'sql': sql,
'data': data,
'row_id': data.get(client_primary_key)
})

# For deleted rows
elif of_type == 'deleted':
Expand Down Expand Up @@ -287,7 +340,7 @@ def failure_handle(res, row_id):
if not status:
return failure_handle(res, item.get('row_id', 0))

# Select added row from the table
# Select added/updated row from the table
if 'select_sql' in item:
params = {
pgadmin_alias[k] if k in pgadmin_alias else k: v
Expand Down
Loading