Hi all,
I have just committed a fix for a crash with the handling of partition
bounds using column references which has been discussed here:
https://www.postgresql.org/message-id/15668-0377b1981aa1a393@postgresql.org
And while discussing on the matter with Amit, the point has been
raised that default expressions with column references can lead to
some funny error messages with the context. For example, take that
with an undefined column:
=# create table foo (a int default (a.a));
ERROR: 42P01: missing FROM-clause entry for table "a"
LINE 1: create table foo (a int default (a.a));
This confusion is old I think, and reproduces down to 9.4 and older.
If using directly a reference from a column's table then things get
correct:
=# create table foo (a int default (foo.a));
ERROR: 42P10: cannot use column references in default expression
LOCATION: cookDefault, heap.c:2948
=# create table foo (a int default (a));
ERROR: 42P10: cannot use column references in default expression
LOCATION: cookDefault, heap.c:2948
We have the same problem for partition bounds actually, which is new
as v12 as partition bound expressions now use the common expression
machinery for transformation:
=# CREATE TABLE list_parted (a int) PARTITION BY LIST (a);
CREATE TABLE
=# CREATE TABLE part_list_crash PARTITION OF list_parted
FOR VALUES IN (somename.somename);
ERROR: 42P01: missing FROM-clause entry for table "somename"
LINE 2: FOR VALUES IN (somename.somename)
One idea which came from Amit, and it seems to me that it is a good
idea, would be to have more context-related error messages directly in
transformColumnRef(), so as we can discard at an early stage column
references which are part of contexts where there is no meaning to
have them. The inconsistent part in HEAD is that cookDefault() and
transformPartitionBoundValue() already discard column references, so
if we move those checks at transformation phase we can simplify the
error handling post-transformation. This would make the whole thing
more consistent.
While this takes care of the RTE issues, this has a downside though.
Take for example this case using an expression with an aggregate and
a column reference:
=# CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted
FOR VALUES IN (sum(a));
-ERROR: aggregate functions are not allowed in partition bound
+ERROR: cannot use column reference in partition bound expression
So this would mean that we would first complain of the most inner
parts of the expression, which is more intuitive actually in my
opinion. The difference can be seen using the patch attached for
partition bounds, as I have added more test coverage with a previous
commit. We also don't have much tests in the code for default
expression patterns, so I have added some.
The docs of CREATE TABLE also look incorrect to me when it comes to
default expressions. It says the following: "other columns in the
current table are not allowed". However *all* columns are not
authorized, including the column which uses the expression.
Thoughts?
--
Michael
