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.