Feedback-ID: ie0a040ee:Fastmail
Message-ID: <6dc196d4-8853-49d7-8dd3-e391058451c1@eisentraut.org>
Date: Wed, 13 Nov 2024 11:11:37 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: SQL:2011 application time
To: Paul Jungwirth <pj@illuminatedcomputing.com>
Cc: jian he <jian.universality@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com>
 <b4363a52-2279-4612-8953-1100fab0c772@illuminatedcomputing.com>
 <30dc3561-d7ab-42be-a1c5-096643423590@illuminatedcomputing.com>
 <b824fc91-f534-4096-8d94-de8e21b06915@illuminatedcomputing.com>
 <73d09e24-b6b9-4c89-8c0a-8842ca0f81f9@illuminatedcomputing.com>
 <c002230c-8879-4b2c-ad05-585d6572347c@illuminatedcomputing.com>
 <CACJufxESqArbu=R8m3a+ZF-Yia26KTGpEnyqz+8d5BXb1o=c=w@mail.gmail.com>
 <CACJufxECGaLjMcN8TJbX1fGrMFKs6qu+6RiZm98=sLcpiVpu2g@mail.gmail.com>
 <081aa43c-3a97-4bd5-a498-ba02e0a7570d@illuminatedcomputing.com>
 <54390149-11d6-4f52-bbe8-f69832ceb3bd@eisentraut.org>
 <eedaaca9-d615-4934-ab00-39ec376d9b37@eisentraut.org>
 <CA+renyUFC13F0tYKxEENZtWA0YVuS5Tv+ZQkEkAwuDO1-Xke-A@mail.gmail.com>
 <9b7cb814-c78c-489f-85b2-32be4398647f@illuminatedcomputing.com>
 <95c6d49b-7d60-4ad0-820c-e00ae1b2a3d4@illuminatedcomputing.com>
 <57ea0668-5205-426e-b934-efc89f2186c2@illuminatedcomputing.com>
Content-Language: en-US
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: <57ea0668-5205-426e-b934-efc89f2186c2@illuminatedcomputing.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/6dc196d4-8853-49d7-8dd3-e391058451c1%40eisentraut.org>
Precedence: bulk

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

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.

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.