Re: SQL:2011 application time

Here are updated patches. I left off the final PERIODs patch because it still has some conflicts
with the new NOT NULL constraints work. I'll soon an update with that soon.

On 11/21/24 04:56, Peter Eisentraut wrote:
>> 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.
>
> Ok, I have committed the fix for this, and I'll continue working through the rest of the patches.

Thanks!

On 11/13/24 02:11, Peter Eisentraut wrote:
> - 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.

Done. The reason I didn't do this before is because we need the opclass, not just the am. I put an
explanation about that into the function comment. If that's a problem I can undo the change.

> 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.

Done.

On 11/13/24 02:50, Peter Eisentraut wrote:
> A quick comment on the patch
>
> v43-0005-Add-UPDATE-DELETE-FOR-PORTION-OF.patch
>
> regarding the code in transformForPortionOfClause() and the additions you made to lsyscache.c:
>
> What you are doing is taking a type OID and a function OID and then converting them back to name and
> namespace and then building a node and then feeding that node back through the parse analysis
> transformation. This all seems quite fishy and cumbersome. I think instead of building a FuncCall
> and transforming it, try to build a FuncExpr directly. Then you wouldn't need these new helper
> functions, which would also reduce the surface area of your patch.

Okay, that makes sense. I changed that for the FOR PORTION OF bounds expression. I still need to get
the rangetype name and namespace to look up its constructor function. Even if I built another FuncExpr
myself there, I don't see any way to get the constructor oid without calling SearchSysCache3(PROCNAMEARGSNSP, ...).
So I'm still doing type oid -> type name -> func oid.

> Additional mini-comment:
>
> #include "utils/rangetypes.h"
>
> in src/include/nodes/execnodes.h appears to be unnecessary (but it is then required in src/backend/
> commands/trigger.c).

Done.

On 11/13/24 02:53, Peter Eisentraut wrote:
> I committed a few fixes in this area today. Has everything here been addressed?

Yes, everything here was addressed in my v41 patches (sent 2024-10-21).

Rebased to 4c4aaa19a6.

Yours,

--
Paul ~{:-)
pj(at)illuminatedcomputing(dot)com