Re: SQL:2011 application time - Mailing list pgsql-hackers

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              ~{:-)
pj@illuminatedcomputing.com