Re: Internal error codes triggered by regression tests and user queries, take 2

Hello Tom,

07.09.2026 00:00, Alexander Lakhin wrote:
>
> 06.09.2026 22:07, Tom Lane wrote:
>
>> @@ -794,8 +794,10 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
>> if (constraint->conname &&
>> notnull_constraint->conname &&
>> strcmp(notnull_constraint->conname, constraint->conname) != 0)
>> - elog(ERROR, "conflicting not-null constraint names \"%s\" and \"%s\"",
>> - notnull_constraint->conname, constraint->conname);
>> + ereport(ERROR,
>> + errcode(ERRCODE_SYNTAX_ERROR),
>> + errmsg("conflicting not-null constraint names \"%s\" and \"%s\"",
>> + notnull_constraint->conname, constraint->conname));
>>
>> This does not seem like a "syntax error". Perhaps
>> ERRCODE_INVALID_OBJECT_DEFINITION would serve?
>
> Yeah, I considered this, but the same errmsg in
> AddRelationNotNullConstraints() uses ERRCODE_SYNTAX_ERROR. I found it
> more consistent to emit the same code. Maybe it would make do the opposite
> -- change the errcode in AddRelationNotNullConstraints() (there are two
> instances there, though)... Moreover, ERRCODE_SYNTAX_ERROR is chosen for
> many other similar messages in transformColumnDefinition(), so I'm not
> sure if it makes sense to change all of those or just report "syntax error"
> for consistency while keeping the patch focused.

I've changed that occurrence to ERRCODE_INVALID_OBJECT_DEFINITION, but as
I mentioned above, transformColumnDefinition() has many other SYNTAX_ERRORs
(originating from a56ff9a0b), besides AddRelationNotNullConstraints().
Please find attached a patch to correct these too, for consistency, if you
find this appropriate. There are other similar errors in parse_utilcmd.c,
but I'd stop here unless we want to start a new campaign not to define,
but correct error codes according to some convention.

>> We'd be well advised to make those two messages more consistent,
>> whether they're worth translating or not. Also I guess a sweep
>> for other "unreachable" errors in typmodout functions might be
>> useful

Changed three occurrences to use the same errmsg and errcode.

Please look at the attached v3-Define-...patch, where I think I addressed
all of your comments.

Thank you for your time!

Best regards,
Alexander