On Sun, Dec 22, 2019 at 10:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
> > Now as far as point 1 goes, I think it's not really that awful to use
> > CheckAttributeType() with a dummy attribute name. The attached
> > incomplete patch uses "partition key" which causes it to emit errors
> > like
> > regression=# create table fool (a int, b int) partition by list ((row(a, b)));
> > ERROR: column "partition key" has pseudo-type record
> > I don't think that that's unacceptable. But if we wanted to improve it,
> > we could imagine adding another flag, say CHKATYPE_IS_PARTITION_KEY,
> > that doesn't affect CheckAttributeType's semantics, just the wording of
> > the error messages it throws.
>
> Here's a fleshed-out patch that does it like that.
>
> While poking at this, I also started to wonder why CheckAttributeType
> wasn't recursing into ranges, since those are our other kind of
> container type. And the answer is that it must, because we allow
> creation of ranges over composite types:
>
> regression=# create table foo (f1 int, f2 int);
> CREATE TABLE
> regression=# create type foorange as range (subtype = foo);
> CREATE TYPE
> regression=# alter table foo add column r foorange;
> ALTER TABLE
>
> Simple things still work on table foo, but surely this is exactly
> what CheckAttributeType is supposed to be preventing. With the
> second attached patch you get
>
> regression=# alter table foo add column r foorange;
> ERROR: composite type foo cannot be made a member of itself
>
> The second patch needs to go back all the way, the first one
> only as far as we have partitions.
While working on regression tests for index collation versioning [1],
I noticed that the 2nd patch apparently broke the ability to create a
table using a range over collatable datatype attribute, which we
apparently don't test anywhere. Simple example to reproduce:
CREATE TYPE myrange_text AS range (subtype = text);
CREATE TABLE test_text(
meh myrange_text
);
ERROR: 42P16: no collation was derived for column "meh" with
collatable type text
HINT: Use the COLLATE clause to set the collation explicitly.
AFAICT, this is only a thinko in CheckAttributeType(), where the range
collation should be provided rather than the original tuple desc one,
as per attached. I also added a create/drop table in an existing
regression test that was already creating range over collatable type.
[1] https://www.postgresql.org/message-id/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com
