Re: range_agg - Mailing list pgsql-hackers

From Pavel Stehule
Subject Re: range_agg
Date
Msg-id CAFj8pRCF0je4dzJoin8RL9xJA-8fFKODmBuhrc90TtQZ+61s7w@mail.gmail.com
Whole thread Raw
In response to Re: range_agg  (Paul A Jungwirth <pj@illuminatedcomputing.com>)
List pgsql-hackers


pá 22. 11. 2019 v 17:25 odesílatel Paul A Jungwirth <pj@illuminatedcomputing.com> napsal:
On Thu, Nov 21, 2019 at 9:21 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
> I though about it, and I think so cast from multirange to range is useless, minimally it should be explicit.

I agree: definitely not implicit. If I think of a good reason for it
I'll add it, but otherwise I'll leave it out.

> On second hand - from range to multirange should be implicit.

Okay.

> The original patch did
>
> 1. MR @x MR = MR
> 2. R @x R = MR
> 3. MR @x R = MR
>
> I think so @1 & @3 has sense, but without introduction of special operator. @2 is bad and can be solved by cast one or second operand.

Yes. I like how #2 follows the int/numeric analogy: if you want a
numeric result from `int / int` you can say `int::numeric / int`.

So my understanding is that conventionally cast functions are named
after the destination type, e.g. int8multirange(int8range) would be
the function to cast an int8range to an int8multirange. And
int8range(int8multirange) would go the other way (if we do that). We
already use these names for the "constructor" functions, but I think
that is actually okay. For the multirange->range cast, the parameter
type & number are different, so there is no real conflict. For the
range->multirange cast, the parameter type is the same, and the
constructor function is variadic---but I think that's fine, because
the semantics are the same: build a multirange whose only element is
the given range:

regression=# select int8multirange(int8range(1,2));
 int8multirange
----------------
 {[1,2)}
(1 row)

Even the NULL handling is already what we want:

regression=# select int8multirange(null);
 int8multirange
----------------
 NULL
(1 row)

So I think it's fine, but I'm curious whether you see any problems
there? (I guess if there is a problem it's no big deal to name the
function something else....)

It looks well now. I am not sure about benefit of cast from MR to R if MR has more than one values. But it can be there for completeness.

I think in this moment is not important to implement all functionality - for start is good to implement basic functionality that can be good. It can be enhanced step by step in next versions.

Pavel

Thanks,
Paul

pgsql-hackers by date:

Previous
From: Ranier Vilela
Date:
Subject: [PATCH][BUG FIX] Uninitialized variable parsed
Next
From: Ranier Vilela
Date:
Subject: [PATCH][BUG FIX] Pointer arithmetic with NULL