Don't allow CREATE TABLE AS to create a column with invalid collation

It is possible that an expression ends up with a collatable type but
without a collation.  CREATE TABLE AS could then create a table based
on that.  But such a column cannot be dumped with valid SQL syntax, so
we disallow creating such a column.

per test report from Noah Misch

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index bfffd6e..50efa47 100644 (file)
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -429,6 +429,7 @@ CheckAttributeNamesTypes(TupleDesc tupdesc, char relkind,
        {
                CheckAttributeType(NameStr(tupdesc->attrs[i]->attname),
                                                   tupdesc->attrs[i]->atttypid,
+                                                  tupdesc->attrs[i]->attcollation,
                                                   allow_system_table_mods);
        }
 }
@@ -442,7 +443,7 @@ CheckAttributeNamesTypes(TupleDesc tupdesc, char relkind,
  * --------------------------------
  */
 void
-CheckAttributeType(const char *attname, Oid atttypid,
+CheckAttributeType(const char *attname, Oid atttypid, Oid attcollation,
                                   bool allow_system_table_mods)
 {
        char            att_typtype = get_typtype(atttypid);
@@ -493,12 +494,24 @@ CheckAttributeType(const char *attname, Oid atttypid,
 
                        if (attr->attisdropped)
                                continue;
-                       CheckAttributeType(NameStr(attr->attname), attr->atttypid,
+                       CheckAttributeType(NameStr(attr->attname), attr->atttypid, attr->attcollation,
                                                           allow_system_table_mods);
                }
 
                relation_close(relation, AccessShareLock);
        }
+
+       /*
+        * This might not be strictly invalid per SQL standard, but it is
+        * pretty useless, and it cannot be dumped, so we must disallow
+        * it.
+        */
+       if (type_is_collatable(atttypid) && !OidIsValid(attcollation))
+                       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/catalog/index.c b/src/backend/catalog/index.c
index 5826512..7a2629e 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -352,6 +352,8 @@ ConstructTupleDescriptor(Relation heapRelation,
                        to->atthasdef = false;
                        to->attislocal = true;
                        to->attinhcount = 0;
+
+                       to->attcollation = collationObjectId[i];
                }
                else
                {
@@ -388,6 +390,8 @@ ConstructTupleDescriptor(Relation heapRelation,
                        to->atttypmod = -1;
                        to->attislocal = true;
 
+                       to->attcollation = collationObjectId[i];
+
                        ReleaseSysCache(tuple);
 
                        /*
@@ -399,11 +403,9 @@ ConstructTupleDescriptor(Relation heapRelation,
                         * whether a table column is of a safe type (which is why we
                         * needn't check for the non-expression case).
                         */
-                       CheckAttributeType(NameStr(to->attname), to->atttypid, false);
+                       CheckAttributeType(NameStr(to->attname), to->atttypid, to->attcollation, false);
                }
 
-               to->attcollation = collationObjectId[i];
-
                /*
                 * We do not yet have the correct relation OID for the index, so just
                 * set it invalid for now.      InitializeAttributeOids() will fix it

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index e76ce2c..3be9a6f 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -4206,7 +4206,7 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
        typeOid = HeapTupleGetOid(typeTuple);
 
        /* make sure datatype is legal for a column */
-       CheckAttributeType(colDef->colname, typeOid, false);
+       CheckAttributeType(colDef->colname, typeOid, collOid, false);
 
        /* construct new attribute's pg_attribute entry */
        attribute.attrelid = myrelid;
@@ -6515,7 +6515,7 @@ ATPrepAlterColumnType(List **wqueue,
        typenameTypeIdModColl(NULL, typeName, &targettype, &targettypmod, &targetcollid);
 
        /* make sure datatype is legal for a column */
-       CheckAttributeType(colName, targettype, false);
+       CheckAttributeType(colName, targettype, targetcollid, false);
 
        if (tab->relkind == RELKIND_RELATION)
        {

diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h
index f62536a..5f9a864 100644 (file)
--- a/src/include/catalog/heap.h
+++ b/src/include/catalog/heap.h
@@ -117,7 +117,7 @@ extern Form_pg_attribute SystemAttributeByName(const char *attname,
 extern void CheckAttributeNamesTypes(TupleDesc tupdesc, char relkind,
                                                 bool allow_system_table_mods);
 
-extern void CheckAttributeType(const char *attname, Oid atttypid,
+extern void CheckAttributeType(const char *attname, Oid atttypid, Oid attcollation,
                                   bool allow_system_table_mods);
 
 #endif   /* HEAP_H */

diff --git a/src/test/regress/expected/collate.linux.utf8.out b/src/test/regress/expected/collate.linux.utf8.out
index c793918..caa65b2 100644 (file)
--- a/src/test/regress/expected/collate.linux.utf8.out
+++ b/src/test/regress/expected/collate.linux.utf8.out
@@ -627,6 +627,9 @@ ERROR:  collation mismatch between implicit collations "en_US.utf8" and "C"
 LINE 1: SELECT a, b FROM collate_test1 EXCEPT SELECT a, b FROM colla...
                                                         ^
 HINT:  You can override the collation by applying the COLLATE clause to one or both expressions.
+CREATE TABLE test_u AS SELECT a, b FROM collate_test1 UNION ALL SELECT a, b FROM collate_test3; -- fail
+ERROR:  no collation was derived for column "b" with collatable type text
+HINT:  Use the COLLATE clause to set the collation explicitly.
 -- casting
 SELECT CAST('42' AS text COLLATE "C");
 ERROR:  COLLATE clause not allowed in cast target

diff --git a/src/test/regress/sql/collate.linux.utf8.sql b/src/test/regress/sql/collate.linux.utf8.sql
index 9fd55e8..c70e5ce 100644 (file)
--- a/src/test/regress/sql/collate.linux.utf8.sql
+++ b/src/test/regress/sql/collate.linux.utf8.sql
@@ -188,6 +188,8 @@ SELECT a, b COLLATE "C" FROM collate_test1 UNION SELECT a, b FROM collate_test3
 SELECT a, b FROM collate_test1 INTERSECT SELECT a, b FROM collate_test3 ORDER BY 2; -- fail
 SELECT a, b FROM collate_test1 EXCEPT SELECT a, b FROM collate_test3 ORDER BY 2; -- fail
 
+CREATE TABLE test_u AS SELECT a, b FROM collate_test1 UNION ALL SELECT a, b FROM collate_test3; -- fail
+
 
 -- casting