Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Avoid bogus scans of partitions when marking FKs enforced

  2. Avoid bogus scans of partitions when validating FKs to partitioned tables

  3. Allow NOT VALID foreign key constraints on partitioned tables

  1. Foreign key validation failure in 18beta1

    Antonin Houska <ah@cybertec.at> — 2025-05-28T07:51:15Z

    I've come across an unexpected ERROR during validation of FK constraint in PG
    18beta1. The same works in PG 17:
    
    drop table if exists fk;
    drop table if exists pk;
    create table pk(i int primary key) partition by range (i);
    create table pk_1 partition of pk for values from (0) to (1);
    create table pk_2 partition of pk for values from (1) to (2);
    insert into pk values (0), (1);
    create table fk(i int);
    insert into fk values (1);
    
    -- Works if the FK constraint is created as valid.
    --alter table fk add foreign key(i) references pk;
    
    -- Fails if the FK constraint is created as NOT VALID and validated
    -- afterwards.
    alter table fk add foreign key(i) references pk not valid;
    alter table fk validate constraint fk_i_fkey;
    
    -- 
    Antonin Houska
    Web: https://www.cybertec-postgresql.com
    
    
    
    
  2. Re: Foreign key validation failure in 18beta1

    Tender Wang <tndrwang@gmail.com> — 2025-05-28T10:54:30Z

    Antonin Houska <ah@cybertec.at> 于2025年5月28日周三 15:51写道:
    
    > I've come across an unexpected ERROR during validation of FK constraint in
    > PG
    > 18beta1. The same works in PG 17:
    >
    > drop table if exists fk;
    > drop table if exists pk;
    > create table pk(i int primary key) partition by range (i);
    > create table pk_1 partition of pk for values from (0) to (1);
    > create table pk_2 partition of pk for values from (1) to (2);
    > insert into pk values (0), (1);
    > create table fk(i int);
    > insert into fk values (1);
    >
    > -- Works if the FK constraint is created as valid.
    > --alter table fk add foreign key(i) references pk;
    >
    > -- Fails if the FK constraint is created as NOT VALID and validated
    > -- afterwards.
    > alter table fk add foreign key(i) references pk not valid;
    > alter table fk validate constraint fk_i_fkey;
    >
    
    git bisect shows since below commit, the failure started.
    
    commit b663b9436e7509b5e73c8c372539f067cd6e66c1
    Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
    Date:   Thu Jan 23 15:54:38 2025 +0100
    
        Allow NOT VALID foreign key constraints on partitioned tables
    
    
    -- 
    Thanks,
    Tender Wang
    
  3. Re: Foreign key validation failure in 18beta1

    Tender Wang <tndrwang@gmail.com> — 2025-05-28T12:12:16Z

    Tender Wang <tndrwang@gmail.com> 于2025年5月28日周三 18:54写道:
    
    >
    >
    > Antonin Houska <ah@cybertec.at> 于2025年5月28日周三 15:51写道:
    >
    >> I've come across an unexpected ERROR during validation of FK constraint
    >> in PG
    >> 18beta1. The same works in PG 17:
    >>
    >> drop table if exists fk;
    >> drop table if exists pk;
    >> create table pk(i int primary key) partition by range (i);
    >> create table pk_1 partition of pk for values from (0) to (1);
    >> create table pk_2 partition of pk for values from (1) to (2);
    >> insert into pk values (0), (1);
    >> create table fk(i int);
    >> insert into fk values (1);
    >>
    >> -- Works if the FK constraint is created as valid.
    >> --alter table fk add foreign key(i) references pk;
    >>
    >> -- Fails if the FK constraint is created as NOT VALID and validated
    >> -- afterwards.
    >> alter table fk add foreign key(i) references pk not valid;
    >> alter table fk validate constraint fk_i_fkey;
    >>
    >
    > git bisect shows since below commit, the failure started.
    >
    > commit b663b9436e7509b5e73c8c372539f067cd6e66c1
    > Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
    > Date:   Thu Jan 23 15:54:38 2025 +0100
    >
    >     Allow NOT VALID foreign key constraints on partitioned tables
    >
    >
    I dided the codes, in QueueFKConstraintValidation(),  we add three
    newconstraint for the
    fk rel, because the pk rel is partition table.
    
    During phase 3 of AlterTable, in ATRewriteTables(),
    call validateForeignKeyConstraint() three times.
    The first time the pk rel is pk, and it's ok.
    The second time the pk rel is only pk_1, and the type(1) is not in pk_1, so
    an error is reported.
    
    In this case, the two children newconstraint  should not be added to the
    queue.
    -- 
    Thanks,
    Tender Wang
    
  4. Re: Foreign key validation failure in 18beta1

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-05-28T12:26:06Z

    On 2025-May-28, Tender Wang wrote:
    
    > I dided the codes, in QueueFKConstraintValidation(),  we add three
    > newconstraint for the
    > fk rel, because the pk rel is partition table.
    > 
    > During phase 3 of AlterTable, in ATRewriteTables(),
    > call validateForeignKeyConstraint() three times.
    > The first time the pk rel is pk, and it's ok.
    > The second time the pk rel is only pk_1, and the type(1) is not in pk_1, so
    > an error is reported.
    > 
    > In this case, the two children newconstraint  should not be added to the
    > queue.
    
    Yeah, I reached the same conclusion and this is the preliminary fix I
    had written for it.  I don't like that I had to duplicate a few lines of
    code, but maybe it's not too bad.  Also the comments need to be
    clarified a bit more.
    
    Clearly b663b9436e75 lacked tests.
    
    -- 
    Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
    "Sallah, I said NO camels! That's FIVE camels; can't you count?"
    (Indiana Jones)
    
  5. Re: Foreign key validation failure in 18beta1

    Tender Wang <tndrwang@gmail.com> — 2025-05-28T12:38:15Z

    Alvaro Herrera <alvherre@alvh.no-ip.org> 于2025年5月28日周三 20:26写道:
    
    > On 2025-May-28, Tender Wang wrote:
    >
    > > I dided the codes, in QueueFKConstraintValidation(),  we add three
    > > newconstraint for the
    > > fk rel, because the pk rel is partition table.
    > >
    > > During phase 3 of AlterTable, in ATRewriteTables(),
    > > call validateForeignKeyConstraint() three times.
    > > The first time the pk rel is pk, and it's ok.
    > > The second time the pk rel is only pk_1, and the type(1) is not in pk_1,
    > so
    > > an error is reported.
    > >
    > > In this case, the two children newconstraint  should not be added to the
    > > queue.
    >
    > Yeah, I reached the same conclusion and this is the preliminary fix I
    > had written for it.  I don't like that I had to duplicate a few lines of
    > code, but maybe it's not too bad.  Also the comments need to be
    > clarified a bit more.
    >
    
    If the child table is still a partitioned table, the patch seems not work.
    
    I figure out a quick fix as the attached. I add a bool argument into
    the QueueFKConstraintValidation().
    If it is true, it means we recursively call QueueFKConstraintValidation(),
    then we don't add the newconstraint to the  queue.
    
    I'm not sure about this fix. Any thoughts?
    
    -- 
    Thanks,
    Tender Wang
    
  6. Re: Foreign key validation failure in 18beta1

    Tender Wang <tndrwang@gmail.com> — 2025-05-29T04:35:37Z

    Tender Wang <tndrwang@gmail.com> 于2025年5月28日周三 20:38写道:
    
    >
    >
    > Alvaro Herrera <alvherre@alvh.no-ip.org> 于2025年5月28日周三 20:26写道:
    >
    >> On 2025-May-28, Tender Wang wrote:
    >>
    >> > I dided the codes, in QueueFKConstraintValidation(),  we add three
    >> > newconstraint for the
    >> > fk rel, because the pk rel is partition table.
    >> >
    >> > During phase 3 of AlterTable, in ATRewriteTables(),
    >> > call validateForeignKeyConstraint() three times.
    >> > The first time the pk rel is pk, and it's ok.
    >> > The second time the pk rel is only pk_1, and the type(1) is not in
    >> pk_1, so
    >> > an error is reported.
    >> >
    >> > In this case, the two children newconstraint  should not be added to the
    >> > queue.
    >>
    >> Yeah, I reached the same conclusion and this is the preliminary fix I
    >> had written for it.  I don't like that I had to duplicate a few lines of
    >> code, but maybe it's not too bad.  Also the comments need to be
    >> clarified a bit more.
    >>
    >
    > If the child table is still a partitioned table, the patch seems not work.
    >
    
    I found a case that proves what I said above.
    create table pk(i int, j int, primary key(i,j)) partition by range (i);
    create table pk_1 partition of pk for values from (0) to (1) partition by
    list(j);
    create table pk_2 partition of pk for values from (1) to (2) partition by
    list(j);
    create table pk_1_1 partition of pk_1 for values in (1);
    create table pk_2_1 partition of pk_2 for values in (2);
    create table fk(i int, j int);
    alter table fk add foreign key(i, j) references pk not valid;
    postgres=# select oid ,conname , contype,
    convalidated,conrelid,conparentid,confrelid from pg_constraint where oid >=
    16384;
      oid  |    conname    | contype | convalidated | conrelid | conparentid |
    confrelid
    -------+---------------+---------+--------------+----------+-------------+-----------
     16422 | fk_i_j_fkey     | f       | f            |    16419 |
     0 |     16384
     16425 | fk_i_j_fkey_1 | f       | f            |    16419 |       16422 |
        16391
     16428 | fk_i_j_fkey_2 | f       | f            |    16419 |       16425 |
        16405
     16431 | fk_i_j_fkey_3 | f       | f            |    16419 |       16422 |
        16398
     16434 | fk_i_j_fkey_4 | f       | f            |    16419 |       16431 |
        16412
    
    alter table fk validate constraint fk_i_j_fkey;
    postgres=# select oid ,conname , contype,
    convalidated,conrelid,conparentid,confrelid from pg_constraint where oid >=
    16384;
      oid  |    conname    | contype | convalidated | conrelid | conparentid |
    confrelid
    -------+---------------+---------+--------------+----------+-------------+-----------
     16428 | fk_i_j_fkey_2 | f       | f            |    16419 |       16425 |
        16405
     16434 | fk_i_j_fkey_4 | f       | f            |    16419 |       16431 |
        16412
     16425 | fk_i_j_fkey_1 | f       | t            |    16419 |       16422 |
        16391
     16431 | fk_i_j_fkey_3 | f       | t            |    16419 |       16422 |
        16398
     16422 | fk_i_j_fkey     | f       | t            |    16419 |
     0 |     16384
    
    The   fk_i_j_fkey_2  and fk_i_j_fkey_4 are still invalid with your patch.
    
    
    > I figure out a quick fix as the attached. I add a bool argument into
    > the QueueFKConstraintValidation().
    > If it is true, it means we recursively call QueueFKConstraintValidation(),
    > then we don't add the newconstraint to the  queue.
    >
    > I'm not sure about this fix. Any thoughts?
    >
    
    -- 
    Thanks,
    Tender Wang
    
  7. Re: Foreign key validation failure in 18beta1

    jian he <jian.universality@gmail.com> — 2025-05-29T07:08:07Z

    On Wed, May 28, 2025 at 8:38 PM Tender Wang <tndrwang@gmail.com> wrote:
    >
    >
    >
    > Alvaro Herrera <alvherre@alvh.no-ip.org> 于2025年5月28日周三 20:26写道:
    >>
    >> On 2025-May-28, Tender Wang wrote:
    >>
    >> > I dided the codes, in QueueFKConstraintValidation(),  we add three
    >> > newconstraint for the
    >> > fk rel, because the pk rel is partition table.
    >> >
    >> > During phase 3 of AlterTable, in ATRewriteTables(),
    >> > call validateForeignKeyConstraint() three times.
    >> > The first time the pk rel is pk, and it's ok.
    >> > The second time the pk rel is only pk_1, and the type(1) is not in pk_1, so
    >> > an error is reported.
    >> >
    >> > In this case, the two children newconstraint  should not be added to the
    >> > queue.
    >>
    >> Yeah, I reached the same conclusion and this is the preliminary fix I
    >> had written for it.  I don't like that I had to duplicate a few lines of
    >> code, but maybe it's not too bad.  Also the comments need to be
    >> clarified a bit more.
    >
    >
    > If the child table is still a partitioned table, the patch seems not work.
    >
    > I figure out a quick fix as the attached. I add a bool argument into the QueueFKConstraintValidation().
    > If it is true, it means we recursively call QueueFKConstraintValidation(), then we don't add the newconstraint to the  queue.
    >
    > I'm not sure about this fix. Any thoughts?
    
    it will fail for the following case:
    
    begin;
    drop table if exists fk;
    drop table if exists pk;
    create table pk(i int primary key);
    insert into pk values (0), (1);
    create table fk(i int) partition by range (i);
    create table fk_1 partition of fk for values from (0) to (1);
    create table fk_2 partition of fk for values from (1) to (3);
    insert into fk values (1),(2);
    alter table fk add foreign key(i) references pk not valid;
    commit;
    alter table fk validate constraint fk_i_fkey; --error, should fail.
    
    -----------
    The attached *draft* patch is based on your idea.
    
    The idea is that we only need to conditionally do
    ``tab->constraints = lappend(tab->constraints, newcon);`` within
    QueueFKConstraintValidation.
    but the catalog update needs to be done recursively.
    
  8. Re: Foreign key validation failure in 18beta1

    amul sul <sulamul@gmail.com> — 2025-05-29T12:12:10Z

    On Thu, May 29, 2025 at 12:38 PM jian he <jian.universality@gmail.com> wrote:
    >
    > On Wed, May 28, 2025 at 8:38 PM Tender Wang <tndrwang@gmail.com> wrote:
    > >
    > >
    > >
    > > Alvaro Herrera <alvherre@alvh.no-ip.org> 于2025年5月28日周三 20:26写道:
    > >>
    > >> On 2025-May-28, Tender Wang wrote:
    > >>
    > >> > [...]
    > The attached *draft* patch is based on your idea.
    >
    > The idea is that we only need to conditionally do
    > ``tab->constraints = lappend(tab->constraints, newcon);`` within
    > QueueFKConstraintValidation.
    > but the catalog update needs to be done recursively.
    
    I like this approach, but I don’t think the flag name "recursing" is
    appropriate, as the flag is meant to indicate whether we want to
    enqueue constraints for validation or not.
    
    Additionally, I noticed that we can skip opening child partitions
    inside QueueFKConstraintValidation() when the referencing table is not
    partitioned -- that is, when it is the same as the input rel. Based on
    that, we can decide whether to queue the validation. However, I am not
    sure this optimization is worth the added complexity in the code.
    
    I have tried this in the attached patch (not the final version), with
    a brief explanation in the code comments. If we choose to move forward
    with this patch, I am happy to refine it and add proper tests.
    
    Regards,
    Amul
    
  9. Re: Foreign key validation failure in 18beta1

    jian he <jian.universality@gmail.com> — 2025-05-29T12:26:31Z

    On Thu, May 29, 2025 at 8:12 PM Amul Sul <sulamul@gmail.com> wrote:
    >
    > > >> > [...]
    > > The attached *draft* patch is based on your idea.
    > >
    > > The idea is that we only need to conditionally do
    > > ``tab->constraints = lappend(tab->constraints, newcon);`` within
    > > QueueFKConstraintValidation.
    > > but the catalog update needs to be done recursively.
    >
    > I like this approach, but I don’t think the flag name "recursing" is
    > appropriate, as the flag is meant to indicate whether we want to
    > enqueue constraints for validation or not.
    >
    
    Later, I came up with "need_validate", but it seems "queueValidation"
    is better.
    
    I just realized we have the same problem with ALTER FOREIGN KEY ENFORCED.
    for example:
    begin;
    drop table if exists fk;
    drop table if exists pk;
    create table pk(i int, b int, primary key(i ,b)) partition by range (i);
    create table pk_1 partition of pk for values from (0) to (10)
    partition by list(b);
    CREATE TABLE pk_1_p1 PARTITION OF pk_1 FOR VALUES IN (0, 1, 2);
    create table pk_2 partition of pk for values from (10) to (12);
    insert into pk values (0, 1), (1,2);
    create table fk(i int, b int);
    insert into fk values (0, 1), (1,3);
    -- alter table fk add foreign key(i) references pk;
    alter table fk add constraint fk_i_fkey foreign key(i, b) references
    pk not enforced;
    commit;
    
    alter table fk alter constraint fk_i_fkey enforced; --error
    delete from fk where i = 1 and b = 3;
    alter table fk alter constraint fk_i_fkey enforced; --ok
    
    
    
    
  10. Re: Foreign key validation failure in 18beta1

    amul sul <sulamul@gmail.com> — 2025-05-29T12:58:08Z

    On Thu, May 29, 2025 at 5:57 PM jian he <jian.universality@gmail.com> wrote:
    >
    > On Thu, May 29, 2025 at 8:12 PM Amul Sul <sulamul@gmail.com> wrote:
    > >
    > > > >> > [...]
    > > > The attached *draft* patch is based on your idea.
    > > >
    > > > The idea is that we only need to conditionally do
    > > > ``tab->constraints = lappend(tab->constraints, newcon);`` within
    > > > QueueFKConstraintValidation.
    > > > but the catalog update needs to be done recursively.
    > >
    > > I like this approach, but I don’t think the flag name "recursing" is
    > > appropriate, as the flag is meant to indicate whether we want to
    > > enqueue constraints for validation or not.
    > >
    >
    > Later, I came up with "need_validate", but it seems "queueValidation"
    > is better.
    >
    > I just realized we have the same problem with ALTER FOREIGN KEY ENFORCED.
    > for example:
    
    Yeah, I think adding a "currcon->confrelid == pkrelid" check before
    enqueueing validation in ATExecAlterConstrEnforceability() would
    address the issue, though I still need to test it thoroughly.
    
    Regards,
    Amul
    
    
    
    
  11. Re: Foreign key validation failure in 18beta1

    jian he <jian.universality@gmail.com> — 2025-05-30T08:06:53Z

    On Thu, May 29, 2025 at 8:58 PM Amul Sul <sulamul@gmail.com> wrote:
    > >
    > > I just realized we have the same problem with ALTER FOREIGN KEY ENFORCED.
    > > for example:
    >
    > Yeah, I think adding a "currcon->confrelid == pkrelid" check before
    > enqueueing validation in ATExecAlterConstrEnforceability() would
    > address the issue, though I still need to test it thoroughly.
    >
    
    adding a "currcon->confrelid == pkrelid"  will fix the problem.
    I have used the attached scripts to test foreign key functionality:
    ALTER TABLE VALIDATE CONSTRAINT and
    ALTER TABLE ALTER CONSTRAINT ENFORCED.
    
    
    v3-0001-foreign_key_validation-WIP also works fine.
    but, I have one minor issue with the comment.
    
    + /*
    + * When the referenced table is partitioned, the referencing table
    + * may have multiple foreign key constraints -- one for each child
    + * table. These individual constraints do not require separate
    + * validation, as validating the constraint on the root partitioned
    + * table is sufficient.
    
    The last sentence doesn't seem to explain why.
    The following is what I came up with (my understanding):
    ""
    If the primary key (PK) is partitioned, we *can't* queue validation (add a
    NewConstraint on PK partition and validate it in phase3) for its partitions.
    Validating foreign key constraints between primary key (PK) partitions and
    foreign keys (FK) can fail. This is because some PK partitions may legitimately
    lack corresponding FK values, which is acceptable but causes validation errors.
    ""
    
  12. Re: Foreign key validation failure in 18beta1

    amul sul <sulamul@gmail.com> — 2025-05-30T10:31:59Z

    On Fri, May 30, 2025 at 1:37 PM jian he <jian.universality@gmail.com> wrote:
    >
    > On Thu, May 29, 2025 at 8:58 PM Amul Sul <sulamul@gmail.com> wrote:
    > > >
    > > > I just realized we have the same problem with ALTER FOREIGN KEY ENFORCED.
    > > > for example:
    > >
    > > Yeah, I think adding a "currcon->confrelid == pkrelid" check before
    > > enqueueing validation in ATExecAlterConstrEnforceability() would
    > > address the issue, though I still need to test it thoroughly.
    > >
    >
    > adding a "currcon->confrelid == pkrelid"  will fix the problem.
    > I have used the attached scripts to test foreign key functionality:
    > ALTER TABLE VALIDATE CONSTRAINT and
    > ALTER TABLE ALTER CONSTRAINT ENFORCED.
    >
    
    Thanks for the checking.
    
    >
    > v3-0001-foreign_key_validation-WIP also works fine.
    > but, I have one minor issue with the comment.
    >
    > + /*
    > + * When the referenced table is partitioned, the referencing table
    > + * may have multiple foreign key constraints -- one for each child
    > + * table. These individual constraints do not require separate
    > + * validation, as validating the constraint on the root partitioned
    > + * table is sufficient.
    >
    > The last sentence doesn't seem to explain why.
    > The following is what I came up with (my understanding):
    > ""
    > If the primary key (PK) is partitioned, we *can't* queue validation (add a
    > NewConstraint on PK partition and validate it in phase3) for its partitions.
    > Validating foreign key constraints between primary key (PK) partitions and
    > foreign keys (FK) can fail. This is because some PK partitions may legitimately
    > lack corresponding FK values, which is acceptable but causes validation errors.
    > ""
    
    I would prefer to use 'referenced' and 'referencing' tables instead of
    'PK' and 'FK' tables, to stay consistent with the existing style. I
    tried to improve the comment in the attached version with the
    appropriate reason.
    
    Kindly take a look at the attached version. I've also added the tests.
    Thanks for your script -- all tests are passing with this patch.
    
    Regards,
    Amul
    
  13. Re: Foreign key validation failure in 18beta1

    jian he <jian.universality@gmail.com> — 2025-06-01T12:35:08Z

    On Fri, May 30, 2025 at 6:32 PM Amul Sul <sulamul@gmail.com> wrote:
    >
    >
    > Kindly take a look at the attached version. I've also added the tests.
    > Thanks for your script -- all tests are passing with this patch.
    >
    
    hi.
    
    + * Note that validation should be performed against the referencing
    + * root table only, not its child partitions. See
    + * QueueFKConstraintValidation() for more details.
      */
            if (rel->rd_rel->relkind == RELKIND_RELATION &&
                currcon->confrelid == pkrelid)
            {
                AlteredTableInfo *tab;
                NewConstraint *newcon;
                newcon = (NewConstraint *) palloc0(sizeof(NewConstraint));
                ....
            }
    
    in the comments "referencing" should be "referenced"?
    
    other than that, it looks good.
    
    
    
    
  14. Re: Foreign key validation failure in 18beta1

    amul sul <sulamul@gmail.com> — 2025-06-02T03:26:27Z

    On Sun, Jun 1, 2025 at 6:05 PM jian he <jian.universality@gmail.com> wrote:
    >
    > On Fri, May 30, 2025 at 6:32 PM Amul Sul <sulamul@gmail.com> wrote:
    > >
    [...]
    >
    > + * Note that validation should be performed against the referencing
    > + * root table only, not its child partitions. See
    > + * QueueFKConstraintValidation() for more details.
    >   */
    >         if (rel->rd_rel->relkind == RELKIND_RELATION &&
    >             currcon->confrelid == pkrelid)
    >         {
    >             AlteredTableInfo *tab;
    >             NewConstraint *newcon;
    >             newcon = (NewConstraint *) palloc0(sizeof(NewConstraint));
    >             ....
    >         }
    >
    > in the comments "referencing" should be "referenced"?
    >
    Yes, fixed in the attached version.
    
    > other than that, it looks good.
    >
    Thanks for the review.
    
    Regards,
    Amul
    
  15. Re: Foreign key validation failure in 18beta1

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-06-02T16:26:29Z

    Hello
    
    I find this coding somewhat confusing.  Now you have a function
    "QueueFkConstraintValidation" which may queue a FK queue constraint
    validation, if the flag "queueValidation" is given, but it may also do
    something else -- makes no sense IMO.  I think it's better to split this
    function in two; one does indeed validate, the other is a simple helper
    that only sets the catalog flag, and does its own recursion.  This
    causes a bit of duplicate code, but is simpler to follow.  (We could
    also refactor the duplicate code by adding another helper routine, but
    I'm not sure it's worth the trouble.)
    
    I would propose something like this instead, what do you think?
    
    
    I hate that pg_constraint.conparentid doesn't have a syscache or at
    least an index :-(
    
    -- 
    Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
    "El miedo atento y previsor es la madre de la seguridad" (E. Burke)
    
  16. Re: Foreign key validation failure in 18beta1

    amul sul <sulamul@gmail.com> — 2025-06-03T04:14:18Z

    On Mon, Jun 2, 2025 at 9:56 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
    >
    > Hello
    >
    > I find this coding somewhat confusing.  Now you have a function
    > "QueueFkConstraintValidation" which may queue a FK queue constraint
    > validation, if the flag "queueValidation" is given, but it may also do
    > something else -- makes no sense IMO.
    
    I wouldn’t disagree with that.
    
    > I think it's better to split this
    > function in two; one does indeed validate, the other is a simple helper
    > that only sets the catalog flag, and does its own recursion.  This
    > causes a bit of duplicate code, but is simpler to follow.  (We could
    > also refactor the duplicate code by adding another helper routine, but
    > I'm not sure it's worth the trouble.)
    >
    > I would propose something like this instead, what do you think?
    >
    >
    
    I found a third approach that requires only a few changes. The key
    idea is to determine the root referenced table and pass it to
    QueueFKConstraintValidation(). We would then enqueue phase 3
    validation only if the constraint tuple’s confrelid matches that root
    table -- similar to what is doing in ATExecAlterConstrEnforceability().
    
    This would also ensure that the logic for adding/skipping phase 3
    validation is consistent in both places.
    
    Adding an extra parameter (i.e., pkrelid) doesn’t seem out of place,
    as it aligns with other functions in the same file, such as
    ATExecAlterConstraintInternal() and ATExecAlterConstrEnforceability().
    
    Kindly take a look at the attached version and share your thoughts.
    
    Regards,
    Amul
    
  17. Re: Foreign key validation failure in 18beta1

    jian he <jian.universality@gmail.com> — 2025-06-04T03:18:41Z

    On Tue, Jun 3, 2025 at 12:14 PM Amul Sul <sulamul@gmail.com> wrote:
    > >
    >
    > I found a third approach that requires only a few changes. The key
    > idea is to determine the root referenced table and pass it to
    > QueueFKConstraintValidation(). We would then enqueue phase 3
    > validation only if the constraint tuple’s confrelid matches that root
    > table -- similar to what is doing in ATExecAlterConstrEnforceability().
    >
    > This would also ensure that the logic for adding/skipping phase 3
    > validation is consistent in both places.
    >
    indeed!
    
    v7 is way more intuitive compared with v5, v6.
    The commit message also looks fine.
    
    
    
    
  18. Re: Foreign key validation failure in 18beta1

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-06-05T17:01:06Z

    On 2025-Jun-04, jian he wrote:
    
    > On Tue, Jun 3, 2025 at 12:14 PM Amul Sul <sulamul@gmail.com> wrote:
    >
    > > I found a third approach that requires only a few changes. The key
    > > idea is to determine the root referenced table and pass it to
    > > QueueFKConstraintValidation(). We would then enqueue phase 3
    > > validation only if the constraint tuple’s confrelid matches that root
    > > table -- similar to what is doing in ATExecAlterConstrEnforceability().
    > >
    > > This would also ensure that the logic for adding/skipping phase 3
    > > validation is consistent in both places.
    >
    > indeed!
    > 
    > v7 is way more intuitive compared with v5, v6.
    
    Agreed, this version is better than the previous ones.  I split it as
    two commits, for NOT VALID and NOT ENFORCED separately, and modified some
    comments a bit as well as the commit messages.
    
    Thank you!
    
    -- 
    Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
    "Cuando mañana llegue pelearemos segun lo que mañana exija" (Mowgli)
    
    
    
    
  19. Re: Foreign key validation failure in 18beta1

    amul sul <sulamul@gmail.com> — 2025-06-06T04:25:44Z

    On Thu, Jun 5, 2025 at 10:31 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
    >
    > On 2025-Jun-04, jian he wrote:
    >
    > > On Tue, Jun 3, 2025 at 12:14 PM Amul Sul <sulamul@gmail.com> wrote:
    > >
    > >
    > > v7 is way more intuitive compared with v5, v6.
    >
    > Agreed, this version is better than the previous ones.  I split it as
    > two commits, for NOT VALID and NOT ENFORCED separately, and modified some
    > comments a bit as well as the commit messages.
    >
    
    Yeah, it makes sense to have this as a separate commit. Thank you!
    
    Regards,
    Amul