public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Eisentraut <[email protected]>
To: Paul A Jungwirth <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: SQL:2011 Application Time Update & Delete
Date: Wed, 12 Nov 2025 08:42:07 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+renyUodzxAvMnpa_LTvo+Ru1ZKH+Su8KaPvD4iMtguFKzq4g@mail.gmail.com>
References: <[email protected]>
<CA+renyW-S0LyG0E4qxFvnKNKsgq_6WWeTStOXHpjCvwj6LKS6Q@mail.gmail.com>
<CA+renyXXJJCmgG0Wdf89JgVNAeRKkVn+EuZGTf4Ph-BWoJafQA@mail.gmail.com>
<CA+renyWA-b00qvz4gDdPbPBjVatacB1T5v7SJc6J_xg3R6-qRw@mail.gmail.com>
<CA+renyVYnV9rGDiUhTQEy8r6gx6Xg-+OEo-2DK7JbuBXcLVq5A@mail.gmail.com>
<CA+renyWcNBdnaW4zc9S03aN+fEbVVB1S+q8e9MEjhM2YB+kkiw@mail.gmail.com>
<CA+renyUiEu2yTHk+Vy-Rt2xA5Vnp2N_ohN=npiNmMKTh53qDfA@mail.gmail.com>
<[email protected]>
<CA+renyW7ZB_k9AgmSFJU2EegL9r1k1sgWo4-9tGGkgwxNqe6kw@mail.gmail.com>
<CA+renyUodzxAvMnpa_LTvo+Ru1ZKH+Su8KaPvD4iMtguFKzq4g@mail.gmail.com>
I have looked at the patch
v59-0004-Add-range_minus_multi-and-multirange_minus_multi.patch
This seems sound in principle.
Perhaps you could restate why you chose a set-returning function rather
than (what I suppose would be the other options) returning multirange or
an array of ranges. (I don't necessarily disagree, but it would be good
to be clear for everyone.) The point about allowing user-defined types
makes sense (but for example, I see types like multipolygon and
multipoint in postgis, so maybe those could also work?).
That said, I think there is a problem in your implementation. Note that
the added regression test cases for range return multiple rows but the
ones for multirange all return a single row with a set {....} value. I
think the problem is that your multirange_minus_multi() calls
multirange_minus_internal() which already returns a set, and you are
packing that set result into a single row.
A few other minor details:
* src/backend/utils/adt/rangetypes.c
+#include "utils/array.h"
seems to be unused.
+ typedef struct
+ {
+ RangeType *rs[2];
+ int n;
+ } range_minus_multi_fctx;
This could be written just as a struct, like
struct range_minus_multi_fctx
{
...
};
Wrapping it in a typedef doesn't achieve any additional useful
abstraction.
The code comment before range_minus_multi_internal() could first
explain briefly what the function does before going into the details
of the arguments. Because we can't assume that someone will have read
the descriptions of the higher-level functions first.
* src/include/catalog/pg_proc.dat
The prorows values for the two new functions should be the same?
(I suppose they are correct now seeing your implementation of
multirange_minus_multi(), but I'm not sure that was intended, as
discussed above.)
view thread (52+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: SQL:2011 Application Time Update & Delete
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox