On Tue, Nov 11, 2025 at 11:42 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> I have looked at the patch
>
> v59-0004-Add-range_minus_multi-and-multirange_minus_multi.patch
>
> This seems sound in principle.
Thank you for the review! I've attached new patches addressing the
feedback from you and Chao Li. Details below:
> 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?).
Allowing user-defined types is the main motivation. I wanted
ExecForPortionOfLeftovers to avoid type-specific logic, so that users
could use whatever type they like. As you say, spatial types seem like
a natural fit. I'm also interested in using FOR PORTION OF with a
future extension for mdranges ("multi-dimensional ranges"), which
would let people track multiple dimensions of application time. At
least one author (Tom Johnston) refers to this as "assertion time",
where a dimension represents a truth claim about the world. Others
have also expressed interest in "tri-temporal" tables. I think people
could come up with all kinds of interesting ways to use this feature.
So we need a function that takes the existing row's value (in some
type T) and subtracts the value targeted by the update/delete. It
needs to return zero or more Ts, one for each temporal leftover. It
can't return an array of Ts, because anyrange doesn't work that way.
(Likewise anymultirange.) Given a function with an anyrange argument
and an anyarray return value, Postgres expects an array of the range's
*base type*. In other words we can do this:
array<T> minus_multi<T>(range<T> r1, range<T> r2)
but not this:
array<T> minus_multi<T where T is rangetype>(T r1, T r2)
But what I want *is* possible as a set-returning function. Because
then the signature is just `anyrange f(anyrange, anyrange)`.
> 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.
I think you are misunderstanding. The curly braces are just the
multirange string notation, not a set. (Mathematically a multirange is
a set though.) The function is still a Set-Returning Function, to
match the interface we want, but it never needs to return more than
one row, because a single multirange can always accommodate the result
of mr1 - mr2 (unlike with range types). Note it can *also* return zero
rows, if the result would be empty. (There are examples of this in the
regress tests.) Each row from these SRFs becomes an INSERTed temporal
leftover in ExecForPortionOfLeftovers. Multiranges can insert zero or
one. Ranges can insert up to two. A user-defined type might insert
more.
> A few other minor details:
>
> * src/backend/utils/adt/rangetypes.c
>
> +#include "utils/array.h"
>
> seems to be unused.
You're right; removed.
> + 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.
Okay.
> 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.
Done, with some extra word-smithing.
> * 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.)
Right, rangetypes are prorows 2 and multiranges are prorows 1.
I'll reply to Chao Li separately, but those changes are included in
the patches here.
Rebased to 705601c5ae.
Yours,
--
Paul ~{:-)
pj@illuminatedcomputing.com