Fix segfault in column_to_column_name (#3260)

Fix segfault in column_to_column_name

src/backend/distributed/metadata/metadata_sync.c

@@ -643,7 +643,7 @@ DistributionCreateCommand(DistTableCacheEntry *cacheEntry)
 	}
 	else
 	{
-		char *partitionKeyColumnName = ColumnNameToColumn(relationId, partitionKeyString);
+		char *partitionKeyColumnName = ColumnToColumnName(relationId, partitionKeyString);
 		appendStringInfo(tablePartitionKeyString, "column_name_to_column(%s,%s)",
 						 quote_literal_cstr(qualifiedRelationName),
 						 quote_literal_cstr(partitionKeyColumnName));

src/backend/distributed/planner/multi_router_planner.c

@@ -715,7 +715,7 @@ ModifyQuerySupported(Query *queryTree, Query *originalQuery, bool multiShardQuer
 				DistTableCacheEntry *cacheEntry = DistributedTableCacheEntry(
 					distributedTableId);
 				char *partitionKeyString = cacheEntry->partitionKeyString;
-				char *partitionColumnName = ColumnNameToColumn(distributedTableId,
+				char *partitionColumnName = ColumnToColumnName(distributedTableId,
 															   partitionKeyString);
 
 				appendStringInfo(errorHint, "Consider using an equality filter on "
@@ -2556,7 +2556,7 @@ BuildRoutesForInsert(Query *query, DeferredErrorMessage **planningError)
 		if (prunedShardIntervalCount != 1)
 		{
 			char *partitionKeyString = cacheEntry->partitionKeyString;
-			char *partitionColumnName = ColumnNameToColumn(distributedTableId,
+			char *partitionColumnName = ColumnToColumnName(distributedTableId,
 														   partitionKeyString);
 			StringInfo errorMessage = makeStringInfo();
 			StringInfo errorHint = makeStringInfo();

src/backend/distributed/utils/distribution_column.c

@@ -105,7 +105,7 @@ column_to_column_name(PG_FUNCTION_ARGS)
 
 	CheckCitusVersion(ERROR);
 
-	char *columnName = ColumnNameToColumn(relationId, columnNodeString);
+	char *columnName = ColumnToColumnName(relationId, columnNodeString);
 
 	text *columnText = cstring_to_text(columnName);
 
@@ -167,17 +167,22 @@ BuildDistributionKeyFromColumnName(Relation distributedRelation, char *columnNam
 
 
 /*
- * ColumnNameToColumn returns the human-readable name of a column given a
+ * ColumnToColumnName returns the human-readable name of a column given a
  * relation identifier and the column's internal textual (Var) representation.
  * This function will raise an ERROR if no such column can be found or if the
  * provided Var refers to a system column.
  */
 char *
-ColumnNameToColumn(Oid relationId, char *columnNodeString)
+ColumnToColumnName(Oid relationId, char *columnNodeString)
 {
 	Node *columnNode = stringToNode(columnNodeString);
 
-	Assert(IsA(columnNode, Var));
+	if (columnNode == NULL || !IsA(columnNode, Var))
+	{
+		ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						errmsg("not a valid column")));
+	}
+
 	Var *column = (Var *) columnNode;
 
 	AttrNumber columnNumber = column->varattno;

src/include/distributed/distribution_column.h

@@ -21,6 +21,6 @@
 /* Remaining metadata utility functions  */
 extern Var * BuildDistributionKeyFromColumnName(Relation distributedRelation,
 												char *columnName);
-extern char * ColumnNameToColumn(Oid relationId, char *columnNodeString);
+extern char * ColumnToColumnName(Oid relationId, char *columnNodeString);
 
 #endif   /* DISTRIBUTION_COLUMN_H */

src/test/regress/expected/multi_distribution_metadata.out

@@ -203,6 +203,24 @@ SELECT partmethod, column_to_column_name(logicalrelid, partkey) FROM pg_dist_par
  h          | id
 (1 row)
 
+-- test column_to_column_name with illegal arguments
+SELECT column_to_column_name(1204127312,'');
+ERROR:  not a valid column
+SELECT column_to_column_name('customers','');
+ERROR:  not a valid column
+SELECT column_to_column_name('pg_dist_node'::regclass, NULL);
+ column_to_column_name 
+-----------------------
+ 
+(1 row)
+
+SELECT column_to_column_name('pg_dist_node'::regclass,'{FROMEXPR :fromlist ({RANGETBLREF :rtindex 1 }) :quals <>}');
+ERROR:  not a valid column
+-- test column_name_to_column with illegal arguments
+SELECT column_name_to_column(1204127312,'');
+ERROR:  could not open relation with OID 1204127312
+SELECT column_name_to_column('customers','notacolumn');
+ERROR:  column "notacolumn" of relation "customers" does not exist
 -- make one huge shard and manually inspect shard row
 SELECT create_monolithic_shard_row('customers') AS new_shard_id
 \gset
@@ -486,73 +504,73 @@ SELECT create_distributed_table('users_table_count', 'user_id');
 SELECT relation_count_in_query($$-- we can support arbitrary subqueries within UNIONs
 SELECT ("final_query"."event_types") as types, count(*) AS sumOfEventType
 FROM
-  ( SELECT 
+  ( SELECT
       *, random()
     FROM
-     (SELECT 
+     (SELECT
         "t"."user_id", "t"."time", unnest("t"."collected_events") AS "event_types"
       FROM
-        ( SELECT 
+        ( SELECT
             "t1"."user_id", min("t1"."time") AS "time", array_agg(("t1"."event") ORDER BY TIME ASC, event DESC) AS collected_events
           FROM (
-                (SELECT 
+                (SELECT
                     *
                  FROM
-                   (SELECT 
+                   (SELECT
                           events_table."time", 0 AS event, events_table."user_id"
-                    FROM 
+                    FROM
                        "events_table_count" as events_table
-                    WHERE 
+                    WHERE
-                      events_table.event_type IN (1, 2) ) events_subquery_1) 
+                      events_table.event_type IN (1, 2) ) events_subquery_1)
-                UNION 
+                UNION
                  (SELECT *
                   FROM
                     (
                           SELECT * FROM
                           (
-                              SELECT 
+                              SELECT
                                 max("events_table_count"."time"),
                                 0 AS event,
                                 "events_table_count"."user_id"
-                              FROM 
+                              FROM
                                 "events_table_count", users_table_count as "users"
-                              WHERE 
+                              WHERE
                                  "events_table_count".user_id = users.user_id AND
                                 "events_table_count".event_type IN (1, 2)
                                 GROUP BY   "events_table_count"."user_id"
                           ) as events_subquery_5
                      ) events_subquery_2)
-               UNION 
+               UNION
                  (SELECT *
                   FROM
-                    (SELECT 
+                    (SELECT
                         "events_table_count"."time", 2 AS event, "events_table_count"."user_id"
-                     FROM 
+                     FROM
                        "events_table_count"
-                     WHERE 
+                     WHERE
                       event_type IN (3, 4) ) events_subquery_3)
-               UNION 
+               UNION
                  (SELECT *
                   FROM
                     (SELECT
                        "events_table_count"."time", 3 AS event, "events_table_count"."user_id"
-                     FROM 
+                     FROM
                        "events_table_count"
-                     WHERE 
+                     WHERE
                       event_type IN (5, 6)) events_subquery_4)
                  ) t1
-         GROUP BY "t1"."user_id") AS t) "q" 
+         GROUP BY "t1"."user_id") AS t) "q"
 INNER JOIN
-     (SELECT 
+     (SELECT
         "events_table_count"."user_id"
-      FROM 
+      FROM
         users_table_count as "events_table_count"
-      WHERE 
+      WHERE
-        value_1 > 0 and value_1 < 4) AS t 
+        value_1 > 0 and value_1 < 4) AS t
      ON (t.user_id = q.user_id)) as final_query
-GROUP BY 
+GROUP BY
   types
-ORDER BY 
+ORDER BY
   types;$$);
  relation_count_in_query 
 -------------------------

src/test/regress/sql/multi_distribution_metadata.sql

@@ -150,6 +150,16 @@ VALUES
 SELECT partmethod, column_to_column_name(logicalrelid, partkey) FROM pg_dist_partition
 	WHERE logicalrelid = 'customers'::regclass;
 
+-- test column_to_column_name with illegal arguments
+SELECT column_to_column_name(1204127312,'');
+SELECT column_to_column_name('customers','');
+SELECT column_to_column_name('pg_dist_node'::regclass, NULL);
+SELECT column_to_column_name('pg_dist_node'::regclass,'{FROMEXPR :fromlist ({RANGETBLREF :rtindex 1 }) :quals <>}');
+
+-- test column_name_to_column with illegal arguments
+SELECT column_name_to_column(1204127312,'');
+SELECT column_name_to_column('customers','notacolumn');
+
 -- make one huge shard and manually inspect shard row
 SELECT create_monolithic_shard_row('customers') AS new_shard_id
 \gset