On Sun, Dec 22, 2019 at 10:51 PM Tom Lane <t...@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
fix_collatable_range-v1.diff
Description: Binary data