On Thu, May 29, 2025 at 8:09 PM vignesh C wrote:
>
> These comments are handled in the attached v2025029 version patch.
>
1. The current syntax to publish sequences is:
CREATE PUBLICATION pub1 FOR ALL TABLES, ALL SEQUENCES;
The other alternative could be:
CREATE PUBLICATION pub1 FOR ALL TABLES,
On Thu, May 29, 2025 at 8:09 PM vignesh C wrote:
> These comments are handled in the attached v2025029 version patch.
>
Thanks for the patches. I am still reviewing but please find few comments:
1)
Only persistent sequences are included in the publication. Temporary
sequences a
On Fri, May 23, 2025 at 3:12 AM vignesh C wrote:
>
>
> The attached v20250522 patch has the changes for the same.
>
> Regards,
> Vignesh
>
Some review comments for patch 0001:
1. In src/backend/commands/sequence.c
in pg_sequence_state()
+ /* open and lock sequence */
+ init_sequence(seq_relid, &
On Thu, May 22, 2025 at 10:42 PM vignesh C wrote:
>
>
> The attached v20250522 patch has the changes for the same.
>
Thank you for the patches, please find comments for patch-0004.
1)
+/*
+ * report_error_sequences
+ *
+ * Logs a warning listing all sequences that are missing on the publisher,
+
On Tue, 20 May 2025 at 09:54, shveta malik wrote:
>
> On Tue, May 20, 2025 at 8:35 AM Nisha Moond wrote:
> >
> > >
> > > Thanks for the comments, these are handled in the attached v20250516
> > > version patch.
> > >
> >
> > Thanks for the patches. Here are my review comments -
> >
> > Patch-0004
On Tue, May 20, 2025 at 11:13 AM Peter Smith wrote:
>
> > Test-scenario:
> > --Created 250 sequences on both pub and sub.
> > --There were 10 sequences mismatched.
> > --Sequence replication worked as expected. Logs look better now:
> >
> > LOG: Logical replication sequence synchronization for su
On Tue, May 20, 2025 at 8:35 AM Nisha Moond wrote:
>
> >
> > Thanks for the comments, these are handled in the attached v20250516
> > version patch.
> >
>
> Thanks for the patches. Here are my review comments -
>
> Patch-0004: src/backend/replication/logical/sequencesync.c
>
Hi,
Currently, the b
> Test-scenario:
> --Created 250 sequences on both pub and sub.
> --There were 10 sequences mismatched.
> --Sequence replication worked as expected. Logs look better now:
>
> LOG: Logical replication sequence synchronization for subscription
> "sub1" - total unsynchronized: 250; batch #1 = 100 att
On Tue, May 20, 2025 at 8:35 AM Nisha Moond wrote:
>
> >
> > Thanks for the comments, these are handled in the attached v20250516
> > version patch.
> >
>
> Thanks for the patches. Here are my review comments -
>
> Patch-0004: src/backend/replication/logical/sequencesync.c
>
> The sequence count l
>
> Thanks for the comments, these are handled in the attached v20250516
> version patch.
>
Thanks for the patches. Here are my review comments -
Patch-0004: src/backend/replication/logical/sequencesync.c
The sequence count logic using curr_seq in copy_sequences() seems buggy.
Currently, curr_se
Hi Vignesh.
Some minor review comments for the patches in set v20250514.
==
Patch 0001.
1.1
For function 'pg_sequence_state', the DOCS call the 2nd parameter
'sequence_name', but the pg_proc.dat file calls it 'seq_name'. Should
these be made the same?
Patch 0004.
pg_
On Wed, 14 May 2025 at 09:55, Nisha Moond wrote:
>
> On Sat, May 3, 2025 at 7:28 PM vignesh C wrote:
> >
> >
> > There was one pending open comment #6 from [1]. This has been
> > addressed in the attached patch.
>
> Thank you for the patches, here are my comments for patch-004: sequencesync.c
>
>
On Sat, May 3, 2025 at 7:28 PM vignesh C wrote:
>
>
> There was one pending open comment #6 from [1]. This has been
> addressed in the attached patch.
Thank you for the patches, here are my comments for patch-004: sequencesync.c
copy_sequences()
---
1)
+ if (first)
+ first = fals
On Sat, May 3, 2025 at 7:27 PM vignesh C wrote:
>
> >
> > Thanks for the comments, the updated patch has the changes for the same.
>
Thanks for the patches. Please find few comments:
1)
patch004 commit msg:
- Drop published sequences are removed from pg_subscription_rel.
Drop -->Dropped
2)
c
Hi Vignesh.
Some trivial review comments for DOCS patch v20250428-0005.
==
doc/src/sgml/logical-replication.sgml
1.
+ Publications may currently only contain tables or sequences. Objects must be
+ added explicitly, except when a publication is created using
+ FOR TABLES IN SCHEMA, or F
Hi Vignesh.
Some review comments for v20250426-0005.
==
doc/src/sgml/catalogs.sgml
1.
- State code:
+ State code for tables:
i = initialize,
d = data is being copied,
f = finished table copy,
s = synchronized,
r = ready (normal repl
Hi Vignesh.
FYI, patch v20250424-0004 reported whitespace errors when applied.
[postgres@CentOS7-x64 oss_postgres_misc]$ git apply
../patches_misc/v20250424-0004-Enhance-sequence-synchronization-during-su.patch
../patches_misc/v20250424-0004-Enhance-sequence-synchronization-during-su.patch:366:
t
Hi Vignesh,
Some comments for v20250422-0004.
==
src/backend/commands/sequence.c
pg_sequence_state:
1.
+ * The page_lsn will be utilized in logical replication sequence
+ * synchronization to record the page_lsn of sequence in the
pg_subscription_rel
+ * system catalog. It will reflect the
Hi Vignesh,
Some review comments for patch v20250422-0003.
==
src/backend/replication/logical/syncutils.c
1.
+/*
+ * Exit routine for synchronization worker.
+ */
+pg_noreturn void
+SyncFinishWorker(void)
Why does this have the pg_noreturn annotation? None of the other void
functions do.
~
Hi Vignesh.
Review comments for patch v20250422-0001.
==
Commit message
1.
This patch introduces a new function: pg_sequence_state function
allows retrieval of sequence values including LSN.
SUGGESTION
This patch introduces a new function, 'pg_sequence_state', which
allows retrieval of sequ
Hi Vignesh,
No comments for patch v20250416-0001
No comments for patch v20250416-0002
No comments for patch v20250416-0003
Here are some comments for patch v20250416-0004
==
src/backend/catalog/system_views.sql
1.
+CREATE VIEW pg_publication_sequences AS
+SELECT
+P.pubname AS pu
Hi Vignesh,
Some review comments for patch v20250416-0005 (docs)
==
doc/src/sgml/catalogs.sgml
(52.55. pg_subscription_rel)
1.
State code:
i = initialize,
- d = data is being copied,
- f = finished table copy,
- s = synchronized,
+ d = data is
Hi Vignesh,
Some review comments for patch v20250325-0005 (docs).
==
doc/src/sgml/catalogs.sgml
(52.55. pg_subscription_rel)
1.
State code: i = initialize, d = data is being copied, f = finished
table copy, s = synchronized, r = ready (normal replication)
~
This is not part of the patch,
Hi Vignesh,
Some review comments for v20250525-0004.
==
Commit message
1.
A new sequencesync worker is launched as needed to synchronize sequences.
It does the following:
a) Retrieves remote values of sequences with pg_sequence_state() INIT.
b) Log a warning if the sequence parameter
Hi Vignesh,
FYI, the patch v20250325-0004 failed to apply (atop 0001,0002,0002)
due to recent master changes.
Checking patch src/backend/commands/sequence.c...
error: while searching for:
(long long) minv, (long long) maxv)));
/* Set the currval() state only if iscalled = true */
if (iscalled)
{
Hi Vignesh,
Some review comments for patch v20250325-0002
==
Commit message
1.
Furthermore, enhancements to psql commands (\d and \dRp) now allow for better
display of publications containing specific sequences or sequences included
in a publication.
~
That doesn't seem as clear as it migh
Hi Vignesh,
Here are some review comments for patch v20250325-0001.
==
src/test/regress/expected/sequence.out
1.
+SELECT last_value, log_cnt, is_called FROM
pg_sequence_state('sequence_test');
+ last_value | log_cnt | is_called
++-+---
+ 99 | 32 | t
On Wed, 12 Mar 2025 at 09:14, vignesh C wrote:
>
> The patch was not applying on top of HEAD because of recent commits,
> here is a rebased version.
I have moved this to the next CommitFest since it will not be
committed in the current release. This also allows reviewers to focus
on the remaining
Hi Vignesh,
FYI, looks like your attached patchset was misnamed 20250204 instead
of 20250104. Anyway, it does not affect the reviews, but I am going to
refer to it as 0104 from now.
~~~
I have no comments for the patch v20250104-0001.
Some comments for the patch v20250104-0002.
==
doc/src/
On Fri, 3 Jan 2025 at 09:07, Peter Smith wrote:
>
> Hi Vignesh,
>
> Some minor review comments for the patch v20241230-0003.
>
> ==
> src/backend/replication/logical/syncutils.c
>
> 1.
> + * syncutils.c
> + * PostgreSQL logical replication: common synchronization code
> + *
> + * Copyright (
Hi Vignesh,
Some minor review comments for the patch v20241230-0003.
==
src/backend/replication/logical/syncutils.c
1.
+ * syncutils.c
+ * PostgreSQL logical replication: common synchronization code
+ *
+ * Copyright (c) 2024, PostgreSQL Global Development Group
Happy New Year.
s/2024/20
Hi Vignesh.
Here are some review comments for patch v20241230-0002
==
1. SYNTAX
The proposed syntax is currently:
CREATE PUBLICATION name
[ FOR ALL object_type [, ...]
| FOR publication_object [, ... ] ]
[ WITH ( publication_parameter [= value] [, ... ] ) ]
where object_type
Dear Vignesh,
Thanks for updating the patch! Here are my comments.
01. SyncingRelationsState
```
* SYNC_RELATIONS_STATE_NEEDS_REBUILD The subscription relations state is no
* longer valid and the subscription relations should be rebuilt.
```
Can we follow the style like other lines? Like:
SY
On Fri, 20 Dec 2024 at 08:05, Peter Smith wrote:
>
> Hi Vignesh.
>
> Here are some review comments for the patch v20241211-0005.
>
> ==
>
> Section "29.6.3. Examples"
>
> 2.
> Should the Examples section also have an example of ALTER SUBSCRIPTION
> ... REFRESH PUBLICATION to demonstrate (like
On Thu, 19 Dec 2024 at 04:58, Peter Smith wrote:
>
> Hi Vignesh,
>
> Here are some review comments for the patch v20241211-0003.
>
> ~~~
>
> 4.
> +/*
> + * Common code to fetch the up-to-date sync state info into the static lists.
> + *
> + * Returns true if subscription has 1 or more tables, else
Hi Vignesh.
Here are some review comments for the patch v20241211-0005.
==
doc/src/sgml/logical-replication.sgml
Section "29.6.1. Sequence Definition Mismatches"
1.
+
+
+ If there are differences in sequence definitions between the publisher and
+ subscriber, a WARNING is log
Hi Vignesh,
Here are some review comments for the patch v20241211-0004.
==
GENERAL
1.
There are more than a dozen places where the relation (relkind) is
checked to see if it is a SEQUENCE:
e.g. + get_rel_relkind(subrel->srrelid) != RELKIND_SEQUENCE &&
e.g. + if (get_rel_relkind(subrel->srre
Hi Vignesh,
Here are some review comments for the patch v20241211-0003.
==
src/backend/replication/logical/syncutils.c
1.
+typedef enum
+{
+ SYNC_RELATIONS_STATE_NEEDS_REBUILD,
+ SYNC_RELATIONS_STATE_REBUILD_STARTED,
+ SYNC_RELATIONS_STATE_VALID,
+} SyncingRelationsState;
+
Even though the
Hi Vignesh,
Here are some review comments for the patch v20241211-0002.
==
doc/src/sgml/ref/create_publication.sgml
1.
+where object
type is one of:
+
+TABLES
+SEQUENCES
The replaceable "object_type" is missing an underscore.
~~~
publish option effect fro SEQUENCE replication?
2.
Hi Vignesh.
Here are some review comments for patch v20241211-0001.
==
src/backend/commands/sequence.c
pg_sequence_state:
1.
+ TupleDesc tupdesc;
+ HeapTuple tuple;
+ Datum values[4];
+ bool nulls[4] = {false, false, false, false};
SUGGESTION
bool nulls[4] = {0};
The above achieves the s
On Tue, Oct 8, 2024 at 2:46 AM vignesh C wrote:
>
> On Fri, 4 Oct 2024 at 15:39, shveta malik wrote:
> >
> > On Sun, Sep 29, 2024 at 12:34 PM vignesh C wrote:
> > >
> > > On Thu, 26 Sept 2024 at 11:07, shveta malik
> > > wrote:
> > > >
> > > > On Fri, Sep 20, 2024 at 9:36 AM vignesh C wrote:
On Sun, Sep 29, 2024 at 12:34 PM vignesh C wrote:
>
> On Thu, 26 Sept 2024 at 11:07, shveta malik wrote:
> >
> > On Fri, Sep 20, 2024 at 9:36 AM vignesh C wrote:
> > >
> > > On Wed, 21 Aug 2024 at 11:54, vignesh C wrote:
> > > >
> > > > On Wed, 21 Aug 2024 at 08:33, Peter Smith wrote:
> > > >
On Thu, 26 Sept 2024 at 11:07, shveta malik wrote:
>
> On Fri, Sep 20, 2024 at 9:36 AM vignesh C wrote:
> >
> > On Wed, 21 Aug 2024 at 11:54, vignesh C wrote:
> > >
> > > On Wed, 21 Aug 2024 at 08:33, Peter Smith wrote:
> > > >
> > > > Hi Vignesh, Here are my only review comments for the latest
On Fri, Sep 20, 2024 at 9:36 AM vignesh C wrote:
>
> On Wed, 21 Aug 2024 at 11:54, vignesh C wrote:
> >
> > On Wed, 21 Aug 2024 at 08:33, Peter Smith wrote:
> > >
> > > Hi Vignesh, Here are my only review comments for the latest patch set.
> >
> > Thanks, these issues have been addressed in the
Hi Vignesh, Here are my only review comments for the latest patch set.
v20240820-0003.
nit - missing period for comment in FetchRelationStates
nit - typo in function name 'ProcessSyncingTablesFoSync'
==
Kind Regards,
Peter Smith.
Fujitsu Australia
diff --git a/src/backend/replication/logical
Hi Vignesh, Here are my review comments for the latest patchset
v20240819-0001. No changes. No comments.
v20240819-0002. No changes. No comments.
v20240819-0003. See below.
v20240819-0004. See below.
v20240819-0005. No changes. No comments.
///
PATCH v20240819-0003
==
sr
Here are my review comments for the latest patchset
v20240817-0001. No changes. No comments.
v20240817-0002. No changes. No comments.
v20240817-0003. See below.
v20240817-0004. See below.
v20240817-0005. No changes. No comments.
//
v20240817-0003 and 0004.
(This is a repeat of the same comm
Hi Vignesh. I looked at the latest v20240815* patch set.
I have only the following few comments for patch v20240815-0004, below.
==
Commit message.
Please see the attachment for some suggested updates.
==
src/backend/commands/subscriptioncmds.c
CreateSubscription:
nit - fix wording in
Hi Vignesh, I have reviewed your latest patchset:
v20240814-0001. No comments
v20240814-0002. No comments
v20240814-0003. No comments
v20240814-0004. See below
v20240814-0005. No comments
//
v20240814-0004.
==
src/backend/commands/subscriptioncmds.c
CreateSubscription:
nit - XXX commen
Hi Vignesh, Here are my review comments for the latest patchset:
Patch v20240813-0001. No comments
Patch v20240813-0002. No comments
Patch v20240813-0003. No comments
Patch v20240813-0004. See below
Patch v20240813-0005. No comments
//
Patch v20240813-0004
==
src/backend/catalog/pg_subs
On Tue, Aug 13, 2024 at 10:00 PM vignesh C wrote:
>
> On Tue, 13 Aug 2024 at 09:19, Peter Smith wrote:
> >
> > 3.1. GENERAL
> >
> > Hmm. I am guessing this was provided as a separate patch to aid review
> > by showing that existing functions are moved? OTOH you can't really
> > judge this patch p
On Tue, 13 Aug 2024 at 12:31, Peter Smith wrote:
>
> OBSERVATION #2
>
> When 1000s of sequences are refreshed (set to INIT) then there are
> 1000s of logs like below:
>
> ...
> 2024-08-13 16:13:57.873 AEST [10301] LOG: sequence "public.seq_0698"
> of subscription "sub3" set to INIT state
> 2024-0
Hi Vignesh,
I have been using the latest patchset, trying a few things using many
(1000) sequences.
Here are some observations, plus some suggestions for consideration.
~
OBSERVATION #1
When 1000s of sequences are refreshed using REFRESH PUBLICATION
SEQUENCES the logging is excessive. For
On Mon, Aug 12, 2024 at 11:07 PM vignesh C wrote:
>
> On Mon, 12 Aug 2024 at 10:40, Peter Smith wrote:
> >
> > Hi Vignesh,
> >
> > I found that when 2 subscriptions are both subscribing to a
> > publication publishing sequences, an ERROR occurs on refresh.
> >
> > ==
> >
> > Publisher:
> > --
Hi Vignesh, Here are my review comments for latest v20240812* patchset:
patch v20240812-0001. No comments.
patch v20240812-0002. Fixed docs.LGTM
patch v20240812-0003. This is new refactoring. See below.
patch v20240812-0004. (was 0003). See below.
patch v20240812-0005. (was 0004). No comments.
//
On Mon, 12 Aug 2024 at 09:59, Peter Smith wrote:
>
> Hi Vignesh,
>
> I noticed it is not currently possible (there is no syntax way to do
> it) to ALTER an existing publication so that it will publish
> SEQUENCES.
>
> Isn't that a limitation? Why?
>
> For example,. Why should users be prevented fr
On Mon, 12 Aug 2024 at 10:40, Peter Smith wrote:
>
> Hi Vignesh,
>
> I found that when 2 subscriptions are both subscribing to a
> publication publishing sequences, an ERROR occurs on refresh.
>
> ==
>
> Publisher:
> --
>
> test_pub=# create publication pub1 for all sequences;
>
> Subs
Hi Vignesh,
I found that when 2 subscriptions are both subscribing to a
publication publishing sequences, an ERROR occurs on refresh.
==
Publisher:
--
test_pub=# create publication pub1 for all sequences;
Subscriber:
---
test_sub=# create subscription sub1 connection 'dbna
Hi Vignesh,
I noticed it is not currently possible (there is no syntax way to do
it) to ALTER an existing publication so that it will publish
SEQUENCES.
Isn't that a limitation? Why?
For example,. Why should users be prevented from changing a FOR ALL
TABLES publication into a FOR ALL TABLES, SEQ
Hi Vignesh,
v20240809-0001. No comments.
v20240809-0002. See below.
v20240809-0003. See below.
v20240809-0004. No comments.
//
Here are my review comments for patch v20240809-0002.
nit - Tweak wording in new docs example, because a publication only
publishes the sequences; it doesn't "s
On Fri, 9 Aug 2024 at 12:40, Peter Smith wrote:
>
> Hi Vignesh, here are my review comments for the sequences docs patch
> v20240808-0004.
>
> ==
> doc/src/sgml/logical-replication.sgml
>
> The new section content looked good.
>
> Just some nitpicks including:
> - renamed the section "Replicat
On Fri, 9 Aug 2024 at 05:51, Peter Smith wrote:
>
> Hi Vignesh, I reviewed the latest v20240808-0003 patch.
>
> Attached are my minor change suggestions.
Thanks, these changes are merged in the v20240809 version posted at [1].
[1] -
https://www.postgresql.org/message-id/CALDaNm0LJCtGoBCO6DFY-RDj
On Fri, 9 Aug 2024 at 12:13, shveta malik wrote:
>
> On Wed, Aug 7, 2024 at 2:00 PM vignesh C wrote:
> >
> > On Wed, 7 Aug 2024 at 08:09, Amit Kapila wrote:
> > >
> > > On Tue, Aug 6, 2024 at 5:13 PM vignesh C wrote:
> > > >
> > > > On Mon, 5 Aug 2024 at 18:05, shveta malik
> > > > wrote:
> >
Hi Vignesh, here are my review comments for the sequences docs patch
v20240808-0004.
==
doc/src/sgml/logical-replication.sgml
The new section content looked good.
Just some nitpicks including:
- renamed the section "Replicating Sequences"
- added missing mention about how to publish sequence
On Wed, Aug 7, 2024 at 2:00 PM vignesh C wrote:
>
> On Wed, 7 Aug 2024 at 08:09, Amit Kapila wrote:
> >
> > On Tue, Aug 6, 2024 at 5:13 PM vignesh C wrote:
> > >
> > > On Mon, 5 Aug 2024 at 18:05, shveta malik wrote:
> > > >
> > > > On Mon, Aug 5, 2024 at 11:04 AM vignesh C wrote:
> > > > >
>
Hi Vignesh, I reviewed the latest v20240808-0003 patch.
Attached are my minor change suggestions.
==
Kind Regards,
Peter Smith.
Fujitsu Australia
diff --git a/src/backend/catalog/pg_subscription.c
b/src/backend/catalog/pg_subscription.c
index 4e2f960..a77e810 100644
--- a/src/backend/catalog
On Wed, 7 Aug 2024 at 10:27, shveta malik wrote:
>
> On Mon, Aug 5, 2024 at 10:26 AM vignesh C wrote:
> >
> > On Thu, 1 Aug 2024 at 04:25, Peter Smith wrote:
> > >
> > > Hi Vignesh,
> > >
> > > I noticed that when replicating sequences (using the latest patches
> > > 0730_2*) the subscriber-sid
On Thu, Aug 8, 2024 at 11:09 AM Peter Smith wrote:
>
> But, I haven't invented a new overloading for "copy_data" option
> (meaning "synchronize") for sequences. The current patchset already
> interprets copy_data exactly this way.
>
> For example, below are patch 0003 results:
>
> ALTER SUBSCRIPTI
On Wed, Aug 7, 2024 at 1:45 PM vignesh C wrote:
>
>
> The remaining comments have been addressed, and the changes are
> included in the attached v20240807 version patch.
Thanks for addressing the comment. Please find few comments for v20240807 :
patch002:
1)
create_publication.sgml:
--I think i
On Thu, Aug 8, 2024 at 1:55 PM Amit Kapila wrote:
>
> On Wed, Aug 7, 2024 at 10:12 AM Peter Smith wrote:
> >
> > This is mostly a repeat of my previous mail from a while ago [1] but
> > includes some corrections, answers, and more examples. I'm going to
> > try to persuade one last time because t
On Wed, Aug 7, 2024 at 10:12 AM Peter Smith wrote:
>
> This is mostly a repeat of my previous mail from a while ago [1] but
> includes some corrections, answers, and more examples. I'm going to
> try to persuade one last time because the current patch is becoming
> stable, so I wanted to revisit t
Hi Vignesh, Here are my v20240807-0003 review comments.
==
1. GENERAL DOCS.
IMO the replication of SEQUENCES is a big enough topic that it
deserves to have its own section in the docs chapter 31 [1].
Some of the create/alter subscription docs content would stay where it
is in, but a new chap
On Wed, 7 Aug 2024 at 08:09, Amit Kapila wrote:
>
> On Tue, Aug 6, 2024 at 5:13 PM vignesh C wrote:
> >
> > On Mon, 5 Aug 2024 at 18:05, shveta malik wrote:
> > >
> > > On Mon, Aug 5, 2024 at 11:04 AM vignesh C wrote:
> > > >
> > > > On Wed, 31 Jul 2024 at 14:39, shveta malik
> > > > wrote:
>
On Mon, 5 Aug 2024 at 17:28, Amit Kapila wrote:
>
> On Mon, Aug 5, 2024 at 2:36 PM vignesh C wrote:
> >
> > On Fri, 2 Aug 2024 at 14:24, shveta malik wrote:
> > >
> > > On Thu, Aug 1, 2024 at 9:26 AM shveta malik
> > > wrote:
> > > >
> > > > On Mon, Jul 29, 2024 at 4:17 PM vignesh C wrote:
>
On Tue, 6 Aug 2024 at 09:28, shveta malik wrote:
>
> On Tue, Aug 6, 2024 at 8:49 AM shveta malik wrote:
> >
>
> Do we need some kind of coordination between table sync and sequence
> sync for internally generated sequences? Lets say we have an identity
> column with a 'GENERATED ALWAYS' sequence.
On Mon, Aug 5, 2024 at 10:26 AM vignesh C wrote:
>
> On Thu, 1 Aug 2024 at 04:25, Peter Smith wrote:
> >
> > Hi Vignesh,
> >
> > I noticed that when replicating sequences (using the latest patches
> > 0730_2*) the subscriber-side checks the *existence* of the sequence,
> > but apparently it is n
Hi Vignesh,
This is mostly a repeat of my previous mail from a while ago [1] but
includes some corrections, answers, and more examples. I'm going to
try to persuade one last time because the current patch is becoming
stable, so I wanted to revisit this syntax proposal before it gets too
late to ch
On Tue, Aug 6, 2024 at 5:13 PM vignesh C wrote:
>
> On Mon, 5 Aug 2024 at 18:05, shveta malik wrote:
> >
> > On Mon, Aug 5, 2024 at 11:04 AM vignesh C wrote:
> > >
> > > On Wed, 31 Jul 2024 at 14:39, shveta malik wrote:
> > > >
> > > > On Mon, Jun 10, 2024 at 5:00 PM vignesh C wrote:
> > > > >
On Mon, 5 Aug 2024 at 18:05, shveta malik wrote:
>
> On Mon, Aug 5, 2024 at 11:04 AM vignesh C wrote:
> >
> > On Wed, 31 Jul 2024 at 14:39, shveta malik wrote:
> > >
> > > On Mon, Jun 10, 2024 at 5:00 PM vignesh C wrote:
> > > >
> > > > On Mon, 10 Jun 2024 at 12:24, Amul Sul wrote:
> > > > >
>
Here are some review comments for the patch v20240805_2-0003.
==
doc/src/sgml/catalogs.sgml
nitpick - removed the word "either"
==
doc/src/sgml/ref/alter_subscription.sgml
I felt the discussions about "how to handle warnings" are a bit scattered:
e.g.1 - ALTER SUBSCRIPTION REFRESH PUBLI
On Tue, 6 Aug 2024 at 10:24, shveta malik wrote:
>
> On Tue, Aug 6, 2024 at 9:54 AM Amit Kapila wrote:
> >
> > On Tue, Aug 6, 2024 at 8:49 AM shveta malik wrote:
> > >
> > > > > > I was reviewing the design of patch003, and I have a query. Do we
> > > > > > need
> > > > > > to even start an app
On Tue, Aug 6, 2024 at 9:54 AM Amit Kapila wrote:
>
> On Tue, Aug 6, 2024 at 8:49 AM shveta malik wrote:
> >
> > > > > I was reviewing the design of patch003, and I have a query. Do we need
> > > > > to even start an apply worker and create replication slot when
> > > > > subscription created is
On Tue, Aug 6, 2024 at 8:49 AM shveta malik wrote:
>
> On Mon, Aug 5, 2024 at 5:28 PM Amit Kapila wrote:
> >
> > On Mon, Aug 5, 2024 at 2:36 PM vignesh C wrote:
> > >
> > > On Fri, 2 Aug 2024 at 14:24, shveta malik wrote:
> > > >
> > > > On Thu, Aug 1, 2024 at 9:26 AM shveta malik
> > > > wro
On Tue, Aug 6, 2024 at 8:49 AM shveta malik wrote:
>
Do we need some kind of coordination between table sync and sequence
sync for internally generated sequences? Lets say we have an identity
column with a 'GENERATED ALWAYS' sequence. When the sequence is synced
to subscriber, subscriber can also
On Mon, Aug 5, 2024 at 5:28 PM Amit Kapila wrote:
>
> On Mon, Aug 5, 2024 at 2:36 PM vignesh C wrote:
> >
> > On Fri, 2 Aug 2024 at 14:24, shveta malik wrote:
> > >
> > > On Thu, Aug 1, 2024 at 9:26 AM shveta malik
> > > wrote:
> > > >
> > > > On Mon, Jul 29, 2024 at 4:17 PM vignesh C wrote:
On Mon, Aug 5, 2024 at 11:04 AM vignesh C wrote:
>
> On Wed, 31 Jul 2024 at 14:39, shveta malik wrote:
> >
> > On Mon, Jun 10, 2024 at 5:00 PM vignesh C wrote:
> > >
> > > On Mon, 10 Jun 2024 at 12:24, Amul Sul wrote:
> > > >
> > > >
> > > >
> > > > On Sat, Jun 8, 2024 at 6:43 PM vignesh C wro
On Mon, Aug 5, 2024 at 2:36 PM vignesh C wrote:
>
> On Fri, 2 Aug 2024 at 14:24, shveta malik wrote:
> >
> > On Thu, Aug 1, 2024 at 9:26 AM shveta malik wrote:
> > >
> > > On Mon, Jul 29, 2024 at 4:17 PM vignesh C wrote:
> > > >
> > > > Thanks for reporting this, these issues are fixed in the a
On Fri, 2 Aug 2024 at 14:33, shveta malik wrote:
>
> On Fri, Aug 2, 2024 at 2:24 PM shveta malik wrote:
> >
> > On Thu, Aug 1, 2024 at 9:26 AM shveta malik wrote:
> > >
> > > On Mon, Jul 29, 2024 at 4:17 PM vignesh C wrote:
> > > >
> > > > Thanks for reporting this, these issues are fixed in th
On Fri, 2 Aug 2024 at 14:24, shveta malik wrote:
>
> On Thu, Aug 1, 2024 at 9:26 AM shveta malik wrote:
> >
> > On Mon, Jul 29, 2024 at 4:17 PM vignesh C wrote:
> > >
> > > Thanks for reporting this, these issues are fixed in the attached
> > > v20240730_2 version patch.
> > >
>
> I was reviewin
On Wed, 31 Jul 2024 at 14:39, shveta malik wrote:
>
> On Mon, Jun 10, 2024 at 5:00 PM vignesh C wrote:
> >
> > On Mon, 10 Jun 2024 at 12:24, Amul Sul wrote:
> > >
> > >
> > >
> > > On Sat, Jun 8, 2024 at 6:43 PM vignesh C wrote:
> > >>
> > >> On Wed, 5 Jun 2024 at 14:11, Amit Kapila wrote:
> >
On Thu, 1 Aug 2024 at 04:25, Peter Smith wrote:
>
> Hi Vignesh,
>
> I noticed that when replicating sequences (using the latest patches
> 0730_2*) the subscriber-side checks the *existence* of the sequence,
> but apparently it is not checking other sequence attributes.
>
> For example, consider:
On Thu, 1 Aug 2024 at 03:33, Peter Smith wrote:
>
> Hi Vignesh,
>
> I have a question about the subscriber-side behaviour of currval().
>
> ==
>
> AFAIK it is normal for currval() to give error is nextval() has not
> yet been called [1]
>
> For example.
> test_pub=# create sequence s1;
> CREAT
On Fri, Aug 2, 2024 at 2:24 PM shveta malik wrote:
>
> On Thu, Aug 1, 2024 at 9:26 AM shveta malik wrote:
> >
> > On Mon, Jul 29, 2024 at 4:17 PM vignesh C wrote:
> > >
> > > Thanks for reporting this, these issues are fixed in the attached
> > > v20240730_2 version patch.
> > >
>
> I was review
On Thu, Aug 1, 2024 at 9:26 AM shveta malik wrote:
>
> On Mon, Jul 29, 2024 at 4:17 PM vignesh C wrote:
> >
> > Thanks for reporting this, these issues are fixed in the attached
> > v20240730_2 version patch.
> >
I was reviewing the design of patch003, and I have a query. Do we need
to even star
Hi Vignesh,
I noticed that when replicating sequences (using the latest patches
0730_2*) the subscriber-side checks the *existence* of the sequence,
but apparently it is not checking other sequence attributes.
For example, consider:
Publisher: "CREATE SEQUENCE s1 START 1 INCREMENT 2;" should be
Hi Vignesh,
I have a question about the subscriber-side behaviour of currval().
==
AFAIK it is normal for currval() to give error is nextval() has not
yet been called [1]
For example.
test_pub=# create sequence s1;
CREATE SEQUENCE
test_pub=# select * from currval('s1');
2024-08-01 07:42:48.
On Sat, 20 Jul 2024 at 20:48, vignesh C wrote:
>
> On Fri, 12 Jul 2024 at 08:22, Peter Smith wrote:
> >
> > Hi Vignesh. Here are the rest of my comments for patch v20240705-0003.
> > ==
> >
> > 8. logicalrep_sequence_sync_worker_find
> >
> > +/*
> > + * Walks the workers array and searches fo
On Mon, Jun 10, 2024 at 5:00 PM vignesh C wrote:
>
> On Mon, 10 Jun 2024 at 12:24, Amul Sul wrote:
> >
> >
> >
> > On Sat, Jun 8, 2024 at 6:43 PM vignesh C wrote:
> >>
> >> On Wed, 5 Jun 2024 at 14:11, Amit Kapila wrote:
> >> [...]
> >> A new catalog table, pg_subscription_seq, has been introdu
Hi Vignesh,
Here are my review comments for your latest 0730_2* patches.
Patch v20240730_2-0001 looks good to me.
Patch v20240730_2-0002 looks good to me.
My comments for the v20240730_2-0003 patch are below:
//
GENERAL
1. Inconsistent terms
I've noticed there are many variations of
On Thu, 25 Jul 2024 at 15:41, shveta malik wrote:
>
> On Thu, Jul 25, 2024 at 12:08 PM shveta malik wrote:
> >
> > On Thu, Jul 25, 2024 at 9:06 AM vignesh C wrote:
> > >
> > > The attached v20240725 version patch has the changes for the same.
> >
> > Thank You for addressing the comments. Please
1 - 100 of 180 matches
Mail list logo
