Re: Collation versioning

From: Julien Rouhaud <rjuju123(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, Douglas Doole <dougdoole(at)gmail(dot)com>, Christoph Berg <myon(at)debian(dot)org>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Collation versioning
Date: 2020-03-17 10:42:34
Message-ID: 20200317104234.GA89737@nol
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 17, 2020 at 08:19:30AM +0100, Julien Rouhaud wrote:
> On Tue, Mar 17, 2020 at 03:37:49PM +0900, Michael Paquier wrote:
> > On Mon, Mar 16, 2020 at 03:05:20PM +0100, Julien Rouhaud wrote:
> > > On Mon, Mar 16, 2020 at 04:57:38PM +0900, Michael Paquier wrote:
>
> > >> Regarding patch 0003, it would be nice to include some tests
> > >> independent on the rest and making use of the new functions. These
> > >> normally go in regproc.sql. For example with a collation that needs
> > >> double quotes as this is not obvious:
> > >> =# select regcollation('"POSIX"');
> > >> regcollation
> > >> --------------
> > >> "POSIX"
> > >> (1 row)
> > >>
> > >> On top of that, this needs tests with to_regcollation() and tests with
> > >> schema-qualified collations.
> > >
> > >
> > > Done too, using the same collation name, for both with and without schema
> > > qualification.
> >
> > It seems to me that you could add an extra test with a catalog that
> > does not exist, making sure that NULL is returned:
> > SELECT to_regtype('ng_catalog."POSIX"');
>
>
> Agreed, I'll add that, but using a name that looks less like a typo :)

Tests added, including one with an error output, as the not existing schema
doesn't reveal the encoding.

> > Note that patch 0002 fails to compile because it is missing to include
> > utils/builtins.h for CStringGetTextDatum(), and that you cannot pass
> > down a NameData to this API, because it needs a simple char string or
> > you would need NameStr() or such. Anyway, it happens that you don't
> > need recordDependencyOnVersion() at all, because it is removed by
> > patch 0004 in the set, so you could just let it go.
>
>
> Ah good catch, I missed that during the NameData/text refactoring. I'll fix it
> anyway, better to have clean history.

And this should be fixed too.

Attachment Content-Type Size
v16-0001-Remove-pg_collation.collversion.patch text/plain 26.8 KB
v16-0002-Add-pg_depend.refobjversion.patch text/plain 12.2 KB
v16-0003-Implement-type-regcollation.patch text/plain 18.8 KB
v16-0004-Track-collation-versions-for-indexes.patch text/plain 61.7 KB
v16-0005-Preserve-index-dependencies-on-collation-during-.patch text/plain 38.7 KB
v16-0006-Add-ALTER-INDEX-.-ALTER-COLLATION-.-REFRESH-VERS.patch text/plain 14.6 KB
v16-0007-doc-Add-Collation-Versions-section.patch text/plain 2.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2020-03-17 10:59:14 Re: Improving connection scalability: GetSnapshotData()
Previous Message Dmitry Dolgov 2020-03-17 10:26:27 Re: [HACKERS] [PATCH] Generic type subscripting