From 0338d0af3261aa999c72ee9a9d66821af8a6a5b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Villemain?= <Cedric.Villemain@Data-Bene.io>
Date: Mon, 10 Mar 2025 09:24:23 +0100
Subject: [PATCH] Manage special case for "GRANT ALL" with columns

`GRANT/REVOKE ALL (col, ...)` cannot be mixed with other privilege and
because there is at least one column defined, the privileges node is
defined so the existing code is valid for table but not for columns.

Add a condition to ensure `ALL` is defined alone and added to the
privilege string when needed.

Comment added based on:
https://github.com/citusdata/citus/pull/7918#discussion_r1994929886
---
 src/backend/distributed/commands/grant.c | 33 +++++++++++++++++++++---
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/src/backend/distributed/commands/grant.c b/src/backend/distributed/commands/grant.c
index a2645c33f..1f1477281 100644
--- a/src/backend/distributed/commands/grant.c
+++ b/src/backend/distributed/commands/grant.c
@@ -73,6 +73,7 @@ PreprocessGrantStmt(Node *node, const char *queryString,
 	/* deparse the privileges */
 	if (grantStmt->privileges == NIL)
 	{
+		/* this is used for table level only */
 		appendStringInfo(&privsString, "ALL");
 	}
 	else
@@ -88,12 +89,36 @@ PreprocessGrantStmt(Node *node, const char *queryString,
 			{
 				appendStringInfoString(&privsString, ", ");
 			}
+
+			if (priv->priv_name)
+			{
+				appendStringInfo(&privsString, "%s", priv->priv_name);
+			}
+			/*
+			 * ALL can only be set alone.
+			 * And ALL is not added as a keyword in priv_name by parser, but
+			 * because there are column(s) defined, a grantStmt->privileges is
+			 * defined. So we need to handle this special case here (see if
+			 * condition above).
+			 */
+			else if (isFirst)
+			{
+				/* this is used for column level only */
+				appendStringInfo(&privsString, "ALL");
+			}
+			/*
+			 * Instead of relying only on the syntax check done by Postgres and
+			 * adding an assert here, add a default ERROR if ALL is not first
+			 * and no priv_name is defined.
+			 */
+			else
+			{
+				ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR),
+								errmsg("Cannot parse GRANT/REVOKE privileges")));
+			}
+
 			isFirst = false;
 
-			Assert(priv->priv_name != NULL);
-
-			appendStringInfo(&privsString, "%s", priv->priv_name);
-
 			if (priv->cols != NIL)
 			{
 				StringInfoData colsString;