Subject: Re: Collation versioning
To: Julien Rouhaud <rjuju123@gmail.com>, Thomas Munro <thomas.munro@gmail.com>
References: 
 <CAOBaU_b9V502Mw2x0_+=-GT_Q26bdsuiA=8ZzqRhd_4Gi3G5aQ@mail.gmail.com>
 <20191108013654.GA1768@paquier.xyz>
 <CA+hUKGKa_ZKbWVLwH_O0MvLQMHRjTpHOR-9Y_7XCm-OpXLVkcQ@mail.gmail.com>
 <3c3b9ff84d21acf3188558928249d04db84ea2e9.camel@cybertec.at>
 <CA+hUKGLnzJP+O_4Xj2s6gQy+2QjX=+nJHP8Vxfd6tnRWJbWjAg@mail.gmail.com>
 <CA+TgmoZSCjinh6Sfoofp2tfweFvQxXoCUNv+dyJnS6PzNGv_LA@mail.gmail.com>
 <20191128045041.GW237562@paquier.xyz>
 <CAOBaU_ZC0ynR2s8mt72wyfmDu78z__hYmcGm25e=z6Dgh4Fvag@mail.gmail.com>
 <CA+TgmoajjpPzd9KByrzjfmikgvnhsb9mCQp5=7XReqojcrz9pQ@mail.gmail.com>
 <CAOBaU_ZmqPCEmoT6J-=nLqi6n9xvwQAAusNx2gK-qwRUGokbsQ@mail.gmail.com>
 <CAOBaU_Y4f=scCxv675nv0Ef30z5Vvbh8JFRhibojfhA9_qB8Lw@mail.gmail.com>
 <CAOBaU_ap+4uEgZV548NEgxzLTfqVsTdSy0+9S9wp32qYvXo2BA@mail.gmail.com>
 <CA+hUKGK8CwBcTcXWL2kUjpHT+6t2hEFCzkcZ-Z7xXbz=C4NLCQ@mail.gmail.com>
 <CAOBaU_Zg==075nNEhd+Du0x32q0ojHC1DJ86mBCW5JYKaosb5Q@mail.gmail.com>
 <CAOBaU_bP70ybzc23E=Z38mygMAOThPG7Fog1Qoom5DNvsfq=Ng@mail.gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Michael Paquier
 <michael@paquier.xyz>, Laurenz Albe <laurenz.albe@cybertec.at>,
 Douglas Doole <dougdoole@gmail.com>, Christoph Berg <myon@debian.org>,
 Pg Hackers <pgsql-hackers@postgresql.org>
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Organization: 2ndQuadrant
Message-ID: <3ede40e7-5799-7096-dc5f-d7beda8e7145@2ndquadrant.com>
Date: Tue, 28 Jan 2020 13:06:06 +0100
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:52.0)
 Gecko/20100101 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: 
 <CAOBaU_bP70ybzc23E=Z38mygMAOThPG7Fog1Qoom5DNvsfq=Ng@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2019-12-17 14:25, Julien Rouhaud wrote:
> PFA rebased v6, with the following changes:

Some thoughts on this patch set.

I think we are all agreed on the general idea.

0001-0003 seem pretty much OK.  Why is pg_depend.refobjversion of type 
"name" whereas the previous pg_collation.collversion was type "text"? 
Related to that, we previously used null to indicate an unknown 
collation version, and now it's an empty string.

Also, this would limit collation versions to 63 characters.  Perhaps not 
a problem right now, but if someone wants to implement Thomas's previous 
md5-the-file idea with sha256, we'll run out of space.

For 0005, if the new commands are only to be used in binary upgrades, 
then they should be implemented as built-in functions instead of full 
DDL commands.  There is precedent for that.

The documentation snippet for this patch talks about upgrades from PG12 
to later.  But what about upgrades on platforms where we currently don't 
have collation versioning but might introduce it later (FreeBSD, 
Windows)?  This needs to be generalized.

For 0006 ("Add a new ALTER INDEX name DEPENDS ON COLLATION name
  CURRENT VERSION"), I find the syntax misleading.  This command doesn't 
(or shouldn't) add a new dependency, it only changes the version of an 
existing dependency.  The previously used syntax ALTER COLLATION / 
REFRESH VERSION is a better vocabulary, I think.

I think this whole thing needs more tests.  We are designing this 
delicate mechanism but have no real tests for it.  We at least need to 
come up with a framework for how to test this automatically, so that we 
can add more test cases over time as people will invariably report 
issues when this hits the real world.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services