Re: Virtual generated columns

On Mon, 27 Jan 2025 at 15:29, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> On 15.01.25 20:37, Peter Eisentraut wrote:
> > On 15.01.25 15:12, Dean Rasheed wrote:
> >> On Tue, 14 Jan 2025 at 13:37, Peter Eisentraut <peter(at)eisentraut(dot)org>
> >> wrote:
> >>>
> >>> Here is a new patch with that fixed and also a few
> >>> tweaks suggested by Jian.
> >>>
> >>
> >> I'm hoping to push my RETURNING OLD/NEW patch [1] soon, so I thought
> >> that I would check how it works together with this patch. The good
> >> news is that AFAICS everything just works, and it's possible to return
> >> old/new virtual generated columns in DML queries as expected.
> >>
> >> It did require a minor update, because my patch adds a new
> >> "result_relation" argument to ReplaceVarsFromTargetList() -- needed in
> >> DML queries because, when propagating a Var's old/new
> >> varreturningtype, replacement Vars need to be handled differently
> >> depending on whether or not they refer to the result relation. So that
> >> affects expand_generated_columns_internal(), when called from
> >> fireRIRrules(). OTOH, from expand_generated_columns_in_expr() it's OK
> >> to just pass 0 as the result relation index, because there won't be
> >> any old/new Vars in an expression that's not part of a DML query.
> >>
> >> Attached is the delta patch I used to handle this, along with a couple
> >> of simple test cases. It doesn't really matter which feature makes it
> >> in first, but the one that comes second will need to do something like
> >> this.
> >
> > Ok, I'll wait if you want to go ahead with yours soon.
>
> Here is an updated patch that integrates the above changes and also
> makes some adjustments now that the logical replication configuration
> questions are resolved. I think this is complete now.
>
> But I'm seeing mysterious CI failures that have me stumped. For example:
>
> https://cirrus-ci.com/task/5924251028422656
>
> I have seen this particular pgbench test failure sporadically but
> several times, and I have not seen it anywhere without this patch, and
> never locally. The macOS task on the cfbot CI is very flaky right now,
> so it's hard to get a good baseline. Also, it seems to me that this
> failing test could not possibly be further away from the code that the
> patch changes, so I'm thinking timing problems, but it only happens on
> the macOS task. Really weird.

Hi,

I did some testing related to logical replication on the patch:

Test1: With row filter on publisher

-- publisher:
CREATE TABLE t1 (a int, b int GENERATED ALWAYS AS (a * 2) VIRTUAL);
create publication pub1 for table t1 where (b > 50);
INSERT INTO t1 values(1);
INSERT INTO t1 values(32);

-- subscriber
CREATE TABLE t1 (a int, b int);
create subscription test1 connection 'dbname=postgres host=localhost
port=5432' publication pub1;
select * from t1;
a | b
----+---
32 |
(1 row)

Only records where b>50 are replicated to the subscriber.

--------------------------------------------------------------------------------------------------------------------

Test 2: Replication of virtual generated column using user defined operator

-- publisher
CREATE OPERATOR === (
leftarg = integer,
rightarg = integer,
procedure = int4eq
);
CREATE TABLE t1 (a int, b bool GENERATED ALWAYS AS (a === 10)
VIRTUAL); INSERT INTO t1 values(1);
INSERT INTO t1 values(10);

-- create publication with row filter with user defined operator
create publication pub1 for table t1 where (a === 10);

ERROR: invalid publication WHERE expression LINE 1: create
publication pub1 for table t1 where (a === 10)
^
DETAIL: User-defined operators are not allowed.

-- create publication on virtual generated column using user defined operator
create publication pub1 for table t1 where (b = 't');
ERROR: invalid publication WHERE expression
DETAIL: User-defined operators are not allowed.

----------------------------------------------------------------------------------------------------------------

Test 3: CREATE PUBLICATION on column list with Virtual generated column

CREATE TABLE t1 (a int, b int GENERATED ALWAYS AS (a * 2) VIRTUAL);
create publication pub1 for table t1 (a, b);

It is failing with error:
ERROR: cannot use virtual generated column "b" in publication column list.

----------------------------------------------------------------------------------------------------------------

Test 4: Update publication on non virtual gen

CREATE TABLE t1 (a int, b int GENERATED ALWAYS AS (a * 2) VIRTUAL);
create publication pub1 for table t1 (a);
alter table t1 replica identity full;
update t1 set a = 10;

ERROR: cannot update table "t1"
DETAIL: Column list used by the publication does not cover the
replica identity.

----------------------------------------------------------------------------------------------------------------

Test 5: Update publication on non virtual gen with no column list specified

CREATE TABLE t1 (a int, b int GENERATED ALWAYS AS (a * 2) VIRTUAL);
create publication pub1 for table t1;
alter table t1 replica identity full;
update t1 set a = 10;

No error is thrown, and an update is happening. It should have thrown
an ERROR as the unpublished generated column 'b' is part of the
replica identity.

Thanks and Regards,
Shlok Kyal