On 27.01.22 00:32, Tomas Vondra wrote:
On 1/26/22 14:01, Petr Jelinek wrote:
I would not remove it altogether, there is plenty of consumers of this
extension's output in the wild (even if I think it's
unfortunate) that might not be interested in sequences, but changing
it to off by default certainly makes sense.
Indeed. Attached is an updated patch series, with 0003 switching it to
false by default (and fixing the fallout). For commit I'll merge that
into 0002, of course.
(could be done in separate patches too IMO)
test_decoding.c uses %zu several times for int64 values, which is not
correct. This should use INT64_FORMAT or %lld with a cast to (long long
int).
I don't know, maybe it's worth commenting somewhere how the expected
values in contrib/test_decoding/expected/sequence.out come about.
Otherwise, it's quite a puzzle to determine where the 3830 comes from,
for example.
I think the skip-sequences options is better turned around into a
positive name like include-sequences. There is a mix of "skip" and
"include" options in test_decoding, but there are more "include" ones
right now.
In the 0003, a few files have been missed, apparently, so the tests
don't fully pass. See attached patch.
I haven't looked fully through the 0004 patch, but I notice that there
was a confusing mix of FOR ALL SEQUENCE and FOR ALL SEQUENCES. I have
corrected that in the other attached patch.
Other than the mentioned cosmetic issues, I think 0001-0003 are ready to go.
From d01e45ec0bba2afaeab95bceff0022fb32558ae0 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 27 Jan 2022 16:31:57 +0100
Subject: [PATCH 1/3] fixup! Change skip-sequences to false by default
---
contrib/test_decoding/expected/mxact.out | 8 ++++----
contrib/test_decoding/expected/ondisk_startup.out | 4 ++--
contrib/test_decoding/specs/mxact.spec | 2 +-
contrib/test_decoding/specs/ondisk_startup.spec | 2 +-
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/contrib/test_decoding/expected/mxact.out
b/contrib/test_decoding/expected/mxact.out
index 03ad3df099..89d4623e74 100644
--- a/contrib/test_decoding/expected/mxact.out
+++ b/contrib/test_decoding/expected/mxact.out
@@ -7,7 +7,7 @@ step s0init: SELECT 'init' FROM
pg_create_logical_replication_slot('isolation_sl
init
(1 row)
-step s0start: SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false');
+step s0start: SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false', 'skip-sequences', 'true');
data
----
(0 rows)
@@ -27,7 +27,7 @@ t
(1 row)
step s0w: INSERT INTO do_write DEFAULT VALUES;
-step s0start: SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false');
+step s0start: SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false', 'skip-sequences', 'true');
data
--------------------------------------------
BEGIN
@@ -50,7 +50,7 @@ step s0init: SELECT 'init' FROM
pg_create_logical_replication_slot('isolation_sl
init
(1 row)
-step s0start: SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false');
+step s0start: SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false', 'skip-sequences', 'true');
data
----
(0 rows)
@@ -71,7 +71,7 @@ t
step s0alter: ALTER TABLE do_write ADD column ts timestamptz;
step s0w: INSERT INTO do_write DEFAULT VALUES;
-step s0start: SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false');
+step s0start: SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false', 'skip-sequences', 'true');
data
------------------------------------------------------------------------------
BEGIN
diff --git a/contrib/test_decoding/expected/ondisk_startup.out
b/contrib/test_decoding/expected/ondisk_startup.out
index bc7ff07164..e8c47f0205 100644
--- a/contrib/test_decoding/expected/ondisk_startup.out
+++ b/contrib/test_decoding/expected/ondisk_startup.out
@@ -35,7 +35,7 @@ init
step s2c: COMMIT;
step s1insert: INSERT INTO do_write DEFAULT VALUES;
step s1checkpoint: CHECKPOINT;
-step s1start: SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false');
+step s1start: SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false', 'skip-sequences', 'true');
data
--------------------------------------------------------------------
BEGIN
@@ -46,7 +46,7 @@ COMMIT
step s1insert: INSERT INTO do_write DEFAULT VALUES;
step s1alter: ALTER TABLE do_write ADD COLUMN addedbys1 int;
step s1insert: INSERT INTO do_write DEFAULT VALUES;
-step s1start: SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false');
+step s1start: SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false', 'skip-sequences', 'true');
data
--------------------------------------------------------------------------------------------
BEGIN
diff --git a/contrib/test_decoding/specs/mxact.spec
b/contrib/test_decoding/specs/mxact.spec
index ea5b1aa2d6..c7ade959a5 100644
--- a/contrib/test_decoding/specs/mxact.spec
+++ b/contrib/test_decoding/specs/mxact.spec
@@ -13,7 +13,7 @@ teardown
session "s0"
setup { SET synchronous_commit=on; }
step "s0init" {SELECT 'init' FROM
pg_create_logical_replication_slot('isolation_slot', 'test_decoding');}
-step "s0start" {SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false');}
+step "s0start" {SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false', 'skip-sequences', 'true');}
step "s0alter" {ALTER TABLE do_write ADD column ts timestamptz; }
step "s0w" { INSERT INTO do_write DEFAULT VALUES; }
diff --git a/contrib/test_decoding/specs/ondisk_startup.spec
b/contrib/test_decoding/specs/ondisk_startup.spec
index 96ce87f1a4..e91f5868d4 100644
--- a/contrib/test_decoding/specs/ondisk_startup.spec
+++ b/contrib/test_decoding/specs/ondisk_startup.spec
@@ -16,7 +16,7 @@ session "s1"
setup { SET synchronous_commit=on; }
step "s1init" {SELECT 'init' FROM
pg_create_logical_replication_slot('isolation_slot', 'test_decoding');}
-step "s1start" {SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false');}
+step "s1start" {SELECT data FROM pg_logical_slot_get_changes('isolation_slot',
NULL, NULL, 'include-xids', 'false', 'skip-sequences', 'true');}
step "s1insert" { INSERT INTO do_write DEFAULT VALUES; }
step "s1checkpoint" { CHECKPOINT; }
step "s1alter" { ALTER TABLE do_write ADD COLUMN addedbys1 int; }
--
2.34.1
From 698603ee4d4b9ca953ad50935ce625fd9cba340f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 27 Jan 2022 16:35:18 +0100
Subject: [PATCH 3/3] fixup! Add support for decoding sequences to built-in
replication
---
doc/src/sgml/ref/alter_publication.sgml | 2 +-
src/backend/parser/gram.y | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/ref/alter_publication.sgml
b/doc/src/sgml/ref/alter_publication.sgml
index ca1ac3a996..9da8274ae2 100644
--- a/doc/src/sgml/ref/alter_publication.sgml
+++ b/doc/src/sgml/ref/alter_publication.sgml
@@ -33,7 +33,7 @@
TABLE [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ *
] [, ... ]
SEQUENCE <replaceable class="parameter">sequence_name</replaceable> [ * ]
[, ... ]
ALL TABLES IN SCHEMA { <replaceable
class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [, ... ]
- ALL SEQUENCE IN SCHEMA { <replaceable
class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [, ... ]
+ ALL SEQUENCES IN SCHEMA { <replaceable
class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [, ... ]
</synopsis>
</refsynopsisdiv>
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 83841e1eb9..d26528bd6e 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -9767,7 +9767,7 @@ PublicationObjSpec:
$$->pubtable =
makeNode(PublicationTable);
$$->pubtable->relation = $2;
}
- | ALL SEQUENCE IN_P SCHEMA ColId
+ | ALL SEQUENCES IN_P SCHEMA ColId
{
$$ = makeNode(PublicationObjSpec);
$$->pubobjtype =
PUBLICATIONOBJ_SEQUENCES_IN_SCHEMA;
--
2.34.1
