Re: range_agg

From: Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: range_agg
Date: 2020-01-04 05:29:33
Message-ID: CA+renyWmmrTcyxoCozqynkpG82XDg_W5NHKLztADKkzcTLZmmA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

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.

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'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.)

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.
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.

Yours,
Paul

Attachment Content-Type Size
v8-multirange.patch application/octet-stream 330.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-01-04 05:30:09 Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
Previous Message Amit Kapila 2020-01-04 04:30:26 Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions