Horton

Re: Collation versioning

Julien Rouhaud <rjuju123@gmail.com>

View thread View on postgr.es
From: Julien Rouhaud <rjuju123@gmail.com>
To: Thomas Munro <thomas.munro@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-26T12:33:58Z
Lists: pgsql-hackers 
On Sun, Oct 25, 2020 at 7:13 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> On Sun, Oct 25, 2020 at 10:36 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> >
> > On Fri, Oct 23, 2020 at 2:07 AM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > > While reviewing the changes, I found a couple of minor issues
> > > (inherited from the previous versions).  It's missing a
> > > free_objects_addresses in recordDependencyOnCollations, and there's a
> > > small typo.  Inline diff:
> >
> > Thanks, fixed.
> >
> > I spent the past few days trying to simplify this patch further.
> > Here's what I came up with:
>
> Thanks!
>
> >
> > 1.  Drop the function dependencyExists() and related code, which in
> > earlier versions tried to avoid creating duplicate pg_depends rows by
> > merging with existing rows.  This was rather complicated, and there
> > are not many duplicates anyway, and it's easier to suppress duplicate
> > warnings at warning time (see do_collation_version_check()).  (I'm not
> > against working harder to make pg_depend rows unique, but it's not
> > required for this and I didn't like that way of doing it.)
>
> 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.

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!

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.