Subject: Re: range_agg
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>,
 Jeff Davis <pgsql@j-davis.com>, Pavel Stehule <pavel.stehule@gmail.com>,
 David Fetter <david@fetter.org>
References: <20200323233227.GA5607@alvherre.pgsql>
From: Paul Jungwirth <pj@illuminatedcomputing.com>
Message-ID: <1d40004c-335a-a268-454e-6864e188c852@illuminatedcomputing.com>
Date: Mon, 23 Mar 2020 21:23:31 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.2.2
MIME-Version: 1.0
In-Reply-To: <20200323233227.GA5607@alvherre.pgsql>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

Thanks Alvaro!

On Mon, Mar 23, 2020 at 4:33 PM Alvaro Herrera 
<alvherre@2ndquadrant.com> wrote:
 >
 > Thinking about the on-disk representation, can we do better than putting
 > the contained ranges in long-varlena format, including padding; also we
 > include the type OID with each element.  Sounds wasteful.  A more
 > compact representation might be to allow short varlenas and doing away
 > with the alignment padding, put the the type OID just once.  This is
 > important because we cannot change it later.

Can you give me some guidance on this? I don't know how to make the 
on-disk format different from the in-memory format. (And for the 
in-memory format, I think it's important to have actual RangeTypes 
inside the multirange.) Is there something in the documentation, or a 
README in the repo, or even another type I can follow?

 > I'm also wondering if multirange_in() is the right strategy.  Would 
it> be sensible to give each range to range_parse or range_parse_bounde, so
 > that it determines where each range starts and ends?  Then that function
 > doesn't have to worry about each quote and escape, duplicating range
 > parsing code.  (This will probably require changing signature of the
 > rangetypes.c function, and exporting it; for example have
 > range_parse_bound allow bound_str to be NULL and in that case don't mess
 > with the StringInfo and just return the end position of the parsed
 > bound.)

Yeah, I really wanted to do it that way originally too. As you say it 
would require passing back more information from the range-parsing code. 
I can take a stab at making the necessary changes. I'm a bit more 
confident now than I was then in changing the range code we have already.

Regards,

-- 
Paul              ~{:-)
pj@illuminatedcomputing.com