Dear Önder,

Thanks for updating the patch!
I played with your patch and I confirmed that parallel apply worker and 
tablesync worker
could pick the index in typical case.

Followings are comments for v27-0001. Please ignore if it is fixed in newer one.

execReplication.c

```
+       /* Build scankey for every non-expression attribute in the index. */
```

I think that single line comments should not terminated by ".".

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

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.

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.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Reply via email to