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