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

Hi,

> ~~~
> 01. 032_subscribe_use_index.pl - SUBSCRIPTION CAN UPDATE THE INDEX IT
> USES AFTER ANALYZE
>
> ```
> # show that index_b is not used
> $node_subscriber->poll_query_until(
> 'postgres', q{select idx_scan=0 from pg_stat_all_indexes where
> indexrelname = 'index_b';}
> ) or die "Timed out while waiting for check subscriber tap_sub_rep_full
> updates two rows via index scan with index on high cardinality column-2";
> ```
>
> poll_query_until() is still remained here, it should be replaced to is().
>
>
>
Updated

02. 032_subscribe_use_index.pl - SUBSCRIPTION BEHAVIOR WITH ENABLE_INDEXSCAN
>
> ```
> # show that the unique index on replica identity is used even when
> enable_indexscan=false
> $result = $node_subscriber->safe_psql('postgres',
> "select idx_scan from pg_stat_all_indexes where indexrelname =
> 'test_replica_id_full_idx'");
> is($result, qq(0), 'ensure subscriber has not used index with
> enable_indexscan=false');
> ```
>
> Is the comment wrong? The index test_replica_id_full_idx is not used here.
>
>
Yeah, the comment is wrong. It is a copy & paste error from the other test.
Fixed now

>
>
> 03. 032_subscribe_use_index.pl - SUBSCRIPTION BEHAVIOR WITH
> ENABLE_INDEXSCAN
>
> ```
> $node_publisher->safe_psql('postgres',
> "ALTER TABLE test_replica_id_full REPLICA IDENTITY USING INDEX
> test_replica_id_full_unique;");
> ```
>
> I was not sure why ALTER TABLE REPLICA IDENTITY USING INDEX was done on
> the publisher side.
> IIUC this feature works when REPLICA IDENTITY FULL is specified on a
> publisher,
> so it might not be altered here. If so, an index does not have to define
> on the publisher too.
>
>
Yes, not strictly necessary but it is often the case that both
subscriber and publication have the similar schemas when unique index/pkey
is used. For example, see t/028_row_filter.pl where we follow this pattern.

Still, I manually tried that without the index on the publisher (e.g.,
replica identity full), that works as expected. But given that the majority
of the tests already have that approach and this test focuses on
enable_indexscan, I think I'll keep it as is - unless it is confusing?

> 04. 032_subscribe_use_index.pl - SUBSCRIPTION BEHAVIOR WITH
> ENABLE_INDEXSCAN
>
> ```
> $node_subscriber->poll_query_until(
> 'postgres', q{select (idx_scan=1) from pg_stat_all_indexes where
> indexrelname = 'test_replica_id_full_unique'}
> ) or die "Timed out while waiting ensuring subscriber used unique index as
> replica identity even with enable_indexscan=false'";
> ```
>
> 03 comment should be added here.
>
> Yes, done that as well.

Attached v17 now. Thanks for the review!