Re: tablecmds.c/MergeAttributes() cleanup

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: tablecmds.c/MergeAttributes() cleanup
Date: 2023-09-19 13:11:51
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 29.08.23 13:20, Alvaro Herrera wrote:
> On 2023-Aug-29, Peter Eisentraut wrote:
>> @@ -3278,13 +3261,16 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
>> *
>> * constraints is a list of CookedConstraint structs for previous constraints.
>> *
>> - * Returns true if merged (constraint is a duplicate), or false if it's
>> - * got a so-far-unique name, or throws error if conflict.
>> + * If the constraint is a duplicate, then the existing constraint's
>> + * inheritance count is updated. If the constraint doesn't match or conflict
>> + * with an existing one, a new constraint is appended to the list. If there
>> + * is a conflict (same name but different expression), throw an error.
> This wording confused me:
> "If the constraint doesn't match or conflict with an existing one, a new
> constraint is appended to the list."
> I first read it as "doesn't match or conflicts with ..." (i.e., the
> negation only applied to the first verb, not both) which would have been
> surprising (== broken) behavior.
> I think it's clearer if you say "doesn't match nor conflict", but I'm
> not sure if this is grammatically correct.

Here is an updated version of this patch set. I resolved some conflicts
and addressed this comment of yours. I also dropped the one patch with
some catalog header edits that people didn't seem to particularly like.

The patches that are now 0001 through 0004 were previously reviewed and
just held for the not-null constraint patches, I think, so I'll commit
them in a few days if there are no objections.

Patches 0005 through 0007 are also ready in my opinion, but they haven't
really been reviewed, so this would be something for reviewers to focus
on. (0005 and 0007 are trivial, but they go to together with 0006.)

The remaining 0008 and 0009 were still under discussion and contemplation.

Attachment Content-Type Size
v3-0001-Clean-up-MergeAttributesIntoExisting.patch text/plain 12.4 KB
v3-0002-Clean-up-MergeCheckConstraint.patch text/plain 3.9 KB
v3-0003-MergeAttributes-and-related-variable-renaming.patch text/plain 15.2 KB
v3-0004-Add-TupleDescGetDefault.patch text/plain 5.0 KB
v3-0005-Push-attidentity-and-attgenerated-handling-into-B.patch text/plain 1.6 KB
v3-0006-Move-BuildDescForRelation-from-tupdesc.c-to-table.patch text/plain 8.7 KB
v3-0007-Push-attcompression-and-attstorage-handling-into-.patch text/plain 1.8 KB
v3-0008-Refactor-ATExecAddColumn-to-use-BuildDescForRelat.patch text/plain 7.1 KB
v3-0009-MergeAttributes-convert-pg_attribute-back-to-Colu.patch text/plain 16.4 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Matthias van de Meent 2023-09-19 13:28:26 Re: Improving btree performance through specializing by key shape, take 2
Previous Message Amit Langote 2023-09-19 12:51:39 Re: remaining sql/json patches