Re: Collation versioning

On Wed, Nov 13, 2019 at 3:27 AM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
> On Sun, Nov 10, 2019 at 10:08 AM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> > That's because the 0003 patch only calls recordDependencyOnVersion()
> > for simple attribute references. When
> > recordDependencyOnSingleRelExpr() is called by index_create() to
> > analyse ii_Expressions and ii_Predicate, it's going to have to be
> > smart enough to detect collation references and record the versions.
> > There is also some more code that ignores pinned collations hiding in
> > there.
> >
> > This leads to the difficult question of how you recognise a real
> > dependency on a collation's version in an expression. I have some
> > vague ideas but haven't seriously looked into it yet. (The same
> > question comes up for check constraint -> collation dependencies.)
>
> Oh good point. A simple and exhaustive way to deal with that would be
> to teach recordMultipleDependencies() to override isObjectPinned() and
> retrieve the collation version if the referenced object is a collation
> and it's neither C or POSIX collation. It means that we could also
> remove the extra "version" argument and get rid of
> recordDependencyOnVersion to simply call recordMultipleDependencies in
> create_index for direct column references having a collation.
>
> Would it be ok to add this kind of knowledge in
> recordMultipleDependencies() or is it too hacky?

That doesn't seem like the right place; that's a raw data insertion
function, though... I guess it does already have enough brains to skip
pinned objects. Hmm.

> > I think create_index() will need to perform recursive analysis on
> > composite types to look for text attributes, when they appear as
> > simple attributes, and then add direct dependencies index -> collation
> > to capture the versions. Then we might need to do the same for
> > composite types hiding inside ii_Expressions and ii_Predicate (once we
> > figure out what that really means).
>
> Isn't that actually a bug? For instance such an index will have a 0
> indcollation in pg_index, and according to pg_index documentation:
>
> " this contains the OID of the collation to use for the index, or zero
> if the column is not of a collatable data type."
>
> You can't use a COLLATE expression on such data type, but it still has
> a collation used.

I don't think it's a bug. The information is available, but you have
to follow the graph to get it. Considering that the composite type
could be something like CREATE TYPE my_type AS (fr_name text COLLATE
"fr_CA", en_name text COLLATE "en_CA"), there is no single collation
you could put into pg_index.indcollation anyway. As for pg_depend,
it's currently enough for the index to depend on the type, and the
type to depend on the collation, because the purpose of dependencies
is to control dropping and dumping order, but for our new purpose we
also need to create direct dependencies index -> "fr_CA", index ->
"en_CA" so we can record the current versions.

> > 3. Test t/002_pg_dump.pl in src/bin/pg_upgrade fails.
>
> Apparently neither "make check" nor "make world" run this test :(
> This was broken due collversion support in pg_dump, I have fixed it
> locally.

make check-world