Disallow partition key expressions that return pseudo-types.

This wasn't checked originally, but it should have been, because
in general pseudo-types can't be stored to and retrieved from disk.
Notably, partition bound values of type "record" would not be
interpretable by another session.

In v12 and HEAD, add another flag to CheckAttributeType's repertoire
so that it can produce a specific error message for this case.  That's
infeasible in older branches without an ABI break, so fall back to
a slightly-less-nicely-worded error message in v10 and v11.

Problem noted by Amit Langote, though this patch is not his initial
solution.  Back-patch to v10 where partitioning was introduced.

Discussion: https://postgr.es/m/CA+HiwqFUzjfj9HEsJtYWcr1SgQ_=iCAvQ=O2Sx6aQxoDu4OiHw@mail.gmail.com

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 32dceb72786a683022c84801188d279ea1941966..275c19dac3f573ad5c57671db7a4bd83af67e009 100644 (file)
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -573,6 +573,10 @@ CheckAttributeNamesTypes(TupleDesc tupdesc, char relkind,
  * are reliably identifiable only within a session, since the identity info
  * may use a typmod that is only locally assigned.  The caller is expected
  * to know whether these cases are safe.)
+ *
+ * flags can also control the phrasing of the error messages.  If
+ * CHKATYPE_IS_PARTKEY is specified, "attname" should be a partition key
+ * column number as text, not a real column name.
  * --------------------------------
  */
 void
@@ -599,10 +603,19 @@ CheckAttributeType(const char *attname,
        if (!((atttypid == ANYARRAYOID && (flags & CHKATYPE_ANYARRAY)) ||
              (atttypid == RECORDOID && (flags & CHKATYPE_ANYRECORD)) ||
              (atttypid == RECORDARRAYOID && (flags & CHKATYPE_ANYRECORD))))
-           ereport(ERROR,
-                   (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
-                    errmsg("column \"%s\" has pseudo-type %s",
-                           attname, format_type_be(atttypid))));
+       {
+           if (flags & CHKATYPE_IS_PARTKEY)
+               ereport(ERROR,
+                       (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+               /* translator: first %s is an integer not a name */
+                        errmsg("partition key column %s has pseudo-type %s",
+                               attname, format_type_be(atttypid))));
+           else
+               ereport(ERROR,
+                       (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+                        errmsg("column \"%s\" has pseudo-type %s",
+                               attname, format_type_be(atttypid))));
+       }
    }
    else if (att_typtype == TYPTYPE_DOMAIN)
    {
@@ -649,7 +662,7 @@ CheckAttributeType(const char *attname,
            CheckAttributeType(NameStr(attr->attname),
                               attr->atttypid, attr->attcollation,
                               containing_rowtypes,
-                              flags);
+                              flags & ~CHKATYPE_IS_PARTKEY);
        }
 
        relation_close(relation, AccessShareLock);
@@ -680,11 +693,21 @@ CheckAttributeType(const char *attname,
     * useless, and it cannot be dumped, so we must disallow it.
     */
    if (!OidIsValid(attcollation) && type_is_collatable(atttypid))
-       ereport(ERROR,
-               (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
-                errmsg("no collation was derived for column \"%s\" with collatable type %s",
-                       attname, format_type_be(atttypid)),
-                errhint("Use the COLLATE clause to set the collation explicitly.")));
+   {
+       if (flags & CHKATYPE_IS_PARTKEY)
+           ereport(ERROR,
+                   (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+           /* translator: first %s is an integer not a name */
+                    errmsg("no collation was derived for partition key column %s with collatable type %s",
+                           attname, format_type_be(atttypid)),
+                    errhint("Use the COLLATE clause to set the collation explicitly.")));
+       else
+           ereport(ERROR,
+                   (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+                    errmsg("no collation was derived for column \"%s\" with collatable type %s",
+                           attname, format_type_be(atttypid)),
+                    errhint("Use the COLLATE clause to set the collation explicitly.")));
+   }
 }
 
 /*

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3ce9aa974e78b553d41a225ded27a3830e6eaad9..ea978ce6999019fe04b267e6581bddf6dfffcaad 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -4899,8 +4899,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
 
            /*
             * Set all columns in the new slot to NULL initially, to ensure
-            * columns added as part of the rewrite are initialized to
-            * NULL. That is necessary as tab->newvals will not contain an
+            * columns added as part of the rewrite are initialized to NULL.
+            * That is necessary as tab->newvals will not contain an
             * expression for columns with a NULL default, e.g. when adding a
             * column without a default together with a column with a default
             * requiring an actual rewrite.
@@ -15139,11 +15139,23 @@ ComputePartitionAttrs(ParseState *pstate, Relation rel, List *partParams, AttrNu
        {
            /* Expression */
            Node       *expr = pelem->expr;
+           char        partattname[16];
 
            Assert(expr != NULL);
            atttype = exprType(expr);
            attcollation = exprCollation(expr);
 
+           /*
+            * The expression must be of a storable type (e.g., not RECORD).
+            * The test is the same as for whether a table column is of a safe
+            * type (which is why we needn't check for the non-expression
+            * case).
+            */
+           snprintf(partattname, sizeof(partattname), "%d", attn + 1);
+           CheckAttributeType(partattname,
+                              atttype, attcollation,
+                              NIL, CHKATYPE_IS_PARTKEY);
+
            /*
             * Strip any top-level COLLATE clause.  This ensures that we treat
             * "x COLLATE y" and "(x COLLATE y)" alike.

diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h
index eec71c29d5bbbdac75759cd6171cd4798b288859..2e63137897100a0217707a37592f7c0aa190d1b5 100644 (file)
--- a/src/include/catalog/heap.h
+++ b/src/include/catalog/heap.h
@@ -22,6 +22,7 @@
 /* flag bits for CheckAttributeType/CheckAttributeNamesTypes */
 #define CHKATYPE_ANYARRAY      0x01    /* allow ANYARRAY */
 #define CHKATYPE_ANYRECORD     0x02    /* allow RECORD and RECORD[] */
+#define CHKATYPE_IS_PARTKEY        0x04    /* attname is part key # not column */
 
 typedef struct RawColumnDefault
 {

diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index f63016871cad5365292086204e079f72a6de6c5f..50de4b380a5836c8238e9cd2aebd2449b0cc615e 100644 (file)
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -375,7 +375,7 @@ CREATE TABLE partitioned (
 ERROR:  cannot use subquery in partition key expression
 CREATE TABLE partitioned (
    a int
-) PARTITION BY RANGE (('a'));
+) PARTITION BY RANGE ((42));
 ERROR:  cannot use constant expression as partition key
 CREATE FUNCTION const_func () RETURNS int AS $$ SELECT 1; $$ LANGUAGE SQL IMMUTABLE;
 CREATE TABLE partitioned (
@@ -402,6 +402,17 @@ CREATE TABLE partitioned (
 ERROR:  cannot use system column "xmin" in partition key
 LINE 3: ) PARTITION BY RANGE (xmin);
                               ^
+-- cannot use pseudotypes
+CREATE TABLE partitioned (
+   a int,
+   b int
+) PARTITION BY RANGE (((a, b)));
+ERROR:  partition key column 1 has pseudo-type record
+CREATE TABLE partitioned (
+   a int,
+   b int
+) PARTITION BY RANGE (a, ('unknown'));
+ERROR:  partition key column 2 has pseudo-type unknown
 -- functions in key must be immutable
 CREATE FUNCTION immut_func (a int) RETURNS int AS $$ SELECT a + random()::int; $$ LANGUAGE SQL;
 CREATE TABLE partitioned (

diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index e835b65ac44ee131a20ecba54677548dd1eb3413..9a40d7b8cdffb791b4b9f1581c5ec5a5fc4bab9d 100644 (file)
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -361,7 +361,7 @@ CREATE TABLE partitioned (
 
 CREATE TABLE partitioned (
    a int
-) PARTITION BY RANGE (('a'));
+) PARTITION BY RANGE ((42));
 
 CREATE FUNCTION const_func () RETURNS int AS $$ SELECT 1; $$ LANGUAGE SQL IMMUTABLE;
 CREATE TABLE partitioned (
@@ -384,6 +384,16 @@ CREATE TABLE partitioned (
    a int
 ) PARTITION BY RANGE (xmin);
 
+-- cannot use pseudotypes
+CREATE TABLE partitioned (
+   a int,
+   b int
+) PARTITION BY RANGE (((a, b)));
+CREATE TABLE partitioned (
+   a int,
+   b int
+) PARTITION BY RANGE (a, ('unknown'));
+
 -- functions in key must be immutable
 CREATE FUNCTION immut_func (a int) RETURNS int AS $$ SELECT a + random()::int; $$ LANGUAGE SQL;
 CREATE TABLE partitioned (