Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher

Hi Shi Yu, all

> e.g.
> -- pub
> CREATE TABLE test_replica_id_full (x int, y int);
> ALTER TABLE test_replica_id_full REPLICA IDENTITY FULL;
> CREATE PUBLICATION tap_pub_rep_full FOR TABLE test_replica_id_full;
> -- sub
> CREATE TABLE test_replica_id_full (x int, y int, z int);
> CREATE INDEX test_replica_id_full_idx ON test_replica_id_full(z);
> CREATE SUBSCRIPTION tap_sub_rep_full_0 CONNECTION 'dbname=postgres
> port=5432' PUBLICATION tap_pub_rep_full;
>
> I didn't see in any cases the behavior changed after applying the patch,
> which
> looks good. Besides, I tested the performance for such case.
>

Thanks for testing this edge case. I thought we had a test for this, but it
seems to be missing.

>
> For the cases that all values of extra columns on the subscriber are NULL,
> index
> scan can't do better than sequential scan. This is not a real scenario and
> I
> think it only degrades when there are many NULL values in the index
> column, so
> this is probably not a case to worry about.

I also debugged this case as well, and don't see any problems with that
either. But I think this is a valid
test case given at some point we might forget about this case and somehow
break.

So, I'll add a new test with *PUBLICATION LACKS THE COLUMN ON THE SUBS
INDEX *on v36.

> I just share this case and then we
> can discuss should we pick the index which only contain the extra columns
> on the
> subscriber.
>
>
I think its performance implications come down to the discussion on [1].
Overall, I prefer
avoiding adding any additional complexity in the code for some edge cases.
The code
can handle this sub-optimal user pattern, with a sub-optimal performance.

Still, happy to hear other thoughts on this.

Thanks,
Onder KALACI

[1]
https://www.postgresql.org/message-id/20230307195119.ars36cx6gwqftoen%40awork3.anarazel.de