On 11/13/24 02:11, Peter Eisentraut wrote:
I have committed the documentation patches
v43-0001-Add-WITHOUT-OVERLAPS-and-PERIOD-to-ALTER-TABLE-r.patch
v43-0002-Update-conexclop-docs-for-WITHOUT-OVERLAPS.patch
Thanks!
For the logical replication fixes
v43-0003-Fix-logical-replication-for-temporal-tables.patch
can you summarize what the issues currently are? Is it currently broken, or just not working as
well as it could?
AFAICT, there might be two separate issues. One is that you can't use a temporal index as replica
identity, because ALTER TABLE rejects it. The other is that a subscriber fails to make use of a
replica identity index, because it uses the wrong strategy numbers.
Correct, there are two issues this commit fixes:
On the publisher side: You can use REPLICA IDENTITY DEFAULT with a temporal PK/UNIQUE index. There
is no validation step, and sending the changes works fine. But REPLICA IDENTITY USING INDEX fails
because the validation step rejects the non-btree index.
Then on the subscriber side, we are not applying changes correctly, because we assume the strategy
numbers are btree numbers.
This conditional is really hard to understand:
+ /*
+ * The AM must support uniqueness, and the index must in fact be unique.
+ * If we have a WITHOUT OVERLAPS constraint (identified by uniqueness +
+ * exclusion), we can use that too.
+ */
+ if ((!indexRel->rd_indam->amcanunique ||
+ !indexRel->rd_index->indisunique) &&
+ !(indexRel->rd_index->indisunique &&
indexRel->rd_index->indisexclusion))
Why can we have a indisunique index when the AM is not amcanunique? Are we
using the fields wrong?
- return get_equal_strategy_number_for_am(am);
+ /* For GiST indexes we need to ask the opclass what strategy number to
use. */
+ if (am == GIST_AM_OID)
+ return GistTranslateStratnum(opclass, RTEqualStrategyNumber);
+ else
+ return get_equal_strategy_number_for_am(am);
This code should probably be pushed into get_equal_strategy_number_for_am(). That function already
has a switch based on index AM OIDs. Also, there are other callers of
get_equal_strategy_number_for_am(), which also might want to become aware of GiST support.
For the new test file, remember to add it to src/test/subscription/meson.build.
Also, maybe add a introductory comment in the test file to describe generally
what it's trying to test.
Okay, I'll make these changes and re-send the patch.
Yours,
--
Paul ~{:-)
p...@illuminatedcomputing.com
