Bogus collation version recording in recordMultipleDependencies

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Bogus collation version recording in recordMultipleDependencies
Date: 2021-04-14 17:18:07
Message-ID: 3564817.1618420687@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I noticed some broken-looking logic in recordMultipleDependencies
concerning how it records collation versions. It was a bit harder
than I expected to demonstrate the bugs, but I eventually succeeded
with

u8=# create function foo(varchar) returns bool language sql return false;
CREATE FUNCTION
u8=# create collation mycoll from "en_US";
CREATE COLLATION
u8=# CREATE DOMAIN d4 AS character varying(3) COLLATE "aa_DJ"
CONSTRAINT yes_or_no_check CHECK (value = 'YES' collate mycoll or foo(value));
CREATE DOMAIN
u8=# select objid, pg_describe_object(classid,objid,objsubid) as obj, pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype, refobjversion from pg_depend where objid = 'd4'::regtype;
objid | obj | ref | deptype | refobjversion
-------+---------+-------------------+---------+---------------
37421 | type d4 | schema public | n |
37421 | type d4 | collation "aa_DJ" | n |
(2 rows)

u8=# select objid, pg_describe_object(classid,objid,objsubid) as obj, pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype, refobjversion from pg_depend where refobjid = 'd4'::regtype;
objid | obj | ref | deptype | refobjversion
-------+----------------------------+---------+---------+---------------
37420 | type d4[] | type d4 | i |
37422 | constraint yes_or_no_check | type d4 | a |
(2 rows)

u8=# select objid, pg_describe_object(classid,objid,objsubid) as obj, pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype, refobjversion from pg_depend where objid = 37422;
objid | obj | ref | deptype | refobjversion
-------+----------------------------+---------------------------------+---------+---------------
37422 | constraint yes_or_no_check | type d4 | a |
37422 | constraint yes_or_no_check | collation mycoll | n | 2.28
37422 | constraint yes_or_no_check | function foo(character varying) | n | 2.28
37422 | constraint yes_or_no_check | collation "default" | n |
(4 rows)

(This is in a glibc-based build, with C as the database's default
collation.)

One question here is whether it's correct that the domain's dependency
on collation "aa_DJ" is unversioned. Maybe that's intentional, but it
seems worth asking.

Anyway, there are two pretty obvious bugs in the dependencies for the
domain's CHECK constraint: the version for collation mycoll leaks
into the entry for function foo, and an entirely useless (because
unversioned) dependency is recorded on the default collation.

... well, it's almost entirely useless. If we fix things to not do that
(as per patch 0001 below), the results of the create_index regression
test become unstable, because there's two queries that inquire into the
dependencies of indexes, and their results change depending on whether
the default collation has a version or not. I'd be inclined to just
take out the portions of that test that depend on that question, but
maybe somebody will complain that there's a loss of useful coverage.
I don't agree, but maybe I'll be overruled.

If we do feel we need to stay bug-compatible with that behavior, then
the alternate 0002 patch just fixes the version-leakage-across-entries
problem, while still removing the unnecessary assumption that C, POSIX,
and DEFAULT are the only pinned collations.

(To be clear: 0002 passes check-world as-is, while 0001 is not
committable without some regression-test fiddling.)

Thoughts?

regards, tom lane

Attachment Content-Type Size
0001-fix-collation-version-dependencies.patch text/x-diff 1.8 KB
0002-partial-fix-collation-version-dependencies.patch text/x-diff 1.8 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Mark Dilger 2021-04-14 17:18:30 Re: Converting contrib SQL functions to new style
Previous Message Robert Haas 2021-04-14 17:17:33 Re: pg_amcheck contrib application