Skip to content

Commit b81a71a

Browse files
michaelpqmichaelpq
committed
Assign error codes where missing for user-facing failures
All the errors triggered in the code paths patched here would cause the backend to issue an internal_error errcode, which is a state that should be used only for "can't happen" situations. However, these code paths are reachable by the regression tests, and could be seen by users in valid cases. Some regression tests expect internal errcodes as they manipulate the backend state to cause corruption (like checksums), or use elog() because it is more convenient (like injection points), these have no need to change. This reduces the number of internal failures triggered in a check-world by more than half, while providing correct errcodes for these valid cases. Reviewed-by: Robert Haas Discussion: https://postgr.es/m/Zic_GNgos5sMxKoa@paquier.xyz
1 parent 6897f0e commit b81a71a

File tree

12 files changed

+59
-26
lines changed

12 files changed

+59
-26
lines changed

‎contrib/pg_walinspect/pg_walinspect.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ InitXLogReaderState(XLogRecPtr lsn)
101101
*/
102102
if (lsn < XLOG_BLCKSZ)
103103
ereport(ERROR,
104-
(errmsg("could not read WAL at LSN %X/%X",
104+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
105+
errmsg("could not read WAL at LSN %X/%X",
105106
LSN_FORMAT_ARGS(lsn))));
106107

107108
private_data = (ReadLocalXLogPageNoWaitPrivate *)

‎src/backend/access/transam/xlogrecovery.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1898,7 +1898,8 @@ PerformWalRecovery(void)
18981898
recoveryTarget != RECOVERY_TARGET_UNSET &&
18991899
!reachedRecoveryTarget)
19001900
ereport(FATAL,
1901-
(errmsg("recovery ended before configured recovery target was reached")));
1901+
(errcode(ERRCODE_CONFIG_FILE_ERROR),
1902+
errmsg("recovery ended before configured recovery target was reached")));
19021903
}
19031904

19041905
/*

‎src/backend/backup/basebackup.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,12 +2045,14 @@ _tarWriteHeader(bbsink *sink, const char *filename, const char *linktarget,
20452045
break;
20462046
case TAR_NAME_TOO_LONG:
20472047
ereport(ERROR,
2048-
(errmsg("file name too long for tar format: \"%s\"",
2048+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2049+
errmsg("file name too long for tar format: \"%s\"",
20492050
filename)));
20502051
break;
20512052
case TAR_SYMLINK_TOO_LONG:
20522053
ereport(ERROR,
2053-
(errmsg("symbolic link target too long for tar format: "
2054+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2055+
errmsg("symbolic link target too long for tar format: "
20542056
"file name \"%s\", target \"%s\"",
20552057
filename, linktarget)));
20562058
break;

‎src/backend/commands/matview.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -803,7 +803,9 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
803803
* That's a pretty silly thing to do.)
804804
*/
805805
if (!foundUniqueIndex)
806-
elog(ERROR, "could not find suitable unique index on materialized view");
806+
ereport(ERROR,
807+
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
808+
errmsg("could not find suitable unique index on materialized view"));
807809

808810
appendStringInfoString(&querybuf,
809811
" AND newdata.* OPERATOR(pg_catalog.*=) mv.*) "

‎src/backend/commands/tablecmds.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11363,7 +11363,8 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, bool recurse,
1136311363
}
1136411364

1136511365
ereport(ERROR,
11366-
(errmsg("cannot alter constraint \"%s\" on relation \"%s\"",
11366+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
11367+
errmsg("cannot alter constraint \"%s\" on relation \"%s\"",
1136711368
cmdcon->conname, RelationGetRelationName(rel)),
1136811369
ancestorname && ancestortable ?
1136911370
errdetail("Constraint \"%s\" is derived from constraint \"%s\" of relation \"%s\".",

‎src/backend/commands/trigger.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,6 +1519,7 @@ renametrig(RenameStmt *stmt)
15191519
*/
15201520
if (OidIsValid(trigform->tgparentid))
15211521
ereport(ERROR,
1522+
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
15221523
errmsg("cannot rename trigger \"%s\" on table \"%s\"",
15231524
stmt->subname, RelationGetRelationName(targetrel)),
15241525
errhint("Rename the trigger on the partitioned table \"%s\" instead.",

‎src/backend/commands/user.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1730,6 +1730,7 @@ AddRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
17301730
*/
17311731
if (memberid == ROLE_PG_DATABASE_OWNER)
17321732
ereport(ERROR,
1733+
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
17331734
errmsg("role \"%s\" cannot be a member of any role",
17341735
get_rolespec_name(memberRole)));
17351736

@@ -2121,6 +2122,7 @@ check_role_membership_authorization(Oid currentUserId, Oid roleid,
21212122
*/
21222123
if (is_grant && roleid == ROLE_PG_DATABASE_OWNER)
21232124
ereport(ERROR,
2125+
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
21242126
errmsg("role \"%s\" cannot have explicit members",
21252127
GetUserNameFromId(roleid, false)));
21262128

‎src/backend/libpq/be-secure-openssl.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,8 @@ be_tls_init(bool isServerStart)
250250
if (ssl_ver_min > ssl_ver_max)
251251
{
252252
ereport(isServerStart ? FATAL : LOG,
253-
(errmsg("could not set SSL protocol version range"),
253+
(errcode(ERRCODE_CONFIG_FILE_ERROR),
254+
errmsg("could not set SSL protocol version range"),
254255
errdetail("\"%s\" cannot be higher than \"%s\"",
255256
"ssl_min_protocol_version",
256257
"ssl_max_protocol_version")));

‎src/backend/optimizer/util/appendinfo.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,15 @@ make_inh_translation_list(Relation oldrelation, Relation newrelation,
160160

161161
/* Found it, check type and collation match */
162162
if (atttypid != att->atttypid || atttypmod != att->atttypmod)
163-
elog(ERROR, "attribute \"%s\" of relation \"%s\" does not match parent's type",
164-
attname, RelationGetRelationName(newrelation));
163+
ereport(ERROR,
164+
(errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
165+
errmsg("attribute \"%s\" of relation \"%s\" does not match parent's type",
166+
attname, RelationGetRelationName(newrelation))));
165167
if (attcollation != att->attcollation)
166-
elog(ERROR, "attribute \"%s\" of relation \"%s\" does not match parent's collation",
167-
attname, RelationGetRelationName(newrelation));
168+
ereport(ERROR,
169+
(errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
170+
errmsg("attribute \"%s\" of relation \"%s\" does not match parent's collation",
171+
attname, RelationGetRelationName(newrelation))));
168172

169173
vars = lappend(vars, makeVar(newvarno,
170174
(AttrNumber) (new_attno + 1),

‎src/backend/replication/slotfuncs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,8 @@ pg_replication_slot_advance(PG_FUNCTION_ARGS)
523523

524524
if (XLogRecPtrIsInvalid(moveto))
525525
ereport(ERROR,
526-
(errmsg("invalid target WAL LSN")));
526+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
527+
errmsg("invalid target WAL LSN")));
527528

528529
/* Build a tuple descriptor for our result type */
529530
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)

‎src/backend/utils/adt/pg_locale.c

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1481,7 +1481,8 @@ make_icu_collator(const char *iculocstr,
14811481
UCOL_DEFAULT, UCOL_DEFAULT_STRENGTH, NULL, &status);
14821482
if (U_FAILURE(status))
14831483
ereport(ERROR,
1484-
(errmsg("could not open collator for locale \"%s\" with rules \"%s\": %s",
1484+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
1485+
errmsg("could not open collator for locale \"%s\" with rules \"%s\": %s",
14851486
iculocstr, icurules, u_errorName(status))));
14861487
}
14871488

@@ -2609,7 +2610,8 @@ pg_ucol_open(const char *loc_str)
26092610
if (U_FAILURE(status) || status == U_STRING_NOT_TERMINATED_WARNING)
26102611
{
26112612
ereport(ERROR,
2612-
(errmsg("could not get language from locale \"%s\": %s",
2613+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
2614+
errmsg("could not get language from locale \"%s\": %s",
26132615
loc_str, u_errorName(status))));
26142616
}
26152617

@@ -2630,7 +2632,8 @@ pg_ucol_open(const char *loc_str)
26302632
if (U_FAILURE(status))
26312633
ereport(ERROR,
26322634
/* use original string for error report */
2633-
(errmsg("could not open collator for locale \"%s\": %s",
2635+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
2636+
errmsg("could not open collator for locale \"%s\": %s",
26342637
orig_str, u_errorName(status))));
26352638

26362639
if (U_ICU_VERSION_MAJOR_NUM < 54)
@@ -2646,7 +2649,8 @@ pg_ucol_open(const char *loc_str)
26462649
{
26472650
ucol_close(collator);
26482651
ereport(ERROR,
2649-
(errmsg("could not open collator for locale \"%s\": %s",
2652+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
2653+
errmsg("could not open collator for locale \"%s\": %s",
26502654
orig_str, u_errorName(status))));
26512655
}
26522656
}
@@ -2957,7 +2961,8 @@ icu_language_tag(const char *loc_str, int elevel)
29572961

29582962
if (elevel > 0)
29592963
ereport(elevel,
2960-
(errmsg("could not convert locale name \"%s\" to language tag: %s",
2964+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
2965+
errmsg("could not convert locale name \"%s\" to language tag: %s",
29612966
loc_str, u_errorName(status))));
29622967
return NULL;
29632968
}
@@ -2998,7 +3003,8 @@ icu_validate_locale(const char *loc_str)
29983003
if (U_FAILURE(status) || status == U_STRING_NOT_TERMINATED_WARNING)
29993004
{
30003005
ereport(elevel,
3001-
(errmsg("could not get language from ICU locale \"%s\": %s",
3006+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
3007+
errmsg("could not get language from ICU locale \"%s\": %s",
30023008
loc_str, u_errorName(status)),
30033009
errhint("To disable ICU locale validation, set the parameter \"%s\" to \"%s\".",
30043010
"icu_validation_level", "disabled")));
@@ -3027,7 +3033,8 @@ icu_validate_locale(const char *loc_str)
30273033

30283034
if (!found)
30293035
ereport(elevel,
3030-
(errmsg("ICU locale \"%s\" has unknown language \"%s\"",
3036+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
3037+
errmsg("ICU locale \"%s\" has unknown language \"%s\"",
30313038
loc_str, lang),
30323039
errhint("To disable ICU locale validation, set the parameter \"%s\" to \"%s\".",
30333040
"icu_validation_level", "disabled")));

‎src/backend/utils/adt/tid.c

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -312,9 +312,11 @@ currtid_internal(Relation rel, ItemPointer tid)
312312
return currtid_for_view(rel, tid);
313313

314314
if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind))
315-
elog(ERROR, "cannot look at latest visible tid for relation \"%s.%s\"",
316-
get_namespace_name(RelationGetNamespace(rel)),
317-
RelationGetRelationName(rel));
315+
ereport(ERROR,
316+
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
317+
errmsg("cannot look at latest visible tid for relation \"%s.%s\"",
318+
get_namespace_name(RelationGetNamespace(rel)),
319+
RelationGetRelationName(rel)));
318320

319321
ItemPointerCopy(tid, result);
320322

@@ -349,16 +351,22 @@ currtid_for_view(Relation viewrel, ItemPointer tid)
349351
if (strcmp(NameStr(attr->attname), "ctid") == 0)
350352
{
351353
if (attr->atttypid != TIDOID)
352-
elog(ERROR, "ctid isn't of type TID");
354+
ereport(ERROR,
355+
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
356+
errmsg("ctid isn't of type TID"));
353357
tididx = i;
354358
break;
355359
}
356360
}
357361
if (tididx < 0)
358-
elog(ERROR, "currtid cannot handle views with no CTID");
362+
ereport(ERROR,
363+
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
364+
errmsg("currtid cannot handle views with no CTID"));
359365
rulelock = viewrel->rd_rules;
360366
if (!rulelock)
361-
elog(ERROR, "the view has no rules");
367+
ereport(ERROR,
368+
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
369+
errmsg("the view has no rules"));
362370
for (i = 0; i < rulelock->numLocks; i++)
363371
{
364372
rewrite = rulelock->rules[i];
@@ -368,7 +376,9 @@ currtid_for_view(Relation viewrel, ItemPointer tid)
368376
TargetEntry *tle;
369377

370378
if (list_length(rewrite->actions) != 1)
371-
elog(ERROR, "only one select rule is allowed in views");
379+
ereport(ERROR,
380+
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
381+
errmsg("only one select rule is allowed in views"));
372382
query = (Query *) linitial(rewrite->actions);
373383
tle = get_tle_by_resno(query->targetList, tididx + 1);
374384
if (tle && tle->expr && IsA(tle->expr, Var))

0 commit comments

Comments
 (0)