Обсуждение: foreign key on virtual generated column

Hi Jian , Thanks for working on this , I started reviewing these patches ,
just need a very small rebase to v1-0002 because a recent
commit(cc2ac0e) removed catalog/pg_proc.h header from ri_triggers.c .

Hi Jian,

did initial testing and review ,the patches LGTM except
indentation we can solve this by running pg_indent.

i have one doubt that why we are using compute_virtual
argument todo the same work in both cases, is it because
of future proofing ,like we may do something different for
stored and virtual in future?

/* No luck, so prepare the expression for execution */
if (attgenerated == ATTRIBUTE_GENERATED_STORED)
{
ri_GeneratedExprs[i] = ExecPrepareExpr(expr, estate);
ri_NumGeneratedNeeded++;
}
else if (compute_virtual)
{
ri_GeneratedExprs[i] = ExecPrepareExpr(expr, estate);
ri_NumGeneratedNeeded++;
}

Hi Jian,

while continuing my review , i observed these:

1) if we have multiple virtual generated columns in table,
and foreign key on a virtual generated column, during
RI check in RI_FKey_check, i observed that we are
computing other virtual generated columns which are
not part of foreign key,which i think is (not too) slow but
slowness depends on how complex the generated
expressions are, number of virtual generated columns
in the table ,i think we can use riinfo inside

ExecComputeGenerated to only compute the columns
in foreign key.

                         Table "public.gtest23b"
 Column |  Type   | Collation | Nullable |            Default            
--------+---------+-----------+----------+-------------------------------
 a      | integer |           | not null |
 b      | bigint  |           |          | generated always as ((a * 1))
 c      | integer |           |          | generated always as ((a * 2))
Indexes:
    "gtest23b_pkey" PRIMARY KEY, btree (a)
Foreign-key constraints:
    "gtest23b_b_fkey" FOREIGN KEY (b) REFERENCES gtest23a(x) ON DELETE CASCADE

when we do "insert into gtest23b values(3);"
RI kicks in and computes the b,c columns
to check the values of "b" exists in primary key,
but here the virtual generated columns which
are not part of the foreign key like "c" are also
computed.

2) during ALTER type or expression of a column
which is involved in foreign key , we go through
TryReuseForeignKey to determine whether
revalidation of this constraint can be skipped, here
i observed for this patch that if we saw a virtual
generated column in foreign key we simply bail out
and won't skip the revalidation, i have a doubt and
an observation here

doubt:- why are we simply bailing out and doing
revalidation if we found out if there's a virtual
generated column in foreign key? in TryReuseForeignKey,
is it because if we change the expression obviously get
new values ,so we anyway need to revalidate them with
primary key but what about if virtual column's type gets
ALTERed in such a way that the equality operator is same
underneath like varchar(10) to varchar(40) , then i think
we can allow the normal flow as we do with normal columns
by appending the OID to old_conpfeqop as below.

for (i = 0; i < numkeys; i++)
con->old_conpfeqop = lappend_oid(con->old_conpfeqop, conpfeqop[i]);

observation:- if we go with current logic of bailing out and
doing the revalidation if a virtual gen column is present in
foreign key, then even if we are only altering a standard
column that is perfectly safe (binary compatible) like
varchar(10) to varchar(40) and not doing any change
to the virtual generated column in a composite foreign
key, even then we are going to do revalidation and
goes to validateForeignKeyConstraint which is
kind of slow, as the values in foreign key are same
as before even after ALTER, so its not needed to
revalidate again primary key, and if we try to simulate
the same scenario without the virtual generated column
and instead a normal column then postgres skips the revalidation,
because the ALTERed column is a standard column
and the type underneath is "compatible",so there won't
be any changes to the values in foreign key to revalidate
with the primary key, so it's skipped.

for example:

CREATE TABLE parent (
    id_std text,
    id_extra int,
    PRIMARY KEY (id_std, id_extra)
);

INSERT INTO parent VALUES ('A', 1), ('B', 2);


Scenario 1:  No Virtual Columns
Here, we have a composite FK, but both columns are standard.
We alter 'col_std' (varchar 10 -> 40).
Expectation: Optimization SUCCEEDS. Validation SKIPPED.

CREATE TABLE child_a (
    col_std varchar(10),
    col_extra int,
    FOREIGN KEY (col_std, col_extra) REFERENCES parent(id_std, id_extra)
);

INSERT INTO child_a VALUES ('A', 1);

ALTER TABLE child_a ALTER COLUMN col_std TYPE varchar(40);


Scenario 2: With Virtual Column
Here, we have a composite FK where one column is Virtual.
We alter 'col_std' (varchar 10 -> 50). 'col_virt' is Untouched.
Expectation: Optimization FAILS. Validation FORCED (Slow).

CREATE TABLE child_b (
    col_std varchar(10),
    col_virt int GENERATED ALWAYS AS (1) VIRTUAL,
    FOREIGN KEY (col_std, col_virt) REFERENCES parent_comp(id_std, id_extra)
);

INSERT INTO child_b VALUES ('A');

This should trigger the "validateForeignKeyConstraint" path because TryReuseForeignKey bails out.
ALTER TABLE child_b ALTER COLUMN col_std TYPE varchar(40);