From 0473be5ecb286656e228f1bd25a2b49fc97e2466 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Thu, 20 Aug 2026 14:20:29 +0530 Subject: [PATCH v1] Fix internal errors reachable from SQL Several conditions that ordinary SQL can reach were reported as XX000 (internal_error), either because the error was raised without an errcode() or because a case that can never work reached an elog(). Report proper error codes instead, following what comparable errors already use. * nodeModifyTable.c: a null FOR PORTION OF target is invalid input, so use ERRCODE_NULL_VALUE_NOT_ALLOWED, and reword the message to "FOR PORTION OF target must not be null" to match executor messages such as "frame starting offset must not be null". Oversight in commit 8e72d914c52. (bug #19630) * rewriteManip.c: referencing a system column of the target table in the RETURNING list of a query rewritten by a rule with its own RETURNING clause hit an internal error, since the rule's list only has entries for user columns. Such queries can never work, because the rewritten query need not scan the original target relation at all, so throw a user-facing error. (bug #19632) * stat_utils.c: the variadic name/value checks validate user-supplied arguments, so use ERRCODE_INVALID_PARAMETER_VALUE, as the rest of the file does; the pg_statistic slot limit is ERRCODE_PROGRAM_LIMIT_EXCEEDED. (bug #19629) * tid.c: currtid_for_view() cannot always resolve the view's ctid to a base relation. The other unsupported-view checks beside it report ERRCODE_FEATURE_NOT_SUPPORTED, so match them. (bug #19631) * varlena.c: unicode_assigned() rejects non-UTF8 server encodings; unicode_normalize() and formatting.c use ERRCODE_SYNTAX_ERROR for the same restriction, so match them. * pg_controldata.c: the pg_control_*() functions raise a control file CRC mismatch, which is ERRCODE_DATA_CORRUPTED. * gist.c: an inner tuple left invalid by a pre-9.1 crash is a corrupt index, so use ERRCODE_INDEX_CORRUPTED, like the checks in gistutil.c. * collationcmds.c: refusing ALTER COLLATION ... REFRESH VERSION for the default collation is ERRCODE_WRONG_OBJECT_TYPE. Author: Zsolt Parragi Author: Ayush Tiwari Bug: #19629 Bug: #19630 Bug: #19631 Bug: #19632 Reported-by: Zheng Wang Reported-by: Yanjie Zhao Reported-by: Yiyang Liu Discussion: https://postgr.es/m/19629-76babc04b683594d@postgresql.org Discussion: https://postgr.es/m/19630-9f10ca28426295fa@postgresql.org Discussion: https://postgr.es/m/19631-b443dd6cd8d4e40b@postgresql.org Discussion: https://postgr.es/m/19632-9155d9baec763c8c@postgresql.org --- src/backend/access/gist/gist.c | 3 ++- src/backend/commands/collationcmds.c | 3 ++- src/backend/executor/nodeModifyTable.c | 5 +++-- src/backend/rewrite/rewriteManip.c | 19 ++++++++++++++++++- src/backend/statistics/stat_utils.c | 10 +++++++--- src/backend/utils/adt/tid.c | 4 +++- src/backend/utils/adt/varlena.c | 3 ++- src/backend/utils/misc/pg_controldata.c | 12 ++++++++---- src/test/regress/expected/for_portion_of.out | 8 ++++---- src/test/regress/expected/returning.out | 7 +++++++ src/test/regress/sql/returning.sql | 5 +++++ 11 files changed, 61 insertions(+), 18 deletions(-) diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index 8565e225be7..44597793433 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -762,7 +762,8 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, */ if (GistTupleIsInvalid(idxtuple)) ereport(ERROR, - (errmsg("index \"%s\" contains an inner tuple marked as invalid", + (errcode(ERRCODE_INDEX_CORRUPTED), + errmsg("index \"%s\" contains an inner tuple marked as invalid", RelationGetRelationName(r)), errdetail("This is caused by an incomplete page split at crash recovery before upgrading to PostgreSQL 9.1."), errhint("Please REINDEX it."))); diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c index cfa0e4610d9..92faa60a750 100644 --- a/src/backend/commands/collationcmds.c +++ b/src/backend/commands/collationcmds.c @@ -443,7 +443,8 @@ AlterCollation(AlterCollationStmt *stmt) if (collOid == DEFAULT_COLLATION_OID) ereport(ERROR, - (errmsg("cannot refresh version of default collation"), + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("cannot refresh version of default collation"), /* translator: %s is an SQL command */ errhint("Use %s instead.", "ALTER DATABASE ... REFRESH COLLATION VERSION"))); diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index ca954729f1e..3056b850f73 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -5645,8 +5645,9 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) */ if (isNull) ereport(ERROR, - (errmsg("FOR PORTION OF target was null")), - executor_errposition(estate, forPortionOf->targetLocation)); + (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), + errmsg("FOR PORTION OF target must not be null"), + executor_errposition(estate, forPortionOf->targetLocation))); /* Create state for FOR PORTION OF operation */ diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c index 9c9d1cad33b..7ea55f50246 100644 --- a/src/backend/rewrite/rewriteManip.c +++ b/src/backend/rewrite/rewriteManip.c @@ -1851,7 +1851,24 @@ ReplaceVarFromTargetList(const Var *var, switch (nomatch_option) { case REPLACEVARS_REPORT_ERROR: - /* fall through, throw error below */ + + /* + * A system column can never match a targetlist entry, since + * those all have positive resnos. The way to get here is to + * refer to a system column of the target table in the + * RETURNING list of a query that is subject to a rewrite + * rule with its own RETURNING clause; the rule has no way to + * provide values for such a column, so throw a suitable + * user-facing error. + */ + if (var->varattno < 0) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot use system column \"%s\" in RETURNING list of a query that is rewritten by a rule", + get_rte_attribute_name(target_rte, + var->varattno)))); + + /* else fall through, throw internal error below */ break; case REPLACEVARS_CHANGE_VARNO: diff --git a/src/backend/statistics/stat_utils.c b/src/backend/statistics/stat_utils.c index 5ff37ef4cf8..f911a46bddb 100644 --- a/src/backend/statistics/stat_utils.c +++ b/src/backend/statistics/stat_utils.c @@ -370,6 +370,7 @@ stats_fill_fcinfo_from_arg_pairs(FunctionCallInfo pairs_fcinfo, if (nargs % 2 != 0) ereport(ERROR, + errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("variadic arguments must be name/value pairs"), errhint("Provide an even number of variadic arguments that can be divided into pairs.")); @@ -385,11 +386,13 @@ stats_fill_fcinfo_from_arg_pairs(FunctionCallInfo pairs_fcinfo, if (argnulls[i]) ereport(ERROR, - (errmsg("name at variadic position %d is null", i + 1))); + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("name at variadic position %d is null", i + 1))); if (types[i] != TEXTOID) ereport(ERROR, - (errmsg("name at variadic position %d has type %s, expected type %s", + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("name at variadic position %d has type %s, expected type %s", i + 1, format_type_be(types[i]), format_type_be(TEXTOID)))); @@ -654,7 +657,8 @@ statatt_set_slot(Datum *values, bool *nulls, bool *replaces, if (slotidx >= STATISTIC_NUM_SLOTS) ereport(ERROR, - (errmsg("maximum number of statistics slots exceeded: %d", + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("maximum number of statistics slots exceeded: %d", slotidx + 1))); stakind_attnum = Anum_pg_statistic_stakind1 - 1 + slotidx; diff --git a/src/backend/utils/adt/tid.c b/src/backend/utils/adt/tid.c index a97873f91ba..9b2cca0db4d 100644 --- a/src/backend/utils/adt/tid.c +++ b/src/backend/utils/adt/tid.c @@ -432,7 +432,9 @@ currtid_for_view(Relation viewrel, const ItemPointerData *tid) break; } } - elog(ERROR, "currtid cannot handle this view"); + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("currtid cannot handle this view")); return NULL; } diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index a09a9e5d5bb..823381073b8 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -5482,7 +5482,8 @@ unicode_assigned(PG_FUNCTION_ARGS) if (GetDatabaseEncoding() != PG_UTF8) ereport(ERROR, - (errmsg("Unicode categorization can only be performed if server encoding is UTF8"))); + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("Unicode categorization can only be performed if server encoding is UTF8"))); /* convert to char32_t */ size = pg_mbstrlen_with_len(VARDATA_ANY(input), VARSIZE_ANY_EXHDR(input)); diff --git a/src/backend/utils/misc/pg_controldata.c b/src/backend/utils/misc/pg_controldata.c index ab74d169c96..4fb56aa413c 100644 --- a/src/backend/utils/misc/pg_controldata.c +++ b/src/backend/utils/misc/pg_controldata.c @@ -47,7 +47,8 @@ pg_control_system(PG_FUNCTION_ARGS) LWLockRelease(ControlFileLock); if (!crc_ok) ereport(ERROR, - (errmsg("calculated CRC checksum does not match value stored in file"))); + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("calculated CRC checksum does not match value stored in file"))); values[0] = Int32GetDatum(ControlFile->pg_control_version); nulls[0] = false; @@ -87,7 +88,8 @@ pg_control_checkpoint(PG_FUNCTION_ARGS) LWLockRelease(ControlFileLock); if (!crc_ok) ereport(ERROR, - (errmsg("calculated CRC checksum does not match value stored in file"))); + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("calculated CRC checksum does not match value stored in file"))); /* * Calculate name of the WAL file containing the latest checkpoint's REDO @@ -184,7 +186,8 @@ pg_control_recovery(PG_FUNCTION_ARGS) LWLockRelease(ControlFileLock); if (!crc_ok) ereport(ERROR, - (errmsg("calculated CRC checksum does not match value stored in file"))); + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("calculated CRC checksum does not match value stored in file"))); values[0] = LSNGetDatum(ControlFile->minRecoveryPoint); nulls[0] = false; @@ -225,7 +228,8 @@ pg_control_init(PG_FUNCTION_ARGS) LWLockRelease(ControlFileLock); if (!crc_ok) ereport(ERROR, - (errmsg("calculated CRC checksum does not match value stored in file"))); + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("calculated CRC checksum does not match value stored in file"))); values[0] = Int32GetDatum(ControlFile->maxAlign); nulls[0] = false; diff --git a/src/test/regress/expected/for_portion_of.out b/src/test/regress/expected/for_portion_of.out index a6cb1ba8380..1a53e549de0 100644 --- a/src/test/regress/expected/for_portion_of.out +++ b/src/test/regress/expected/for_portion_of.out @@ -407,7 +407,7 @@ UPDATE for_portion_of_test FOR PORTION OF valid_at (NULL) SET name = 'one^3' WHERE id = '[1,2)'; -ERROR: FOR PORTION OF target was null +ERROR: FOR PORTION OF target must not be null LINE 2: FOR PORTION OF valid_at (NULL) ^ -- Updating with a direct target of empty does nothing @@ -884,7 +884,7 @@ LINE 2: FOR PORTION OF valid_at (4) DELETE FROM for_portion_of_test FOR PORTION OF valid_at (NULL) WHERE id = '[1,2)'; -ERROR: FOR PORTION OF target was null +ERROR: FOR PORTION OF target must not be null LINE 2: FOR PORTION OF valid_at (NULL) ^ -- Deleting with a direct target of empty does nothing @@ -1971,7 +1971,7 @@ UPDATE for_portion_of_test2 FOR PORTION OF valid_at (NULL) SET name = 'one^3' WHERE id = '[1,2)'; -ERROR: FOR PORTION OF target was null +ERROR: FOR PORTION OF target must not be null LINE 2: FOR PORTION OF valid_at (NULL) ^ -- Updating with empty does nothing @@ -2035,7 +2035,7 @@ LINE 2: FOR PORTION OF valid_at (4) DELETE FROM for_portion_of_test2 FOR PORTION OF valid_at (NULL) WHERE id = '[2,3)'; -ERROR: FOR PORTION OF target was null +ERROR: FOR PORTION OF target must not be null LINE 2: FOR PORTION OF valid_at (NULL) ^ -- Deleting with empty does nothing diff --git a/src/test/regress/expected/returning.out b/src/test/regress/expected/returning.out index ca83d9fcc09..8e3a654497b 100644 --- a/src/test/regress/expected/returning.out +++ b/src/test/regress/expected/returning.out @@ -702,6 +702,13 @@ DELETE FROM foo WHERE f1 = 4 RETURNING old.*,new.*, *; 4 | conflicted | -1 | 99 | 4 | conflicted (deleted) | -1 | -1 | 4 | conflicted (deleted) | -1 | -1 (1 row) +-- system columns are not available when the query is rewritten by a rule +DELETE FROM foo WHERE f1 = 4 RETURNING tableoid; +ERROR: cannot use system column "tableoid" in RETURNING list of a query that is rewritten by a rule +DELETE FROM foo WHERE f1 = 4 RETURNING old.ctid; +ERROR: cannot use system column "ctid" in RETURNING list of a query that is rewritten by a rule +DELETE FROM foo WHERE f1 = 4 RETURNING new.xmin; +ERROR: cannot use system column "xmin" in RETURNING list of a query that is rewritten by a rule -- UPDATE on view with rule EXPLAIN (verbose, costs off) UPDATE joinview SET f3 = f3 + 1 WHERE f3 = 57 diff --git a/src/test/regress/sql/returning.sql b/src/test/regress/sql/returning.sql index 27fa4a374ed..a893f47e34d 100644 --- a/src/test/regress/sql/returning.sql +++ b/src/test/regress/sql/returning.sql @@ -289,6 +289,11 @@ EXPLAIN (verbose, costs off) DELETE FROM foo WHERE f1 = 4 RETURNING old.*,new.*, *; DELETE FROM foo WHERE f1 = 4 RETURNING old.*,new.*, *; +-- system columns are not available when the query is rewritten by a rule +DELETE FROM foo WHERE f1 = 4 RETURNING tableoid; +DELETE FROM foo WHERE f1 = 4 RETURNING old.ctid; +DELETE FROM foo WHERE f1 = 4 RETURNING new.xmin; + -- UPDATE on view with rule EXPLAIN (verbose, costs off) UPDATE joinview SET f3 = f3 + 1 WHERE f3 = 57 base-commit: 170c9344defaefb351a9dbd0134f4c37a9847358 -- 2.34.1