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: | 2020-01-10 09:38:21 |
Message-ID: | CAFj8pRDn-f9-2jbUfR4XpVXiTwfcGo5CKYk8y2ktzdm64vVmTg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
>
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2020-01-10 09:50:12 | Re: Add pg_file_sync() to adminpack |
Previous Message | Konstantin Knizhnik | 2020-01-10 08:47:42 | Re: [Proposal] Global temporary tables |