Re: Internal error codes triggered by regression tests and user queries, take 2 - Mailing list pgsql-hackers

From Alexander Lakhin
Subject Re: Internal error codes triggered by regression tests and user queries, take 2
Date
Msg-id 7a9b3652-134c-4565-82ad-839035a3a382@gmail.com
Whole thread
Responses Re: pg_plan_advice: fix parsing underscore in numbers
Re: Internal error codes triggered by regression tests and user queries, take 2
List pgsql-hackers
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
Attachment

pgsql-hackers by date:

Previous
From: Mark Wong
Date:
Subject: Re: updates for handling optional argument in system functions
Next
From: Denis Smirnov
Date:
Subject: Re: [PATCH] libpq: Add PQpassfileLookup()