RE: Dropped and generated columns might cause wrong data on subs when REPLICA IDENTITY FULL

[shiy.f...@fujitsu.com]

On Tue, Mar 21, 2023 4:51 PM Önder Kalacı <onderkal...@gmail.com> wrote:
> 
> Hi Amit, Shi Yu
> 
> Now attaching the similar patches for generated columns.
> 

Thanks for your patches. Here are some comments.

1.
 $node_publisher->safe_psql(
        'postgres', qq(
                ALTER TABLE dropped_cols DROP COLUMN b_drop;
+               ALTER TABLE generated_cols DROP COLUMN b_gen;
 ));
 $node_subscriber->safe_psql(
        'postgres', qq(
                ALTER TABLE dropped_cols DROP COLUMN b_drop;
+               ALTER TABLE generated_cols DROP COLUMN b_gen;
 ));

I think we want to test generated columns, so we don't need to drop columns.
Otherwise the generated column problem can't be detected.

2. 
# The bug was that when the REPLICA IDENTITY FULL is used with dropped columns,
# we fail to apply updates and deletes

Maybe we should mention generated columns in comment of the test.

3.
I ran pgindent and it modified some lines. Maybe we can improve the patch
as the following.

@@ -292,8 +292,8 @@ tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
                att = TupleDescAttr(slot1->tts_tupleDescriptor, attrnum);
 
                /*
-                * Ignore dropped and generated columns as the publisher
-                * doesn't send those
+                * Ignore dropped and generated columns as the publisher 
doesn't send
+                * those
                 */
                if (att->attisdropped || att->attgenerated)
                        continue;

Regards,
Shi Yu