Horton

Re: unsupportable composite type partition keys

Julien Rouhaud <rjuju123@gmail.com>

View thread View on postgr.es
From: Julien Rouhaud <rjuju123@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Amit Langote <amitlangote09@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-01-31T10:25:07Z
Lists: pgsql-hackers

Attachments

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

Commits

  1. Fix CheckAttributeType's handling of collations for ranges.

  2. Improve comments in utils/rel.h.

  3. Allow whole-row Vars to be used in partitioning expressions.

  4. Remove equalPartitionDescs().

  5. Load relcache entries' partitioning data on-demand, not immediately.

  6. Disallow partition key expressions that return pseudo-types.

  7. Prevent a rowtype from being included in itself via a range.