Horton

Re: Collation versioning

Thomas Munro <thomas.munro@gmail.com>

View thread View on postgr.es
From: Thomas Munro <thomas.munro@gmail.com>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, Michael Paquier <michael@paquier.xyz>, Robert Haas <robertmhaas@gmail.com>, Laurenz Albe <laurenz.albe@cybertec.at>, Douglas Doole <dougdoole@gmail.com>, Christoph Berg <myon@debian.org>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2020-10-29T12:20:28Z
Lists: pgsql-hackers

Attachments

On Tue, Oct 27, 2020 at 1:34 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
> On Sun, Oct 25, 2020 at 7:13 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > I didn't review all the changes yet, so I'll probably post a deeper
> > review tomorrow.  I'm not opposed to this new approach, as it indeed
> > saves a lot of code.  However, looking at
> > do_collation_version_check(), it seems that you're saving the
> > collation in context->checked_calls even if it didn't raise a WARNING.
> > Since you can now have indexes with dependencies on a same collation
> > with both version tracked and untracked (see for instance
> > icuidx00_val_pattern_where in the regression tests), can't this hide
> > corruption warning reports if the untracked version is found first?
> > That can be easily fixed, so no objection to that approach of course.

Right, fixed.

> I finish looking at the rest of the patches.  I don't have much to
> say, it all looks good and I quite like how much useless code you got
> rid of!

Thanks!  I tested a bunch of permutations[1] of cross-version
pg_update, with and without ICU, with and without libc version
support, and fixed some problems I found in pg_dump:

1.  We need to print OIDs as %u, not %d.  Also, let's use
'%u'::pg_catalog.oid to be consistent with nearby things.

2.  We dump binary_upgrade_set_index_coll_version(<index>, NULL, ...)
to blow away the new cluster's versions before we import the old
versions.  OK, but the function was marked STRICT...

3.  We dump binary_upgrade_set_index_coll_version(<index>,
<collation>, <version>), to import the old cluster's version, where
<collation> is an OID.  OK, but we need the new cluster's OID, not the
old one, so it needs to be an expression like
'pg_catalog."fr_FR"'::regcollation (compare the other references to
collations in the dump, which are all by name).

4.  I didn't really like the use of '' for unknown.  I figured out how
to use NULL for that.

[1] https://github.com/macdice/some_pg_upgrade_tests

Commits

  1. Doc: Document known problem with Windows collation versions.

  2. Add collation versions for FreeBSD.

  3. Tolerate version lookup failure for old style Windows locale names.

  4. Track collation versions for indexes.

  5. Add pg_depend.refobjversion.

  6. Remove pg_collation.collversion.

  7. Fix the MSVC build for versions 2015 and later.

  8. Add collation versions for Windows.

  9. Implement type regcollation

  10. Use libc version as a collation version on glibc systems.

  11. Make type "name" collation-aware.