While reading this code, I noticed that function expr_allowed_in_node()
has a very strange API: it doesn't have any return convention at all
other than "if we didn't modify errdetail_str then all is good". I was
tempted to add an "Assert(*errdetail_msg == NULL)" at the start of it,
just to make sure that it is not called if a message is already set.
I think it would be much saner to inline the few lines of that function
in its sole caller, as in the attached.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"E pur si muove" (Galileo Galilei)
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 8895e1be4e..f9792df05f 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -447,36 +447,6 @@ contain_mutable_or_user_functions_checker(Oid func_id, void *context)
func_id >= FirstNormalObjectId);
}
-/*
- * Check if the node contains any disallowed object. Subroutine for
- * check_simple_rowfilter_expr_walker.
- *
- * If a disallowed object is found, *errdetail_msg is set to a (possibly
- * translated) message to use as errdetail. If none, *errdetail_msg is not
- * modified.
- */
-static void
-expr_allowed_in_node(Node *node, ParseState *pstate, char **errdetail_msg)
-{
- if (IsA(node, List))
- {
- /*
- * OK, we don't need to perform other expr checks for List nodes
- * because those are undefined for List.
- */
- return;
- }
-
- if (exprType(node) >= FirstNormalObjectId)
- *errdetail_msg = _("User-defined types are not allowed.");
- else if (check_functions_in_node(node, contain_mutable_or_user_functions_checker,
- (void *) pstate))
- *errdetail_msg = _("User-defined or built-in mutable functions are not allowed.");
- else if (exprCollation(node) >= FirstNormalObjectId ||
- exprInputCollation(node) >= FirstNormalObjectId)
- *errdetail_msg = _("User-defined collations are not allowed.");
-}
-
/*
* The row filter walker checks if the row filter expression is a "simple
* expression".
@@ -586,12 +556,32 @@ check_simple_rowfilter_expr_walker(Node *node, ParseState *pstate)
}
/*
- * For all the supported nodes, check the types, functions, and collations
- * used in the nodes.
+ * For all the supported nodes, if we haven't already found a problem,
+ * check the types, functions, and collations used in it.
*/
if (!errdetail_msg)
- expr_allowed_in_node(node, pstate, &errdetail_msg);
+ {
+ if (IsA(node, List))
+ {
+ /*
+ * OK, we don't need to perform other expr checks for List nodes
+ * because those are undefined for List.
+ */
+ }
+ else if (exprType(node) >= FirstNormalObjectId)
+ errdetail_msg = _("User-defined types are not allowed.");
+ else if (check_functions_in_node(node, contain_mutable_or_user_functions_checker,
+ (void *) pstate))
+ errdetail_msg = _("User-defined or built-in mutable functions are not allowed.");
+ else if (exprCollation(node) >= FirstNormalObjectId ||
+ exprInputCollation(node) >= FirstNormalObjectId)
+ errdetail_msg = _("User-defined collations are not allowed.");
+ }
+ /*
+ * Finally, if we found a problem in this node, throw error now. Otherwise
+ * keep going.
+ */
if (errdetail_msg)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
