Re: range_agg

Hi

so 4. 1. 2020 v 6:29 odesílatel Paul A Jungwirth <
pj(at)illuminatedcomputing(dot)com> napsal:

> On Fri, Dec 20, 2019 at 11:29 AM Alvaro Herrera
> <alvherre(at)2ndquadrant(dot)com> wrote:
> > > Should I just give up on implicit casts and require you to specify
> > > one? That makes it a little more annoying to mix range & multirange
> > > types, but personally I'm okay with that. This is my preferred
> > > approach.
> >
> > +1
>
> Here is a patch adding the casts, rebasing on the latest master, and
> incorporating Alvaro's changes. Per his earlier suggestion I combined
> it all into one patch file, which also makes it easier to apply
> rebases & updates.
>

This patch was applied cleanly and all tests passed

>
> My work on adding casts also removes the @+ / @- / @* operators and
> adds + / - / * operators where both parameters are multiranges. I
> retained other operators with mixed range/multirange parameters, both
> because there are already range operators with mixed range/scalar
> parameters (e.g. <@), and because it seemed like the objection to @+ /
> @- / @* was not mixed parameters per se, but rather their
> unguessability. Since the other operators are the same as the existing
> range operators, they don't share that problem.
>

looks well

>
> This still leaves the question of how best to format the docs for
> these operators. I like being able to combine all the <@ variations
> (e.g.) into one table row, but if that is too ugly I could give them
> separate rows instead. Giving them all their own row consumes a lot of
> vertical space though, and to me that makes the docs more tedious to
> read & browse, so it's harder to grasp all the available range-related
> operations at a glance.
>

I have similar opinion - maybe is better do documentation for range and
multirange separately. Sometimes there are still removed operators @+

> I'm skeptical of changing pg_type.typtype from 'm' to 'r'. A
> multirange isn't a range, so why should we give it the same type? Also
> won't this break any queries that are using that column to find range
> types? What is the motivation to use the same typtype for both ranges
> and multiranges? (There is plenty I don't understand here, e.g. why we
> have both typtype and typcategory, so maybe there is a good reason I'm
> missing.)
>

If you can share TYPTYPE_RANGE in code for multiranges, then it should be
'r'. If not, then it needs own value.

> I experimented with setting pg_type.typelem to the multirange's range
> type, but it seemed to break a lot of things, and reading the code I
> saw some places that treat a non-zero typelem as synonymous with being
> an array. So I'm reluctant to make this change also, especially when
> it is just as easy to query pg_range to get a multirange's range type.
>

ok, it is unhappy, but it is true. This note should be somewhere in code,
please

> Also range types themselves don't set typelem to their base type, and
> it seems like we'd want to treat ranges and multiranges the same way
> here.
>
> Alvaro also suggested renaming pg_range.mltrngtypid to
> pg_range.rngmultitypid, so it shares the same "rng" prefix as the
> other columns in this table. Having a different prefix does stand out.
> I've included that change in this patch too.
>

Personally I have not any comments to implemented functionality.

>
> Yours,
> Paul
>