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.
