Re: range_agg

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Paul A Jungwirth <pj(at)illuminatedcomputing(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: 2019-12-20 17:43:21
Message-ID: 20191220174321.GA5956@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

I took the liberty of rebasing this series on top of recent branch
master. The first four are mostly Paul's originals, except for conflict
fixes; the rest are changes I'm proposing as I go along figuring out the
whole thing. (I would post just my proposed changes, if it weren't for
the rebasing; apologies for the messiness.)

I am not convinced that adding TYPTYPE_MULTIRANGE is really necessary.
Why can't we just treat those types as TYPTYPE_RANGE and distinguish
them using TYPCATEGORY_MULTIRANGE? That's what we do for arrays. I'll
try to do that next.

I think the algorithm for coming up with the multirange name is
suboptimal. It works fine with the name is short enough that we can add
a few extra letters, but otherwise the result look pretty silly. I
think we can still improve on that. I propose to make
makeUniqueTypeName accept a suffix, and truncate the letters that appear
*before* the suffix rather than truncating after it's been appended.

There's a number of ereport() calls that should become elog(); and a
bunch of others that should probably acquire errcode() and be
reformatted per our style.

Regarding Pavel's documentation markup issue,

> I am not sure how much is correct to use <literallayout class="monospaced">
> in doc. It is used for ranges, and multiranges, but no in other places

I looked at the generated PDF and the table looks pretty bad; the words
in those entries overlap the words in the cell to their right. But that
also happens with entries that do not use <literallayout class="x">!
See [1] for an example of the existing docs being badly formatted. The
docbook documentation [2] seems to suggest that what Paul used is the
appropriate way to do this.

Maybe a way is to make each entry have more than one row -- so the
example would appear below the other three fields in its own row, and
would be able to use the whole width of the table.


Álvaro Herrera
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
v7-0001-multirange-type-basics.patch text/x-diff 139.9 KB
v7-0002-multirange-operators-and-functions.patch text/x-diff 173.0 KB
v7-0003-multirange-pg_dump.patch text/x-diff 17.8 KB
v7-0004-multirange-docs.patch text/x-diff 25.5 KB
v7-0005-Simplify-makeMultirangeConstructors.patch text/x-diff 6.2 KB
v7-0006-silence-compiler-warning.patch text/x-diff 785 bytes
v7-0007-Be-less-verbose-on-variable-names.patch text/x-diff 4.8 KB
v7-0008-Protect-comment-against-pgindent.patch text/x-diff 1.3 KB
v7-0009-pgindent.patch text/x-diff 3.5 KB
v7-0010-fix-typo.patch text/x-diff 733 bytes
v7-0011-minor-style-change.patch text/x-diff 1.0 KB
v7-0012-remove-spurious-newline.patch text/x-diff 753 bytes
v7-0013-rewrite-makeUniqueTypeName.patch text/x-diff 5.9 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-12-20 17:46:34 Re: Optimizing TransactionIdIsCurrentTransactionId()
Previous Message Tom Lane 2019-12-20 17:37:51 Re: Created feature for to_date() conversion using patterns 'YYYY-WW', 'YYYY-WW-D', 'YYYY-MM-W' and 'YYYY-MM-W-D'