Skip to content

Commit 09cecdf

Browse files
tglsfdctglsfdc
committed
Fix a number of places that produced XX000 errors in the regression tests.
It's against project policy to use elog() for user-facing errors, or to omit an errcode() selection for errors that aren't supposed to be "can't happen" cases. Fix all the violations of this policy that result in ERRCODE_INTERNAL_ERROR log entries during the standard regression tests, as errors that can reliably be triggered from SQL surely should be considered user-facing. I also looked through all the files touched by this commit and fixed other nearby problems of the same ilk. I do not claim to have fixed all violations of the policy, just the ones in these files. In a few places I also changed existing ERRCODE choices that didn't seem particularly appropriate; mainly replacing ERRCODE_SYNTAX_ERROR by something more specific. Back-patch to 9.5, but no further; changing ERRCODE assignments in stable branches doesn't seem like a good idea.
1 parent 690ed2b commit 09cecdf

File tree

13 files changed

+181
-93
lines changed

13 files changed

+181
-93
lines changed

‎contrib/tablefunc/tablefunc.c

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,9 @@ crosstab(PG_FUNCTION_ARGS)
432432
break;
433433
default:
434434
/* result type isn't composite */
435-
elog(ERROR, "return type must be a row type");
435+
ereport(ERROR,
436+
(errcode(ERRCODE_DATATYPE_MISMATCH),
437+
errmsg("return type must be a row type")));
436438
break;
437439
}
438440

@@ -1350,7 +1352,9 @@ build_tuplestore_recursively(char *key_fld,
13501352
appendStringInfo(&chk_current_key, "%s%s%s",
13511353
branch_delim, current_key, branch_delim);
13521354
if (strstr(chk_branchstr.data, chk_current_key.data))
1353-
elog(ERROR, "infinite recursion detected");
1355+
ereport(ERROR,
1356+
(errcode(ERRCODE_INVALID_RECURSION),
1357+
errmsg("infinite recursion detected")));
13541358
}
13551359

13561360
/* OK, extend the branch */
@@ -1429,7 +1433,7 @@ validateConnectbyTupleDesc(TupleDesc tupdesc, bool show_branch, bool show_serial
14291433
{
14301434
if (tupdesc->natts != (CONNECTBY_NCOLS + serial_column))
14311435
ereport(ERROR,
1432-
(errcode(ERRCODE_SYNTAX_ERROR),
1436+
(errcode(ERRCODE_DATATYPE_MISMATCH),
14331437
errmsg("invalid return type"),
14341438
errdetail("Query-specified return tuple has " \
14351439
"wrong number of columns.")));
@@ -1438,7 +1442,7 @@ validateConnectbyTupleDesc(TupleDesc tupdesc, bool show_branch, bool show_serial
14381442
{
14391443
if (tupdesc->natts != CONNECTBY_NCOLS_NOBRANCH + serial_column)
14401444
ereport(ERROR,
1441-
(errcode(ERRCODE_SYNTAX_ERROR),
1445+
(errcode(ERRCODE_DATATYPE_MISMATCH),
14421446
errmsg("invalid return type"),
14431447
errdetail("Query-specified return tuple has " \
14441448
"wrong number of columns.")));
@@ -1447,35 +1451,41 @@ validateConnectbyTupleDesc(TupleDesc tupdesc, bool show_branch, bool show_serial
14471451
/* check that the types of the first two columns match */
14481452
if (tupdesc->attrs[0]->atttypid != tupdesc->attrs[1]->atttypid)
14491453
ereport(ERROR,
1450-
(errcode(ERRCODE_SYNTAX_ERROR),
1454+
(errcode(ERRCODE_DATATYPE_MISMATCH),
14511455
errmsg("invalid return type"),
14521456
errdetail("First two columns must be the same type.")));
14531457

14541458
/* check that the type of the third column is INT4 */
14551459
if (tupdesc->attrs[2]->atttypid != INT4OID)
14561460
ereport(ERROR,
1457-
(errcode(ERRCODE_SYNTAX_ERROR),
1461+
(errcode(ERRCODE_DATATYPE_MISMATCH),
14581462
errmsg("invalid return type"),
14591463
errdetail("Third column must be type %s.",
14601464
format_type_be(INT4OID))));
14611465

14621466
/* check that the type of the fourth column is TEXT if applicable */
14631467
if (show_branch && tupdesc->attrs[3]->atttypid != TEXTOID)
14641468
ereport(ERROR,
1465-
(errcode(ERRCODE_SYNTAX_ERROR),
1469+
(errcode(ERRCODE_DATATYPE_MISMATCH),
14661470
errmsg("invalid return type"),
14671471
errdetail("Fourth column must be type %s.",
14681472
format_type_be(TEXTOID))));
14691473

14701474
/* check that the type of the fifth column is INT4 */
14711475
if (show_branch && show_serial && tupdesc->attrs[4]->atttypid != INT4OID)
1472-
elog(ERROR, "query-specified return tuple not valid for Connectby: "
1473-
"fifth column must be type %s", format_type_be(INT4OID));
1476+
ereport(ERROR,
1477+
(errcode(ERRCODE_DATATYPE_MISMATCH),
1478+
errmsg("query-specified return tuple not valid for Connectby: "
1479+
"fifth column must be type %s",
1480+
format_type_be(INT4OID))));
14741481

14751482
/* check that the type of the fifth column is INT4 */
14761483
if (!show_branch && show_serial && tupdesc->attrs[3]->atttypid != INT4OID)
1477-
elog(ERROR, "query-specified return tuple not valid for Connectby: "
1478-
"fourth column must be type %s", format_type_be(INT4OID));
1484+
ereport(ERROR,
1485+
(errcode(ERRCODE_DATATYPE_MISMATCH),
1486+
errmsg("query-specified return tuple not valid for Connectby: "
1487+
"fourth column must be type %s",
1488+
format_type_be(INT4OID))));
14791489

14801490
/* OK, the tupdesc is valid for our purposes */
14811491
}
@@ -1496,7 +1506,7 @@ compatConnectbyTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc)
14961506
*/
14971507
if (sql_tupdesc->natts < 2)
14981508
ereport(ERROR,
1499-
(errcode(ERRCODE_SYNTAX_ERROR),
1509+
(errcode(ERRCODE_DATATYPE_MISMATCH),
15001510
errmsg("invalid return type"),
15011511
errdetail("Query must return at least two columns.")));
15021512

@@ -1511,7 +1521,7 @@ compatConnectbyTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc)
15111521
if (ret_atttypid != sql_atttypid ||
15121522
(ret_atttypmod >= 0 && ret_atttypmod != sql_atttypmod))
15131523
ereport(ERROR,
1514-
(errcode(ERRCODE_SYNTAX_ERROR),
1524+
(errcode(ERRCODE_DATATYPE_MISMATCH),
15151525
errmsg("invalid return type"),
15161526
errdetail("SQL key field type %s does " \
15171527
"not match return key field type %s.",
@@ -1525,7 +1535,7 @@ compatConnectbyTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc)
15251535
if (ret_atttypid != sql_atttypid ||
15261536
(ret_atttypmod >= 0 && ret_atttypmod != sql_atttypmod))
15271537
ereport(ERROR,
1528-
(errcode(ERRCODE_SYNTAX_ERROR),
1538+
(errcode(ERRCODE_DATATYPE_MISMATCH),
15291539
errmsg("invalid return type"),
15301540
errdetail("SQL parent key field type %s does " \
15311541
"not match return parent key field type %s.",
@@ -1556,7 +1566,7 @@ compatCrosstabTupleDescs(TupleDesc ret_tupdesc, TupleDesc sql_tupdesc)
15561566
sql_atttypid = sql_tupdesc->attrs[0]->atttypid;
15571567
if (ret_atttypid != sql_atttypid)
15581568
ereport(ERROR,
1559-
(errcode(ERRCODE_SYNTAX_ERROR),
1569+
(errcode(ERRCODE_DATATYPE_MISMATCH),
15601570
errmsg("invalid return type"),
15611571
errdetail("SQL rowid datatype does not match " \
15621572
"return rowid datatype.")));

‎src/backend/access/common/reloptions.c

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ allocate_reloption(bits32 kinds, int type, char *name, char *desc)
484484
size = sizeof(relopt_string);
485485
break;
486486
default:
487-
elog(ERROR, "unsupported option type");
487+
elog(ERROR, "unsupported reloption type %d", type);
488488
return NULL; /* keep compiler quiet */
489489
}
490490

@@ -1016,7 +1016,8 @@ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
10161016
parsed = parse_bool(value, &option->values.bool_val);
10171017
if (validate && !parsed)
10181018
ereport(ERROR,
1019-
(errmsg("invalid value for boolean option \"%s\": %s",
1019+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
1020+
errmsg("invalid value for boolean option \"%s\": %s",
10201021
option->gen->name, value)));
10211022
}
10221023
break;
@@ -1027,12 +1028,14 @@ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
10271028
parsed = parse_int(value, &option->values.int_val, 0, NULL);
10281029
if (validate && !parsed)
10291030
ereport(ERROR,
1030-
(errmsg("invalid value for integer option \"%s\": %s",
1031+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
1032+
errmsg("invalid value for integer option \"%s\": %s",
10311033
option->gen->name, value)));
10321034
if (validate && (option->values.int_val < optint->min ||
10331035
option->values.int_val > optint->max))
10341036
ereport(ERROR,
1035-
(errmsg("value %s out of bounds for option \"%s\"",
1037+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
1038+
errmsg("value %s out of bounds for option \"%s\"",
10361039
value, option->gen->name),
10371040
errdetail("Valid values are between \"%d\" and \"%d\".",
10381041
optint->min, optint->max)));
@@ -1045,12 +1048,14 @@ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
10451048
parsed = parse_real(value, &option->values.real_val);
10461049
if (validate && !parsed)
10471050
ereport(ERROR,
1048-
(errmsg("invalid value for floating point option \"%s\": %s",
1051+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
1052+
errmsg("invalid value for floating point option \"%s\": %s",
10491053
option->gen->name, value)));
10501054
if (validate && (option->values.real_val < optreal->min ||
10511055
option->values.real_val > optreal->max))
10521056
ereport(ERROR,
1053-
(errmsg("value %s out of bounds for option \"%s\"",
1057+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
1058+
errmsg("value %s out of bounds for option \"%s\"",
10541059
value, option->gen->name),
10551060
errdetail("Valid values are between \"%f\" and \"%f\".",
10561061
optreal->min, optreal->max)));
@@ -1168,7 +1173,7 @@ fillRelOptions(void *rdopts, Size basesize,
11681173
}
11691174
break;
11701175
default:
1171-
elog(ERROR, "unrecognized reloption type %c",
1176+
elog(ERROR, "unsupported reloption type %d",
11721177
options[i].gen->type);
11731178
break;
11741179
}

‎src/backend/access/heap/heapam.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2801,7 +2801,9 @@ heap_delete(Relation relation, ItemPointer tid,
28012801
if (result == HeapTupleInvisible)
28022802
{
28032803
UnlockReleaseBuffer(buffer);
2804-
elog(ERROR, "attempted to delete invisible tuple");
2804+
ereport(ERROR,
2805+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
2806+
errmsg("attempted to delete invisible tuple")));
28052807
}
28062808
else if (result == HeapTupleBeingUpdated && wait)
28072809
{
@@ -3343,7 +3345,9 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
33433345
if (result == HeapTupleInvisible)
33443346
{
33453347
UnlockReleaseBuffer(buffer);
3346-
elog(ERROR, "attempted to update invisible tuple");
3348+
ereport(ERROR,
3349+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
3350+
errmsg("attempted to update invisible tuple")));
33473351
}
33483352
else if (result == HeapTupleBeingUpdated && wait)
33493353
{

‎src/backend/commands/copy.c

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,9 +1422,9 @@ BeginCopy(bool is_from,
14221422
* in any RLS clauses.
14231423
*
14241424
* When this happens, we are passed in the relid of the originally
1425-
* found relation (which we have locked). As the planner will look
1426-
* up the relation again, we double-check here to make sure it found
1427-
* the same one that we have locked.
1425+
* found relation (which we have locked). As the planner will look up
1426+
* the relation again, we double-check here to make sure it found the
1427+
* same one that we have locked.
14281428
*/
14291429
if (queryRelId != InvalidOid)
14301430
{
@@ -1603,10 +1603,12 @@ ClosePipeToProgram(CopyState cstate)
16031603
pclose_rc = ClosePipeStream(cstate->copy_file);
16041604
if (pclose_rc == -1)
16051605
ereport(ERROR,
1606-
(errmsg("could not close pipe to external command: %m")));
1606+
(errcode_for_file_access(),
1607+
errmsg("could not close pipe to external command: %m")));
16071608
else if (pclose_rc != 0)
16081609
ereport(ERROR,
1609-
(errmsg("program \"%s\" failed",
1610+
(errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION),
1611+
errmsg("program \"%s\" failed",
16101612
cstate->filename),
16111613
errdetail_internal("%s", wait_result_to_str(pclose_rc))));
16121614
}
@@ -1703,7 +1705,8 @@ BeginCopyTo(Relation rel,
17031705
cstate->copy_file = OpenPipeStream(cstate->filename, PG_BINARY_W);
17041706
if (cstate->copy_file == NULL)
17051707
ereport(ERROR,
1706-
(errmsg("could not execute command \"%s\": %m",
1708+
(errcode_for_file_access(),
1709+
errmsg("could not execute command \"%s\": %m",
17071710
cstate->filename)));
17081711
}
17091712
else
@@ -1730,7 +1733,10 @@ BeginCopyTo(Relation rel,
17301733
cstate->filename)));
17311734

17321735
if (fstat(fileno(cstate->copy_file), &st))
1733-
elog(ERROR, "could not stat file \"%s\": %m", cstate->filename);
1736+
ereport(ERROR,
1737+
(errcode_for_file_access(),
1738+
errmsg("could not stat file \"%s\": %m",
1739+
cstate->filename)));
17341740

17351741
if (S_ISDIR(st.st_mode))
17361742
ereport(ERROR,
@@ -2271,13 +2277,13 @@ CopyFrom(CopyState cstate)
22712277
{
22722278
if (!ThereAreNoPriorRegisteredSnapshots() || !ThereAreNoReadyPortals())
22732279
ereport(ERROR,
2274-
(ERRCODE_INVALID_TRANSACTION_STATE,
2280+
(errcode(ERRCODE_INVALID_TRANSACTION_STATE),
22752281
errmsg("cannot perform FREEZE because of prior transaction activity")));
22762282

22772283
if (cstate->rel->rd_createSubid != GetCurrentSubTransactionId() &&
22782284
cstate->rel->rd_newRelfilenodeSubid != GetCurrentSubTransactionId())
22792285
ereport(ERROR,
2280-
(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE,
2286+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
22812287
errmsg("cannot perform FREEZE because the table was not created or truncated in the current subtransaction")));
22822288

22832289
hi_options |= HEAP_INSERT_FROZEN;
@@ -2737,7 +2743,8 @@ BeginCopyFrom(Relation rel,
27372743
cstate->copy_file = OpenPipeStream(cstate->filename, PG_BINARY_R);
27382744
if (cstate->copy_file == NULL)
27392745
ereport(ERROR,
2740-
(errmsg("could not execute command \"%s\": %m",
2746+
(errcode_for_file_access(),
2747+
errmsg("could not execute command \"%s\": %m",
27412748
cstate->filename)));
27422749
}
27432750
else
@@ -2752,7 +2759,10 @@ BeginCopyFrom(Relation rel,
27522759
cstate->filename)));
27532760

27542761
if (fstat(fileno(cstate->copy_file), &st))
2755-
elog(ERROR, "could not stat file \"%s\": %m", cstate->filename);
2762+
ereport(ERROR,
2763+
(errcode_for_file_access(),
2764+
errmsg("could not stat file \"%s\": %m",
2765+
cstate->filename)));
27562766

27572767
if (S_ISDIR(st.st_mode))
27582768
ereport(ERROR,

‎src/backend/commands/vacuum.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,10 @@ vacuum(int options, RangeVar *relation, Oid relid, VacuumParams *params,
180180
* calls a hostile index expression that itself calls ANALYZE.
181181
*/
182182
if (in_vacuum)
183-
elog(ERROR, "%s cannot be executed from VACUUM or ANALYZE", stmttype);
183+
ereport(ERROR,
184+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
185+
errmsg("%s cannot be executed from VACUUM or ANALYZE",
186+
stmttype)));
184187

185188
/*
186189
* Send info about dead objects to the statistics collector, unless we are

‎src/backend/executor/execQual.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,8 @@ ExecEvalScalarVar(ExprState *exprstate, ExprContext *econtext,
631631
{
632632
if (variable->vartype != attr->atttypid)
633633
ereport(ERROR,
634-
(errmsg("attribute %d has wrong type", attnum),
634+
(errcode(ERRCODE_DATATYPE_MISMATCH),
635+
errmsg("attribute %d has wrong type", attnum),
635636
errdetail("Table has type %s, but query expects %s.",
636637
format_type_be(attr->atttypid),
637638
format_type_be(variable->vartype))));
@@ -4111,7 +4112,8 @@ ExecEvalFieldSelect(FieldSelectState *fstate,
41114112
/* As in ExecEvalScalarVar, we should but can't check typmod */
41124113
if (fselect->resulttype != attr->atttypid)
41134114
ereport(ERROR,
4114-
(errmsg("attribute %d has wrong type", fieldnum),
4115+
(errcode(ERRCODE_DATATYPE_MISMATCH),
4116+
errmsg("attribute %d has wrong type", fieldnum),
41154117
errdetail("Table has type %s, but query expects %s.",
41164118
format_type_be(attr->atttypid),
41174119
format_type_be(fselect->resulttype))));

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,11 @@ parse_snapshot(const char *str)
334334
return buf_finalize(buf);
335335

336336
bad_format:
337-
elog(ERROR, "invalid input for txid_snapshot: \"%s\"", str_start);
338-
return NULL;
337+
ereport(ERROR,
338+
(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
339+
errmsg("invalid input syntax for type txid_snapshot: \"%s\"",
340+
str_start)));
341+
return NULL; /* keep compiler quiet */
339342
}
340343

341344
/*
@@ -526,8 +529,10 @@ txid_snapshot_recv(PG_FUNCTION_ARGS)
526529
PG_RETURN_POINTER(snap);
527530

528531
bad_format:
529-
elog(ERROR, "invalid snapshot data");
530-
return (Datum) NULL;
532+
ereport(ERROR,
533+
(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
534+
errmsg("invalid external txid_snapshot data")));
535+
PG_RETURN_POINTER(NULL); /* keep compiler quiet */
531536
}
532537

533538
/*

0 commit comments

Comments
 (0)