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

Hi Hayato, Amit, all

>
>
> ```
> + /* Build scankey for every non-expression attribute in the index.
> */
> ```
>
> I think that single line comments should not terminated by ".".
>

Hmm, looking into execReplication.c, many of the single line comments
terminated by ".". Also, On the HEAD, the same comment has single
line comment. So, I'd rather stick to that?

>
> ```
> + /* There should always be at least one attribute for the index
> scan. */
> ```
>
> Same as above.
>

Same as above :)

>
>
> ```
> +#ifdef USE_ASSERT_CHECKING
> + IndexInfo *indexInfo = BuildIndexInfo(idxrel);
> +
> + Assert(!IsIndexOnlyOnExpression(indexInfo));
> +#endif
> ```
>
> I may misunderstand, but the condition of usable index has alteady been
> checked
> when the oid was set but anyway the you confirmed the condition again
> before
> really using that, right?
> So is it OK not to check another assumption that the index is b-tree,
> non-partial,
> and one column reference?

IIUC we can do that by adding new function like
> IsIndexUsableForReplicaIdentityFull()
> that checks these condition, and then call at
> RelationFindReplTupleByIndex() if
> idxIsRelationIdentityOrPK is false.
>

I think adding a function like IsIndexUsableForReplicaIdentityFull is
useful. I can use it inside
FindUsableIndexForReplicaIdentityFull() and assert here. Also good for
readability.

So, I mainly moved this assert to a more generic place with a more generic
check
to RelationFindReplTupleByIndex

>
> 032_subscribe_use_index.pl
>
> ```
> +# Testcase start: SUBSCRIPTION CREATE/DROP INDEX WORKS WITHOUT ISSUES
> ...
> +# Testcase end: SUBSCRIPTION RE-CALCULATES INDEX AFTER CREATE/DROP INDEX
> ```
>
> There is still non-consistent case.
>
>
Fixed, thanks

Anyway, I see your point, so
> if you want to keep the naming as you proposed at least don't change
> the ordering for get_opclass_input_type() call because that looks odd
> to me.

(A small comment from Amit's previous e-mail)

Sure, applied now.

Attaching v31.

Thanks for the reviews!
Onder KALACI