Re: SQL:2011 application time

From: Paul Jungwirth <pj(at)illuminatedcomputing(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: SQL:2011 application time
Date: 2024-05-13 00:51:41
Message-ID: 2dacea9e-d1fe-4d4d-be5e-ac5c504f776f@illuminatedcomputing.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 5/5/24 20:01, jian he wrote:
> hi.
> I hope I understand the problem correctly.
> my understanding is that we are trying to solve a corner case:
> create table t(a int4range, b int4range, primary key(a, b WITHOUT OVERLAPS));
> insert into t values ('[1,2]','empty'), ('[1,2]','empty');
>
>
> I think the entry point is ATAddCheckNNConstraint and index_create.
> in a chain of DDL commands, you cannot be sure which one
> (primary key constraint or check constraint) is being created first,
> you just want to make sure that after both constraints are created,
> then add a dependency between primary key and check constraint.
>
> so you need to validate at different functions
> (ATAddCheckNNConstraint, index_create)
> that these two constraints are indeed created,
> only after that we have a dependency linking these two constraints.
>
>
> I've attached a patch trying to solve this problem.
> the patch is not totally polished, but works as expected, and also has
> lots of comments.

Thanks for this! I've incorporated it into the CHECK constraint patch with some changes. In
particular I thought index_create was a strange place to change the conperiod value of a
pg_constraint record, and it is not actually needed if we are copying that value correctly.

Some other comments on the patch file:

> N.B. we also need to have special care for case
> where check constraint was readded, e.g. ALTER TYPE.
> if ALTER TYPE is altering the PERIOD column of the primary key,
> alter column of primary key makes the index recreate, check constraint recreate,
> however, former interally also including add a check constraint.
> so we need to take care of merging two check constraint.

This is a good point. I've included tests for this based on your patch.

> N.B. the check constraint name is hard-wired, so if you create the constraint
> with the same name, PERIOD primary key cannot be created.

Yes, it may be worth doing something like other auto-named constraints and trying to avoid
duplicates. I haven't taken that on yet; I'm curious what others have to say about it.

> N.B. what about UNIQUE constraint?

See my previous posts on this thread about allowing 'empty' in UNIQUE constraints.

> N.B. seems ok to not care about FOREIGN KEY regarding this corner case?

Agreed.

v3 patches attached, rebased to 3ca43dbbb6.

Yours,

--
Paul ~{:-)
pj(at)illuminatedcomputing(dot)com

Attachment Content-Type Size
v3-0001-Don-t-treat-WITHOUT-OVERLAPS-indexes-as-unique-in.patch text/x-patch 3.9 KB
v3-0002-Add-CHECK-NOT-isempty-constraint-to-PRIMARY-KEYs-.patch text/x-patch 74.6 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2024-05-13 01:42:20 Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.
Previous Message Noah Misch 2024-05-12 23:29:23 Re: race condition in pg_class