On 22.03.21 03:15, Euler Taveira wrote:
I attached a new patch set that addresses:
* fix documentation;
* rename PublicationRelationQual to PublicationRelationInfo;
* remove the memset that was leftover from a previous patch set;
* add new tests to improve coverage (INSERT/UPDATE/DELETE to exercise
the row
filter code).
I have committed the 0001 patch.
Attached are a few fixup patches that I recommend you integrate into
your patch set. They address backward compatibility with PG13, and a
few more stylistic issues.
I suggest you combine your 0002, 0003, and 0004 patches into one. They
can't be used separately, and for example the psql changes in patch 0003
already appear as regression test output changes in 0002, so this
arrangement isn't useful. (0005 can be kept separately, since it's
mostly for debugging right now.)
From c29459505db88c86dd7aa61019fa406202c30b0a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 25 Mar 2021 11:57:48 +0100
Subject: [PATCH 1/6] fixup! Row filter for logical replication
Remove unused header files, clean up whitespace.
---
src/backend/catalog/pg_publication.c | 2 --
src/backend/replication/pgoutput/pgoutput.c | 4 ----
2 files changed, 6 deletions(-)
diff --git a/src/backend/catalog/pg_publication.c
b/src/backend/catalog/pg_publication.c
index f31ae28de2..78f5780fb7 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -33,11 +33,9 @@
#include "catalog/pg_type.h"
#include "funcapi.h"
#include "miscadmin.h"
-
#include "parser/parse_clause.h"
#include "parser/parse_collate.h"
#include "parser/parse_relation.h"
-
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/catcache.h"
diff --git a/src/backend/replication/pgoutput/pgoutput.c
b/src/backend/replication/pgoutput/pgoutput.c
index ce6da8de19..6151f34925 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -16,14 +16,10 @@
#include "catalog/partition.h"
#include "catalog/pg_publication.h"
#include "catalog/pg_publication_rel.h"
-#include "catalog/pg_type.h"
#include "commands/defrem.h"
#include "executor/executor.h"
#include "fmgr.h"
-#include "nodes/execnodes.h"
#include "nodes/nodeFuncs.h"
-#include "optimizer/planner.h"
-#include "optimizer/optimizer.h"
#include "parser/parse_coerce.h"
#include "replication/logical.h"
#include "replication/logicalproto.h"
--
2.30.2
From 91c20ecb45a8627a9252ed589fe6b85927be8b45 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 25 Mar 2021 11:59:13 +0100
Subject: [PATCH 2/6] fixup! Row filter for logical replication
Allow replication from older PostgreSQL versions without prqual.
---
src/backend/replication/logical/tablesync.c | 70 +++++++++++----------
1 file changed, 37 insertions(+), 33 deletions(-)
diff --git a/src/backend/replication/logical/tablesync.c
b/src/backend/replication/logical/tablesync.c
index 40d84dadb5..246510c82e 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -796,49 +796,53 @@ fetch_remote_table_info(char *nspname, char *relname,
walrcv_clear_result(res);
/* Get relation qual */
- resetStringInfo(&cmd);
- appendStringInfo(&cmd,
- "SELECT pg_get_expr(prqual, prrelid) "
- " FROM pg_publication p "
- " INNER JOIN pg_publication_rel pr "
- " ON (p.oid = pr.prpubid) "
- " WHERE pr.prrelid = %u "
- " AND p.pubname IN (",
lrel->remoteid);
-
- first = true;
- foreach(lc, MySubscription->publications)
+ if (walrcv_server_version(wrconn) >= 140000)
{
- char *pubname = strVal(lfirst(lc));
+ resetStringInfo(&cmd);
+ appendStringInfo(&cmd,
+ "SELECT pg_get_expr(prqual,
prrelid) "
+ " FROM pg_publication p "
+ " INNER JOIN
pg_publication_rel pr "
+ " ON (p.oid =
pr.prpubid) "
+ " WHERE pr.prrelid = %u "
+ " AND p.pubname IN (",
lrel->remoteid);
+
+ first = true;
+ foreach(lc, MySubscription->publications)
+ {
+ char *pubname = strVal(lfirst(lc));
- if (first)
- first = false;
- else
- appendStringInfoString(&cmd, ", ");
+ if (first)
+ first = false;
+ else
+ appendStringInfoString(&cmd, ", ");
- appendStringInfoString(&cmd, quote_literal_cstr(pubname));
- }
- appendStringInfoChar(&cmd, ')');
+ appendStringInfoString(&cmd,
quote_literal_cstr(pubname));
+ }
+ appendStringInfoChar(&cmd, ')');
- res = walrcv_exec(wrconn, cmd.data, 1, qualRow);
+ res = walrcv_exec(wrconn, cmd.data, 1, qualRow);
- if (res->status != WALRCV_OK_TUPLES)
- ereport(ERROR,
- (errmsg("could not fetch relation
qualifications for table \"%s.%s\" from publisher: %s",
- nspname, relname, res->err)));
+ if (res->status != WALRCV_OK_TUPLES)
+ ereport(ERROR,
+ (errmsg("could not fetch relation
qualifications for table \"%s.%s\" from publisher: %s",
+ nspname, relname,
res->err)));
- slot = MakeSingleTupleTableSlot(res->tupledesc, &TTSOpsMinimalTuple);
- while (tuplestore_gettupleslot(res->tuplestore, true, false, slot))
- {
- Datum rf = slot_getattr(slot, 1, &isnull);
+ slot = MakeSingleTupleTableSlot(res->tupledesc,
&TTSOpsMinimalTuple);
+ while (tuplestore_gettupleslot(res->tuplestore, true, false,
slot))
+ {
+ Datum rf = slot_getattr(slot, 1, &isnull);
- if (!isnull)
- *qual = lappend(*qual,
makeString(TextDatumGetCString(rf)));
+ if (!isnull)
+ *qual = lappend(*qual,
makeString(TextDatumGetCString(rf)));
- ExecClearTuple(slot);
+ ExecClearTuple(slot);
+ }
+ ExecDropSingleTupleTableSlot(slot);
+
+ walrcv_clear_result(res);
}
- ExecDropSingleTupleTableSlot(slot);
- walrcv_clear_result(res);
pfree(cmd.data);
}
--
2.30.2
From d40b463def29cdded18579d6e584cb9f5f4764d6 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 25 Mar 2021 12:00:35 +0100
Subject: [PATCH 3/6] fixup! Row filter for logical replication
Use more idiomatic style for checking for empty or nonempty lists.
---
src/backend/replication/logical/tablesync.c | 4 ++--
src/backend/replication/pgoutput/pgoutput.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/backend/replication/logical/tablesync.c
b/src/backend/replication/logical/tablesync.c
index 246510c82e..c3d6847b35 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -881,7 +881,7 @@ copy_table(Relation rel)
initStringInfo(&cmd);
/* Regular table with no row filter */
- if (lrel.relkind == RELKIND_RELATION && list_length(qual) == 0)
+ if (lrel.relkind == RELKIND_RELATION && qual == NIL)
appendStringInfo(&cmd, "COPY %s TO STDOUT",
quote_qualified_identifier(lrel.nspname, lrel.relname));
else
@@ -902,7 +902,7 @@ copy_table(Relation rel)
appendStringInfo(&cmd, " FROM %s",
quote_qualified_identifier(lrel.nspname, lrel.relname));
/* list of AND'ed filters */
- if (list_length(qual) > 0)
+ if (qual != NIL)
{
ListCell *lc;
bool first = true;
diff --git a/src/backend/replication/pgoutput/pgoutput.c
b/src/backend/replication/pgoutput/pgoutput.c
index 6151f34925..593a8c96c8 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -568,7 +568,7 @@ pgoutput_row_filter(Relation relation, HeapTuple oldtuple,
HeapTuple newtuple, L
bool result = true;
/* Bail out if there is no row filter */
- if (list_length(rowfilter) == 0)
+ if (rowfilter == NIL)
return true;
elog(DEBUG3, "table \"%s.%s\" has row filter",
@@ -1372,7 +1372,7 @@ rel_sync_cache_publication_cb(Datum arg, int cacheid,
uint32 hashvalue)
entry->pubactions.pubdelete = false;
entry->pubactions.pubtruncate = false;
- if (list_length(entry->qual) > 0)
+ if (entry->qual != NIL)
list_free_deep(entry->qual);
entry->qual = NIL;
}
--
2.30.2
From b82e7b05bb3fd73bede39f480c31d657fe410e41 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 25 Mar 2021 12:02:48 +0100
Subject: [PATCH 4/6] fixup! Row filter for logical replication
elog arguments are not evaluated unless the message level is
interesting, so the previous workaround is unnecessary.
---
src/backend/replication/pgoutput/pgoutput.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/src/backend/replication/pgoutput/pgoutput.c
b/src/backend/replication/pgoutput/pgoutput.c
index 593a8c96c8..00ad8f001f 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -603,17 +603,11 @@ pgoutput_row_filter(Relation relation, HeapTuple
oldtuple, HeapTuple newtuple, L
/* Evaluates row filter */
result = pgoutput_row_filter_exec_expr(exprstate, ecxt);
- if (message_level_is_interesting(DEBUG3))
- {
- char *s = NULL;
-
- s =
TextDatumGetCString(DirectFunctionCall2(pg_get_expr,
CStringGetTextDatum(nodeToString(rfnode)), ObjectIdGetDatum(relation->rd_id)));
- if (result)
- elog(DEBUG3, "row filter \"%s\" matched", s);
- else
- elog(DEBUG3, "row filter \"%s\" not matched",
s);
- pfree(s);
- }
+ elog(DEBUG3, "row filter \"%s\" %smatched",
+ TextDatumGetCString(DirectFunctionCall2(pg_get_expr,
+
CStringGetTextDatum(nodeToString(rfnode)),
+
ObjectIdGetDatum(relation->rd_id))),
+ result ? "" : " not");
/* If the tuple does not match one of the row filters, bail out
*/
if (!result)
--
2.30.2
From af012ad7d7592f88e353642fd715a3825ea2ec72 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 25 Mar 2021 12:07:11 +0100
Subject: [PATCH 5/6] fixup! Publication WHERE condition support for pg_dump
Make pg_dump backward compatible.
Also add necessary parentheses around expression. pg_get_expr will
supply the parentheses in many cases, but it won't for things like
"WHERE TRUE".
---
src/bin/pg_dump/pg_dump.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 72d87f21c8..af57a50f27 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4141,10 +4141,16 @@ getPublicationTables(Archive *fout, TableInfo
tblinfo[], int numTables)
query = createPQExpBuffer();
/* Collect all publication membership info. */
- appendPQExpBufferStr(query,
- "SELECT tableoid, oid,
prpubid, prrelid, "
-
"pg_catalog.pg_get_expr(prqual, prrelid) AS prrelqual "
- "FROM
pg_catalog.pg_publication_rel");
+ if (fout->remoteVersion >= 140000)
+ appendPQExpBufferStr(query,
+ "SELECT tableoid, oid,
prpubid, prrelid, "
+
"pg_catalog.pg_get_expr(prqual, prrelid) AS prrelqual "
+ "FROM
pg_catalog.pg_publication_rel");
+ else
+ appendPQExpBufferStr(query,
+ "SELECT tableoid, oid,
prpubid, prrelid, "
+ "NULL AS prrelqual "
+ "FROM
pg_catalog.pg_publication_rel");
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
ntups = PQntuples(res);
@@ -4233,7 +4239,7 @@ dumpPublicationTable(Archive *fout, const
PublicationRelInfo *pubrinfo)
appendPQExpBuffer(query, " %s",
fmtQualifiedDumpable(tbinfo));
if (pubrinfo->pubrelqual)
- appendPQExpBuffer(query, " WHERE %s", pubrinfo->pubrelqual);
+ appendPQExpBuffer(query, " WHERE (%s)", pubrinfo->pubrelqual);
appendPQExpBufferStr(query, ";\n");
/*
--
2.30.2
From 326c489012f020f8ef38e1187aea72b332d1b9a4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 25 Mar 2021 12:08:51 +0100
Subject: [PATCH 6/6] fixup! Print publication WHERE condition in psql
Make psql backward compatible. Also add necessary parentheses.
---
src/bin/psql/describe.c | 11 +++++++----
src/test/regress/expected/publication.out | 2 +-
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 983ba512f7..6dce968414 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -6038,9 +6038,12 @@ describePublications(const char *pattern)
if (!puballtables)
{
printfPQExpBuffer(&buf,
- "SELECT n.nspname,
c.relname,\n"
- "
pg_get_expr(pr.prqual, c.oid)\n"
- "FROM
pg_catalog.pg_class c,\n"
+ "SELECT n.nspname,
c.relname");
+ if (pset.sversion >= 140000)
+ appendPQExpBuffer(&buf,
+ ",
pg_get_expr(pr.prqual, c.oid)");
+ appendPQExpBuffer(&buf,
+ "\nFROM
pg_catalog.pg_class c,\n"
"
pg_catalog.pg_namespace n,\n"
"
pg_catalog.pg_publication_rel pr\n"
"WHERE c.relnamespace
= n.oid\n"
@@ -6070,7 +6073,7 @@ describePublications(const char *pattern)
PQgetvalue(tabres, j, 1));
if (!PQgetisnull(tabres, j, 2))
- appendPQExpBuffer(&buf, " WHERE %s",
+ appendPQExpBuffer(&buf, " WHERE (%s)",
PQgetvalue(tabres, j, 2));
printTableAddFooter(&cont, buf.data);
diff --git a/src/test/regress/expected/publication.out
b/src/test/regress/expected/publication.out
index c8cf1b685e..da49a2e215 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -181,7 +181,7 @@ ERROR: cannot use a WHERE clause when removing table from
publication "testpub5
--------------------------+------------+---------+---------+---------+-----------+----------
regress_publication_user | f | t | t | t | t
| f
Tables:
- "public.testpub_rf_tbl3" WHERE ((e > 300) AND (e < 500))
+ "public.testpub_rf_tbl3" WHERE (((e > 300) AND (e < 500)))
DROP TABLE testpub_rf_tbl1;
DROP TABLE testpub_rf_tbl2;
--
2.30.2
