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
