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

3 files changed, 22 insertions, 7 deletions

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index bfffd6ef4c7..50efa4782d6 100644
--- 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 58265129e60..7a2629ecd19 100644
--- 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 e76ce2ceb13..3be9a6f3481 100644
--- 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)
 	{