Subject: Propagate sanity checks of ProcessUtility() to standard_ProcessUtility()?

From: Michael Paquier <[email-masked]>
Date: Thu, 29 Feb 2024 16:20:53 +0900

Hi all,

It's been brought to me that an extension may finish by breaking the
assumptions ProcessUtility() relies on when calling
standard_ProcessUtility(), causing breakages when passing down data to
cascading utility hooks.

Isn't the state of the arguments given something we should check not
only in the main entry point ProcessUtility() but also in
standard_ProcessUtility(), to prevent issues if an extension
incorrectly manipulates the arguments it needs to pass down to other
modules that use the utility hook, like using a NULL query string?

See the attached for the idea.
Thanks,
--
Michael
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 8de821f960..ee82551dde 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -88,6 +88,20 @@ static void ProcessUtilitySlow(ParseState *pstate,
 							   QueryCompletion *qc);
 static void ExecDropStmt(DropStmt *stmt, bool isTopLevel);
 
+
+/*
+ * Check the state of the arguments given to entry points for utility
+ * processing.
+ */
+#define UTILITY_CHECKS \
+do { \
+	Assert(IsA(pstmt, PlannedStmt)); \
+	Assert(pstmt->commandType == CMD_UTILITY); \
+	Assert(queryString != NULL);	/* required as of 8.4 */ \
+	Assert(qc == NULL || qc->commandTag == CMDTAG_UNKNOWN); \
+} while (0)
+
+
 /*
  * CommandIsReadOnly: is an executable query read-only?
  *
@@ -512,10 +526,7 @@ ProcessUtility(PlannedStmt *pstmt,
 			   DestReceiver *dest,
 			   QueryCompletion *qc)
 {
-	Assert(IsA(pstmt, PlannedStmt));
-	Assert(pstmt->commandType == CMD_UTILITY);
-	Assert(queryString != NULL);	/* required as of 8.4 */
-	Assert(qc == NULL || qc->commandTag == CMDTAG_UNKNOWN);
+	UTILITY_CHECKS;
 
 	/*
 	 * We provide a function hook variable that lets loadable plugins get
@@ -559,6 +570,8 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 	ParseState *pstate;
 	int			readonly_flags;
 
+	UTILITY_CHECKS;
+
 	/* This can recurse, so check for excessive recursion */
 	check_stack_depth();