v50-0004-fixes-0002.patch

text/x-patch

Filename: v50-0004-fixes-0002.patch
Type: text/x-patch
Part: 3
Message: Re: row filtering for logical replication

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v50-0004
Subject: fixes 0002
File+
src/backend/catalog/pg_publication.c 23 25
src/include/parser/parse_node.h 1 1
src/test/regress/expected/publication.out 5 5
src/test/regress/sql/publication.sql 3 3
From 89c3319e2a7bb5b4ec91c70c3a334e05a900cdbf Mon Sep 17 00:00:00 2001
From: Euler Taveira <euler.taveira@enterprisedb.com>
Date: Sat, 18 Dec 2021 19:08:24 -0300
Subject: [PATCH v50 4/8] fixes 0002

---
 src/backend/catalog/pg_publication.c      | 48 +++++++++++------------
 src/include/parser/parse_node.h           |  2 +-
 src/test/regress/expected/publication.out | 10 ++---
 src/test/regress/sql/publication.sql      |  6 +--
 4 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index e3c154e31f..44ec69e410 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -29,11 +29,11 @@
 #include "catalog/objectaddress.h"
 #include "catalog/pg_inherits.h"
 #include "catalog/pg_namespace.h"
+#include "catalog/pg_proc.h"
 #include "catalog/pg_publication.h"
 #include "catalog/pg_publication_namespace.h"
 #include "catalog/pg_publication_rel.h"
 #include "catalog/pg_type.h"
-#include "catalog/pg_proc.h"
 #include "commands/publicationcmds.h"
 #include "funcapi.h"
 #include "miscadmin.h"
@@ -145,31 +145,29 @@ IsRowFilterSimpleExpr(Node *node)
  * expression".
  *
  * It allows only simple or compound expressions such as:
- * - "(Var Op Const)" or
- * - "(Var Op Var)" or
- * - "(Var Op Const) Bool (Var Op Const)"
+ * - (Var Op Const)
+ * - (Var Op Var)
+ * - (Var Op Const) Bool (Var Op Const)
  * - etc
  * (where Var is a column of the table this filter belongs to)
  *
- * Specifically,
- * - User-defined operators are not allowed.
- * - User-defined functions are not allowed.
- * - User-defined types are not allowed.
- * - Non-immutable builtin functions are not allowed.
+ * The simple expression contains the following restrictions:
+ * - User-defined operators are not allowed;
+ * - User-defined functions are not allowed;
+ * - User-defined types are not allowed;
+ * - Non-immutable built-in functions are not allowed;
  * - System columns are not allowed.
  *
- * Notes:
+ * NOTES
  *
- * We don't allow user-defined functions/operators/types because (a) if the user
- * drops such a user-definition or if there is any other error via its function,
- * the walsender won't be able to recover from such an error even if we fix the
- * function's problem because a historic snapshot is used to access the
- * row-filter; (b) any other table could be accessed via a function, which won't
- * work because of historic snapshots in logical decoding environment.
- *
- * We don't allow anything other than immutable built-in functions because
- * non-immutable functions can access the database and would lead to the problem
- * (b) mentioned in the previous paragraph.
+ * We don't allow user-defined functions/operators/types because
+ * (a) if a user drops a user-defined object used in a row filter expression or
+ * if there is any other while using it, the logical decoding infrastructure
+ * won't be able to recover from such an error even if the object is recreated
+ * again because a historic snapshot is used to evaluate the row filter;
+ * (b) a user-defined function can be used to access tables which could have
+ * unpleasant results because a historic snapshot is used. That's why only
+ * non-immutable built-in functions are allowed in row filter expressions.
  */
 static bool
 rowfilter_walker(Node *node, Relation relation)
@@ -188,11 +186,11 @@ rowfilter_walker(Node *node, Relation relation)
 	{
 		Var		   *var = (Var *) node;
 
-		/* User-defined types not allowed. */
+		/* User-defined types are not allowed. */
 		if (var->vartype >= FirstNormalObjectId)
-			errdetail_msg = _("User-defined types are not allowed");
+			errdetail_msg = _("User-defined types are not allowed.");
 
-		/* System columns not allowed. */
+		/* System columns are not allowed. */
 		else if (var->varattno < InvalidAttrNumber)
 		{
 			Oid			relid = RelationGetRelid(relation);
@@ -225,12 +223,12 @@ rowfilter_walker(Node *node, Relation relation)
 	}
 	else
 	{
-		elog(DEBUG1, "the row filter contained something unexpected: %s", nodeToString(node));
+		elog(DEBUG3, "row filter contains an unexpected expression component: %s", nodeToString(node));
 
 		ereport(ERROR,
 				(errmsg("invalid publication WHERE expression for relation \"%s\"",
 						RelationGetRelationName(relation)),
-				errdetail("Expressions only allow columns, constants and some built-in functions and operators.")
+				errdetail("Expressions only allow columns, constants, built-in operators, built-in data types and non-immutable built-in functions.")
 				));
 	}
 
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index ee179082ce..1d4f3a6eab 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -79,7 +79,7 @@ typedef enum ParseExprKind
 	EXPR_KIND_CALL_ARGUMENT,	/* procedure argument in CALL */
 	EXPR_KIND_COPY_WHERE,		/* WHERE condition in COPY FROM */
 	EXPR_KIND_GENERATED_COLUMN, /* generation expression for a column */
-	EXPR_KIND_CYCLE_MARK,		/* cycle mark value */
+	EXPR_KIND_CYCLE_MARK		/* cycle mark value */
 } ParseExprKind;
 
 
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index e6651bdaae..4a27ae86e2 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -376,23 +376,23 @@ ERROR:  invalid publication WHERE expression for relation "testpub_rf_tbl1"
 DETAIL:  Non-immutable built-in functions are not allowed (random).
 -- ok - NULLIF is allowed
 ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl1 WHERE (NULLIF(1,2) = a);
--- ok - builtin operators are allowed
+-- ok - built-in operators are allowed
 ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl1 WHERE (a IS NULL);
 ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl1 WHERE ((a > 5) IS FALSE);
--- ok - immutable builtin functions are allowed
+-- ok - immutable built-in functions are allowed
 ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl4 WHERE (length(g) < 6);
 -- fail - user-defined types disallowed
 CREATE TYPE rf_bug_status AS ENUM ('new', 'open', 'closed');
 CREATE TABLE rf_bug (id serial, description text, status rf_bug_status);
 CREATE PUBLICATION testpub6 FOR TABLE rf_bug WHERE (status = 'open') WITH (publish = 'insert');
 ERROR:  invalid publication WHERE expression for relation "rf_bug"
-DETAIL:  User-defined types are not allowed
+DETAIL:  User-defined types are not allowed.
 DROP TABLE rf_bug;
 DROP TYPE rf_bug_status;
--- fail - row-filter expression is not simple
+-- fail - row filter expression is not simple
 CREATE PUBLICATION testpub6 FOR TABLE testpub_rf_tbl1 WHERE (a IN (SELECT generate_series(1,5)));
 ERROR:  invalid publication WHERE expression for relation "testpub_rf_tbl1"
-DETAIL:  Expressions only allow columns, constants and some built-in functions and operators.
+DETAIL:  Expressions only allow columns, constants, built-in operators, built-in data types and non-immutable built-in functions.
 -- fail - system columns are not allowed
 CREATE PUBLICATION testpub6 FOR TABLE testpub_rf_tbl1 WHERE ('(0,1)'::tid = ctid);
 ERROR:  invalid publication WHERE expression for relation "testpub_rf_tbl1"
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index a95c71b1cb..73fc103b8c 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -197,10 +197,10 @@ ALTER PUBLICATION testpub5 ADD TABLE testpub_rf_tbl1 WHERE (a >= testpub_rf_func
 ALTER PUBLICATION testpub5 ADD TABLE testpub_rf_tbl1 WHERE (a < random());
 -- ok - NULLIF is allowed
 ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl1 WHERE (NULLIF(1,2) = a);
--- ok - builtin operators are allowed
+-- ok - built-in operators are allowed
 ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl1 WHERE (a IS NULL);
 ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl1 WHERE ((a > 5) IS FALSE);
--- ok - immutable builtin functions are allowed
+-- ok - immutable built-in functions are allowed
 ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl4 WHERE (length(g) < 6);
 -- fail - user-defined types disallowed
 CREATE TYPE rf_bug_status AS ENUM ('new', 'open', 'closed');
@@ -208,7 +208,7 @@ CREATE TABLE rf_bug (id serial, description text, status rf_bug_status);
 CREATE PUBLICATION testpub6 FOR TABLE rf_bug WHERE (status = 'open') WITH (publish = 'insert');
 DROP TABLE rf_bug;
 DROP TYPE rf_bug_status;
--- fail - row-filter expression is not simple
+-- fail - row filter expression is not simple
 CREATE PUBLICATION testpub6 FOR TABLE testpub_rf_tbl1 WHERE (a IN (SELECT generate_series(1,5)));
 -- fail - system columns are not allowed
 CREATE PUBLICATION testpub6 FOR TABLE testpub_rf_tbl1 WHERE ('(0,1)'::tid = ctid);
-- 
2.20.1