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-16 14:05:20
Message-ID: 20200316140520.GA21497@nol
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Mar 16, 2020 at 04:57:38PM +0900, Michael Paquier wrote:
> On Thu, Mar 12, 2020 at 03:00:26PM +0100, Julien Rouhaud wrote:
> > And v15 due to conflict with b08dee24a5 (Add pg_dump support for ALTER obj
> > DEPENDS ON EXTENSION).
>
> I have looked at patches 0001~0003 in the set for now.

Thanks!

> In patch 0002, you have the following addition:
> @@ -103,9 +103,10 @@ ORDER BY 1, 2;
> pg_class | relacl | aclitem[]
> pg_class | reloptions | text[]
> pg_class | relpartbound | pg_node_tree
> + pg_depend | refobjversion | text
> This comes from a regression test doing a sanity check to look for
> catalogs which have a toastable column but no toast tables. As an
> exception, it should be documented in the test's comment. Actually,
> does it need to be an exception? This does not depend on
> relation-level facilities so there should be no risk of recursive
> dependencies, though I have not looked in details at this part.

I totally missed that, and I agree that there's no need for an exception, so
fixed.

> + <para>
> + The only current use of <structfield>refobjversion</structfield> is to
> + record dependencies between indexes and collation versions.
> + </para>
> [...]
> + <row>
> + <entry><structfield>refobjversion</structfield></entry>
> + <entry><type>text</type></entry>
> + <entry></entry>
> + <entry>
> + An optional version for the referenced object; see text
> + </entry>
> + </row>
> Couldn't you merge both paragraphs here?

Done.

> 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.

> Documentation for to_regcollation() is missing. And it looks that
> many parts of the documentation are missing an update. One example in
> datatype.sgml:
> Type <type>oid</type> represents an object identifier. There are also
> several alias types for <type>oid</type>: <type>regproc</type>,
> <type>regprocedure</type>, <type>regoper</type>, <type>regoperator</type>,
> <type>regclass</type>, <type>regtype</type>, <type>regrole</type>,
> <type>regnamespace</type>, <type>regconfig</type>, and
> <type>regdictionary</type>. <xref linkend="datatype-oid-table"/> shows an
> overview.
> At quick glance, there are more sections in need of a refresh..

Indeed. I found missing reference in datatype.sgml; func.sgml and
pgupgrade.sgml.

v16 attached.

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.8 KB
v16-0003-Implement-type-regcollation.patch text/plain 17.7 KB
v16-0004-Track-collation-versions-for-indexes.patch text/plain 62.4 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 Michail Nikolaev 2020-03-16 14:07:53 [PATCH] Btree BackwardScan race condition on Standby during VACUUM
Previous Message David Steele 2020-03-16 13:58:27 Re: row filtering for logical replication