Horton

Thread

Commits

  1. Add missing CommandCounterIncrement

  2. Fix instability in recently added regression tests

  3. Allow NOT VALID foreign key constraints on partitioned tables

  4. Split ATExecValidateConstraint into reusable pieces

  1. Allow NOT VALID foreign key constraints on partitioned tables.

    Amul Sul <sulamul@gmail.com> — 2025-01-02T16:49:13Z

    Hi,

While working on NOT ENFORCED constraints[1], which are by default marked as NOT
VALID, I encountered an error when adding a NOT ENFORCED foreign key (FK)
constraint to a partitioned table [2]. Alvaro also confirmed off-list that NOT
VALID FK constraints have not yet been implemented. This patch addresses that
gap.

When adding a new FK constraint or attaching a partitioned table, where
matching FK constraints are merged, we allow the parent constraint to be NOT
VALID while the child constraint remains VALID, which is harmless. However, the
reverse scenario -- where the parent constraint is VALID and the child is NOT
VALID -- is incorrect. To address this, when merging a NOT VALID FK constraint
from the child with a VALID parent constraint, it implicitly validates the
child constraint against its existing data and marks it as VALID. This behavior
aligns with adding a new FK constraint directly to the child table, which would
also validate the existing data.

The 0001 patch focuses on code refactoring and does not modify or introduce new
behaviors. It splits ATExecValidateConstraint() into two separate functions for
handling FK and CHECK constraints. For this feature, I wanted to reuse the FK
validation logic and make it recursive for partitioned tables, necessitating
its separation. Although CHECK constraint validation isn't required for this
work, separating it simplifies ATExecValidateConstraint() and prepares the
codebase for adding support for other constraint types (e.g., NOT NULL) in the
future. Additional changes in the refactoring include renaming the variable
tuple to contuple within these functions, duplicating a few lines of code that
update pg_constraint.convalidated, and running pgindent, which rearranged the
code and comments. I hope the duplication is not a significant concern.

Please review the attached patches. Any comments or suggestions would
be greatly appreciated. Thank you!

1] https://postgr.es/m/CAAJ_b962c5AcYW9KUt_R_ER5qs3fUGbe4az-SP-vuwPS-w-AGA@mail.gmail.com
2] https://postgr.es/m/CAAJ_b94+0-YFj4LopVqz_+c7ckkUYa77G_5rgTJVnUyepuhmrA@mail.gmail.com

--
Regards,
Amul Sul
EDB: http://www.enterprisedb.com

  2. Re: Allow NOT VALID foreign key constraints on partitioned tables.

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-01-02T18:40:25Z

    
On Thu, Jan 2, 2025, at 5:49 PM, Amul Sul wrote:
> When adding a new FK constraint or attaching a partitioned table, where
> matching FK constraints are merged, we allow the parent constraint to be NOT
> VALID while the child constraint remains VALID, which is harmless. However, the
> reverse scenario -- where the parent constraint is VALID and the child is NOT
> VALID -- is incorrect. To address this, when merging a NOT VALID FK constraint
> from the child with a VALID parent constraint, it implicitly validates the
> child constraint against its existing data and marks it as VALID. This behavior
> aligns with adding a new FK constraint directly to the child table, which would
> also validate the existing data.

Hmm, I'm not sure about this, which may cause surprising delays. Maybe it would be better that the operation fails with an error, so that the user can do VALIDATE CONSTRAINT explicitly and retry the ATTACH once all the partitions have been so processed.

  3. Re: Allow NOT VALID foreign key constraints on partitioned tables.

    Amul Sul <sulamul@gmail.com> — 2025-01-06T04:23:46Z

    On Fri, Jan 3, 2025 at 12:11 AM Álvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
>
>
> On Thu, Jan 2, 2025, at 5:49 PM, Amul Sul wrote:
>
> When adding a new FK constraint or attaching a partitioned table, where
> matching FK constraints are merged, we allow the parent constraint to be NOT
> VALID while the child constraint remains VALID, which is harmless. However, the
> reverse scenario -- where the parent constraint is VALID and the child is NOT
> VALID -- is incorrect. To address this, when merging a NOT VALID FK constraint
> from the child with a VALID parent constraint, it implicitly validates the
> child constraint against its existing data and marks it as VALID. This behavior
> aligns with adding a new FK constraint directly to the child table, which would
> also validate the existing data.
>
>
> Hmm, I'm not sure about this, which may cause surprising delays. Maybe it would be better that the operation fails with an error, so that the user can do VALIDATE CONSTRAINT explicitly and retry the ATTACH once all the partitions have been so processed.

Error reporting would have made this straightforward, but the delay is
not a new, and the patch does not introduce any additional delay.
Setting the patch aside for a moment, consider the current behavior on
the master branch: if you have a partitioned table(see e.g. below)
where one of the child tables has a NOT VALID foreign key constraint,
adding a new VALID foreign key constraint to the partitioned table
will ignore the existing constraint on the child table. Instead, it
creates a new constraint, which ultimately triggers a scan of the
child table to validate the new constraint. E.g.

create table bar(i int primary key);
create table foo(i int) partition by range(i);
create table foo_p1 partition of foo for values from (0) to (10);
insert into foo_p1 values(1); -- value doesn't exists in bar
alter table foo_p1 add constraint fk_con foreign key(i) references bar
NOT VALID; -- ok

-- following triggers the validation and fails.
alter table foo add constraint fk_con foreign key(i) references bar;

The behavior with the patch remains the same, but instead of creating
a new foreign key constraint, it merges with the existing one and
validates it.

Regards,
Amul

  4. Re: Allow NOT VALID foreign key constraints on partitioned tables.

    Amul Sul <sulamul@gmail.com> — 2025-01-15T05:34:51Z

    On Mon, Jan 6, 2025 at 9:53 AM Amul Sul <sulamul@gmail.com> wrote:
>

I made the minor changes to the attached version and rebased it
against the latest master(9a45a89c38f).

Regards,
Amul

  5. Re: Allow NOT VALID foreign key constraints on partitioned tables.

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-01-16T15:48:04Z

    On 2025-Jan-15, Amul Sul wrote:

> I made the minor changes to the attached version and rebased it
> against the latest master(9a45a89c38f).

Pushed 0001, thanks.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/

  6. Re: Allow NOT VALID foreign key constraints on partitioned tables.

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-01-21T21:06:14Z

    Suppose I have a hierarchy like this

      parent
        |
      child
       /\
      /  \
     /    grandchild2
    /
  grandchild1

and I have a validated constraint on grandchild1 and an invalid
constraint on child.  What happens if I add a constraint on parent?  In
my understanding, it should not attempt to revalidate the constraint on
grandchild1, because it's known valid; but I don't think I see code that
would skip validation there.  That is, QueueFKConstraintValidation does
its thing unconditionally (esp. recursing to children), which seems
wrong.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"I am amazed at [the pgsql-sql] mailing list for the wonderful support, and
lack of hesitasion in answering a lost soul's question, I just wished the rest
of the mailing list could be like this."                               (Fotis)
              https://postgr.es/m/200606261359.k5QDxE2p004593@auth-smtp.hol.gr

  7. Re: Allow NOT VALID foreign key constraints on partitioned tables.

    Amul Sul <sulamul@gmail.com> — 2025-01-22T06:12:15Z

    On Wed, Jan 22, 2025 at 2:36 AM Álvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
>
> Suppose I have a hierarchy like this
>
>       parent
>         |
>       child
>        /\
>       /  \
>      /    grandchild2
>     /
>   grandchild1
>
> and I have a validated constraint on grandchild1 and an invalid
> constraint on child.  What happens if I add a constraint on parent?  In
> my understanding, it should not attempt to revalidate the constraint on
> grandchild1, because it's known valid; but I don't think I see code that
> would skip validation there.  That is, QueueFKConstraintValidation does
> its thing unconditionally (esp. recursing to children), which seems
> wrong.
>

You’re correct; it’s fixed in the attached version, along with an
assert(!convalidated) in QueueFKConstraintValidation(), and I’ve
included tests to cover the change. Thanks for reviewing this patch.

Regards,
Amul

  8. Re: Allow NOT VALID foreign key constraints on partitioned tables.

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-01-23T15:04:32Z

    On 2025-Jan-22, Amul Sul wrote:

> You’re correct; it’s fixed in the attached version, along with an
> assert(!convalidated) in QueueFKConstraintValidation(), and I’ve
> included tests to cover the change. Thanks for reviewing this patch.

OK thanks, looks good, I have pushed it now with some trivial
amendments.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Java is clearly an example of money oriented programming"  (A. Stepanov)

  9. Re: Allow NOT VALID foreign key constraints on partitioned tables.

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-24T02:16:46Z

    =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@alvh.no-ip.org> writes:
> OK thanks, looks good, I have pushed it now with some trivial
> amendments.

Looks like some of the queries need ORDER BY for stability.

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=morepork&dt=2025-01-23%2023%3A35%3A57

			regards, tom lane

  10. Re: Allow NOT VALID foreign key constraints on partitioned tables.

    Amul Sul <sulamul@gmail.com> — 2025-01-24T10:50:00Z

    On Fri, Jan 24, 2025 at 7:46 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@alvh.no-ip.org> writes:
> > OK thanks, looks good, I have pushed it now with some trivial
> > amendments.
>
> Looks like some of the queries need ORDER BY for stability.
>
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=morepork&dt=2025-01-23%2023%3A35%3A57
>

Attached is the patch to fix this. Apologies for the sloppy work, and
thanks for pointing it out.

Regards,
Amul

  11. Re: Allow NOT VALID foreign key constraints on partitioned tables.

    Alexander Law <exclusion@gmail.com> — 2025-01-25T05:00:01Z

    Hello Álvaro,

23.01.2025 17:04, Álvaro Herrera wrote:
> OK thanks, looks good, I have pushed it now with some trivial
> amendments.

Please look at the script that produces an error starting from b663b9436:
CREATE TABLE st (a int, primary key (a));
CREATE TABLE pt (a int,
   FOREIGN KEY (a) REFERENCES st ON DELETE SET NULL ON UPDATE SET NULL,
   FOREIGN KEY (a) REFERENCES st ON DELETE SET NULL ON UPDATE SET NULL
) PARTITION BY LIST (a);
CREATE TABLE tp1 PARTITION OF pt FOR VALUES IN (1, 2);
ALTER TABLE pt DETACH PARTITION tp1;
ALTER TABLE pt ATTACH PARTITION tp1 FOR VALUES IN (1, 2);

ERROR:  XX000: tuple already updated by self
LOCATION:  simple_heap_update, heapam.c:4374

Best regards,
Alexander Lakhin
Neon (https://neon.tech)

  12. Re: Allow NOT VALID foreign key constraints on partitioned tables.

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-01-25T10:29:13Z

    On Sat, Jan 25, 2025, at 6:00 AM, Alexander Lakhin wrote:
> Hello Álvaro,
> 
> Please look at the script that produces an error starting from b663b9436:

Ah yes, this is my bug: I moved a CCI where it became conditional. Will fix, thanks for the test case.

  13. Re: Allow NOT VALID foreign key constraints on partitioned tables.

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-01-26T16:38:30Z

    On 2025-Jan-25, Álvaro Herrera wrote:

> On Sat, Jan 25, 2025, at 6:00 AM, Alexander Lakhin wrote:
> > Hello Álvaro,
> > 
> > Please look at the script that produces an error starting from b663b9436:
> 
> Ah yes, this is my bug: I moved a CCI where it became conditional.
> Will fix, thanks for the test case.

Pushed the fix, thanks.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Hay quien adquiere la mala costumbre de ser infeliz" (M. A. Evans)

  14. Re: Allow NOT VALID foreign key constraints on partitioned tables.

    Amul Sul <sulamul@gmail.com> — 2025-01-28T03:39:18Z

    On Sun, Jan 26, 2025 at 10:08 PM Álvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2025-Jan-25, Álvaro Herrera wrote:
>
> > On Sat, Jan 25, 2025, at 6:00 AM, Alexander Lakhin wrote:
> > > Hello Álvaro,
> > >
> > > Please look at the script that produces an error starting from b663b9436:
> >
> > Ah yes, this is my bug: I moved a CCI where it became conditional.
> > Will fix, thanks for the test case.
>
> Pushed the fix, thanks.
>

Thank you !

Regards,
Amul