Re: range_agg

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: range_agg
Date: 2019-11-22 16:57:12
Message-ID: CAFj8pRCF0je4dzJoin8RL9xJA-8fFKODmBuhrc90TtQZ+61s7w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

pá 22. 11. 2019 v 17:25 odesílatel Paul A Jungwirth <
pj(at)illuminatedcomputing(dot)com> napsal:

> On Thu, Nov 21, 2019 at 9:21 PM Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ranier Vilela 2019-11-22 17:19:11 [PATCH][BUG FIX] Pointer arithmetic with NULL
Previous Message Ranier Vilela 2019-11-22 16:50:45 [PATCH][BUG FIX] Uninitialized variable parsed