On Fri, Dec 6, 2024 at 11:10 AM Shlok Kyal wrote:
>
> Thanks Peter, for pointing this out.
> I also feel that the error message suggested by Amit would be better.
> I have attached a patch for the same.
>
Pushed.
--
With Regards,
Amit Kapila.
On Fri, 6 Dec 2024 at 02:44, Peter Smith wrote:
>
> On Thu, Dec 5, 2024 at 8:49 PM Amit Kapila wrote:
> >
> > On Thu, Dec 5, 2024 at 9:32 AM Peter Smith wrote:
> > >
> > > On Thu, Dec 5, 2024 at 2:41 PM Amit Kapila
> > > wrote:
> > > >
> > > > On Thu, Dec 5, 2024 at 7:34 AM Peter Smith
> > >
On Thu, Dec 5, 2024 at 8:49 PM Amit Kapila wrote:
>
> On Thu, Dec 5, 2024 at 9:32 AM Peter Smith wrote:
> >
> > On Thu, Dec 5, 2024 at 2:41 PM Amit Kapila wrote:
> > >
> > > On Thu, Dec 5, 2024 at 7:34 AM Peter Smith wrote:
> > > >
> > > > IIUC, these errors are intended for when there is *any*
On Thu, Dec 5, 2024 at 9:32 AM Peter Smith wrote:
>
> On Thu, Dec 5, 2024 at 2:41 PM Amit Kapila wrote:
> >
> > On Thu, Dec 5, 2024 at 7:34 AM Peter Smith wrote:
> > >
> > > IIUC, these errors are intended for when there is *any* unpublished
> > > generated column found in the RI, and the RI mig
On Thu, Dec 5, 2024 at 2:41 PM Amit Kapila wrote:
>
> On Thu, Dec 5, 2024 at 7:34 AM Peter Smith wrote:
> >
> > IIUC, these errors are intended for when there is *any* unpublished
> > generated column found in the RI, and the RI might also have other
> > columns in it generated or otherwise. So,
On Thu, Dec 5, 2024 at 7:34 AM Peter Smith wrote:
>
> IIUC, these errors are intended for when there is *any* unpublished
> generated column found in the RI, and the RI might also have other
> columns in it generated or otherwise. So, I think those error messages
> saying "consists of" should be r
Hi,
I was looking at the recently pushed code [1]. IMO the wording of some
of those new error messages of function CheckCmdReplicaIdentity() is
not quite correct.
According to my understanding, and according also to Chat-GPT:
--
The sentence "Replica identity consists of an unpublished genera
On Tue, Dec 3, 2024 at 12:31 PM Shlok Kyal wrote:
>
> The changes look good to me. I have included it in the updated patch.
>
The patch looks mostly good to me. I have updated the docs, comments,
and some other cosmetic changes. Please see attached and let me know
what you think.
--
With Regard
On Tue, 3 Dec 2024 at 10:21, Zhijie Hou (Fujitsu)
wrote:
>
> On Friday, November 29, 2024 9:08 PM Shlok Kyal
> wrote:
> >
> >
> > Thanks for reviewing the patch. I have fixed the issue and updated the
> > patch.
>
> Thanks for updating the patch. I have reviewed and have few suggestions.
>
> Pl
On Friday, November 29, 2024 9:08 PM Shlok Kyal
wrote:
>
>
> Thanks for reviewing the patch. I have fixed the issue and updated the patch.
Thanks for updating the patch. I have reviewed and have few suggestions.
Please check the attached diff which includes:
1) Comments in CheckCmdReplicaIde
On Fri, 29 Nov 2024 at 15:49, vignesh C wrote:
>
> On Fri, 29 Nov 2024 at 13:38, Shlok Kyal wrote:
> >
> > On Thu, 28 Nov 2024 at 16:38, Amit Kapila wrote:
> > >
> > > On Thu, Nov 21, 2024 at 5:30 PM Shlok Kyal
> > > wrote:
> > > >
> > >
> > > Review comments:
> > > ===
> > > 1.
>
On Fri, 29 Nov 2024 at 13:38, Shlok Kyal wrote:
>
> On Thu, 28 Nov 2024 at 16:38, Amit Kapila wrote:
> >
> > On Thu, Nov 21, 2024 at 5:30 PM Shlok Kyal wrote:
> > >
> >
> > Review comments:
> > ===
> > 1.
> > +
> > + /*
> > + * true if all generated columns which are part of replica
On Thu, 28 Nov 2024 at 16:38, Amit Kapila wrote:
>
> On Thu, Nov 21, 2024 at 5:30 PM Shlok Kyal wrote:
> >
>
> Review comments:
> ===
> 1.
> +
> + /*
> + * true if all generated columns which are part of replica identity are
> + * published or the publication actions do not include UP
On Thu, Nov 21, 2024 at 5:30 PM Shlok Kyal wrote:
>
Review comments:
===
1.
+
+ /*
+ * true if all generated columns which are part of replica identity are
+ * published or the publication actions do not include UPDATE or DELETE.
+ */
+ bool replident_valid_for_update;
+ bool repliden
On Thu, 21 Nov 2024 at 15:26, vignesh C wrote:
>
> On Tue, 19 Nov 2024 at 19:12, Shlok Kyal wrote:
> >
> > On Tue, 19 Nov 2024 at 14:39, Zhijie Hou (Fujitsu)
> > wrote:
> > >
> > > On Tuesday, November 19, 2024 3:15 PM Shlok Kyal
> > > wrote:
> > >
> > > >
> > > > I noticed that we can add 'pu
On Tue, 19 Nov 2024 at 19:12, Shlok Kyal wrote:
>
> On Tue, 19 Nov 2024 at 14:39, Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Tuesday, November 19, 2024 3:15 PM Shlok Kyal
> > wrote:
> >
> > >
> > > I noticed that we can add 'publish_generated_columns = true' for the case
> > > of
> > > generated
On Tuesday, November 19, 2024 9:42 PM Shlok Kyal
wrote:
> I agree that we can remove the test. I debugged and found the test modified in
> above patch does not hit the condition added in commit adedf54.
> Also, according to me we cannot trigger the bug after the fix in this thread.
> So, I
> thi
On Tue, 19 Nov 2024 at 14:39, Zhijie Hou (Fujitsu)
wrote:
>
> On Tuesday, November 19, 2024 3:15 PM Shlok Kyal
> wrote:
>
> >
> > I noticed that we can add 'publish_generated_columns = true' for the case of
> > generated column. So we won't need to remove the test. I have made the
> > changes in
On Tuesday, November 19, 2024 5:10 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Tuesday, November 19, 2024 3:15 PM Shlok Kyal
> wrote:
>
> >
> > I noticed that we can add 'publish_generated_columns = true' for the case of
> > generated column. So we won't need to remove the test. I have made the
> >
On Tuesday, November 19, 2024 3:15 PM Shlok Kyal
wrote:
>
> I noticed that we can add 'publish_generated_columns = true' for the case of
> generated column. So we won't need to remove the test. I have made the
> changes in v9 patch [1].
I think this would unexpectedly change the original purpo
On Tue, 19 Nov 2024 at 10:22, vignesh C wrote:
>
> On Tue, 19 Nov 2024 at 00:36, Shlok Kyal wrote:
> >
> > On Mon, 18 Nov 2024 at 19:19, vignesh C wrote:
> > >
> > > On Mon, 18 Nov 2024 at 13:07, Shlok Kyal wrote:
> > > >
> > > > Thanks for providing the comments.
> > > >
> > > > On Sat, 16 Nov
On Tue, 19 Nov 2024 at 09:50, Zhijie Hou (Fujitsu)
wrote:
>
> On Tuesday, November 19, 2024 3:06 AM Shlok Kyal
> wrote:
> >
> > I have fixed the comments and attached an updated patch.
>
> Thanks for the patch.
>
> I slightly refactored the codes a bit:
>
> * make the codes in replident_has_unpu
On Tue, 19 Nov 2024 at 00:36, Shlok Kyal wrote:
>
> On Mon, 18 Nov 2024 at 19:19, vignesh C wrote:
> >
> > On Mon, 18 Nov 2024 at 13:07, Shlok Kyal wrote:
> > >
> > > Thanks for providing the comments.
> > >
> > > On Sat, 16 Nov 2024 at 17:29, vignesh C wrote:
> > >
> > > I have attached the up
On Tuesday, November 19, 2024 3:06 AM Shlok Kyal
wrote:
>
> I have fixed the comments and attached an updated patch.
Thanks for the patch.
I slightly refactored the codes a bit:
* make the codes in replident_has_unpublished_gen_col()
consistent with other similar functions.
* Avoid unnecessa
On Mon, 18 Nov 2024 at 19:19, vignesh C wrote:
>
> On Mon, 18 Nov 2024 at 13:07, Shlok Kyal wrote:
> >
> > Thanks for providing the comments.
> >
> > On Sat, 16 Nov 2024 at 17:29, vignesh C wrote:
> >
> > I have attached the updated version of the patch.
>
> Few comments:
> 1) We have the follow
On Mon, 18 Nov 2024 at 13:07, Shlok Kyal wrote:
>
> Thanks for providing the comments.
>
> On Sat, 16 Nov 2024 at 17:29, vignesh C wrote:
>
> I have attached the updated version of the patch.
Few comments:
1) We have the following check for cols validation and rf validation:
/*
* If we
On Saturday, November 16, 2024 2:41 AM Shlok Kyal
wrote:
>
> >
> Thanks for providing the comments. I have fixed all the comments and attached
> the updated patch.
Thanks for the patch. I have one comment for the following codes:
+ /*
+* Bitmap of published column
On Mon, 18 Nov 2024 at 08:57, Zhijie Hou (Fujitsu)
wrote:
>
> On Saturday, November 16, 2024 2:41 AM Shlok Kyal
> wrote:
> >
> > >
> > Thanks for providing the comments. I have fixed all the comments and
> > attached
> > the updated patch.
>
> Thanks for the patch. I have one comment for the fo
Thanks for providing the comments.
On Sat, 16 Nov 2024 at 17:29, vignesh C wrote:
>
> On Sat, 16 Nov 2024 at 00:10, Shlok Kyal wrote:
> >
> > Thanks for providing the comments. I have fixed all the comments and
> > attached the updated patch.
>
> Few comments:
> 1) The replident_has_valid_gen_co
On Sat, 16 Nov 2024 at 00:10, Shlok Kyal wrote:
>
> Thanks for providing the comments. I have fixed all the comments and
> attached the updated patch.
Few comments:
1) The replident_has_valid_gen_cols flag is set when either an update
or delete operation is published by the publication.
+
> > > On Friday, November 8, 2024 7:06 PM Shlok Kyal
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Amit,
> > > > > > > >
> > > > > > > >
gt; > > Hi Amit,
> > > > > > >
> > > > > > > On Thu, 7 Nov 2024 at 11:37, Amit Kapila
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Tue, Nov 5, 2024 at 12:53 PM Shlok Kyal
&g
, November 8, 2024 7:06 PM Shlok Kyal
> > > > > wrote:
> > > > > >
> > > > > > Hi Amit,
> > > > > >
> > > > > > On Thu, 7 Nov 2024 at 11:37, Amit Kapila
> > > > > > wrote:
> > >
gt; > >
> > > > > On Thu, 7 Nov 2024 at 11:37, Amit Kapila
> > > > > wrote:
> > > > > >
> > > > > > On Tue, Nov 5, 2024 at 12:53 PM Shlok Kyal
> > > > > >
> > > > > wrote:
> > > > > > &
gt;
> > > On Friday, November 8, 2024 7:06 PM Shlok Kyal
> > > wrote:
> > > >
> > > > Hi Amit,
> > > >
> > > > On Thu, 7 Nov 2024 at 11:37, Amit Kapila
> > > > wrote:
> > > > >
> > > >
gt; > >
> > > On Thu, 7 Nov 2024 at 11:37, Amit Kapila wrote:
> > > >
> > > > On Tue, Nov 5, 2024 at 12:53 PM Shlok Kyal
> > > wrote:
> > > > >
> > > > > To avoid the issue, we can disallow UP
On Tue, Nov 12, 2024 at 7:05 PM Alvaro Herrera wrote:
>
> On 2024-Nov-12, Amit Kapila wrote:
>
> > I think we still need a fix for the master for the case when generated
> > columns are not published but are part of REPLICA IDENTITY as that
> > could lead to failures in applying UPDATE and DELETE
Tue, Nov 5, 2024 at 12:53 PM Shlok Kyal
> > wrote:
> > > >
> > > > To avoid the issue, we can disallow UPDATE/DELETE on table with
> > > > unpublished generated column as REPLICA IDENTITY. I have attached a
> > > > patch for the same.
> &
On 2024-Nov-12, Amit Kapila wrote:
> I think we still need a fix for the master for the case when generated
> columns are not published but are part of REPLICA IDENTITY as that
> could lead to failures in applying UPDATE and DELETE on subscriber.
Ah, I thought that was already in place.
> Am, I
On 2024-Nov-12, Amit Kapila wrote:
> On Tue, Nov 12, 2024 at 2:15 PM Alvaro Herrera
> wrote:
> > So, another option is to do nothing for stable branches.
>
> Fair enough. The other point in favor of that option is that nobody
> has reported this problem yet but my guess is that they would have
On Tue, Nov 12, 2024 at 5:37 PM Alvaro Herrera wrote:
>
> On 2024-Nov-12, Amit Kapila wrote:
>
>
> > > It's not clear to me why doesn't pgoutput cope with generated columns in
> > > replica identities. Maybe that can be reconsidered?
> >
> > In stable branches, we intentionally skip publishing ge
On 2024-Nov-09, Amit Kapila wrote:
> On Fri, Nov 8, 2024 at 5:17 PM Alvaro Herrera wrote:
> >
> > On 2024-Nov-07, Amit Kapila wrote:
> >
> > > BTW, I was thinking as to how to fix it on back branches and it seems
> > > we should restrict to define REPLICA IDENTITY on stored generated
> > > column
On Tue, Nov 12, 2024 at 2:15 PM Alvaro Herrera wrote:
>
> On 2024-Nov-09, Amit Kapila wrote:
>
> > On Fri, Nov 8, 2024 at 5:17 PM Alvaro Herrera
> > wrote:
> > >
> > > On 2024-Nov-07, Amit Kapila wrote:
> > >
> > > > BTW, I was thinking as to how to fix it on back branches and it seems
> > > > w
On Friday, November 8, 2024 7:06 PM Shlok Kyal wrote:
>
> Hi Amit,
>
> On Thu, 7 Nov 2024 at 11:37, Amit Kapila wrote:
> >
> > On Tue, Nov 5, 2024 at 12:53 PM Shlok Kyal
> wrote:
> > >
> > > To avoid the issue, we can disallow UPDATE/DELETE on tab
On Sat, Nov 9, 2024 at 8:46 AM Amit Kapila wrote:
>
> On Fri, Nov 8, 2024 at 5:17 PM Alvaro Herrera wrote:
> >
> > On 2024-Nov-07, Amit Kapila wrote:
> >
> > > BTW, I was thinking as to how to fix it on back branches and it seems
> > > we should restrict to define REPLICA IDENTITY on stored gener
On Fri, Nov 8, 2024 at 5:17 PM Alvaro Herrera wrote:
>
> On 2024-Nov-07, Amit Kapila wrote:
>
> > BTW, I was thinking as to how to fix it on back branches and it seems
> > we should restrict to define REPLICA IDENTITY on stored generated
> > columns in the first place in back branches as those can
On 2024-Nov-07, Amit Kapila wrote:
> BTW, I was thinking as to how to fix it on back branches and it seems
> we should restrict to define REPLICA IDENTITY on stored generated
> columns in the first place in back branches as those can't be
> replicated. So, the following should fail:
>
> CREATE TA
Hi Amit,
On Thu, 7 Nov 2024 at 11:37, Amit Kapila wrote:
>
> On Tue, Nov 5, 2024 at 12:53 PM Shlok Kyal wrote:
> >
> > To avoid the issue, we can disallow UPDATE/DELETE on table with
> > unpublished generated column as REPLICA IDENTITY. I have attached a
> > patch
On Thu, Nov 7, 2024 at 11:04 AM Amit Kapila wrote:
>
> On Wed, Nov 6, 2024 at 5:48 PM Aleksander Alekseev
> wrote:
>
> We should fix this in the HEAD and back branches.
>
BTW, I was thinking as to how to fix it on back branches and it seems
we should restrict to define REPLICA IDENTITY on stored
On Tue, Nov 5, 2024 at 12:53 PM Shlok Kyal wrote:
>
> To avoid the issue, we can disallow UPDATE/DELETE on table with
> unpublished generated column as REPLICA IDENTITY. I have attached a
> patch for the same.
>
+CREATE PUBLICATION pub_gencol FOR TABLE testpub_gencol;
+UPDATE tes
On Wed, Nov 6, 2024 at 5:48 PM Aleksander Alekseev
wrote:
>
> > So, I think this behavior would be acceptable. Thoughts?
>
> That's a fair point, thanks for sharing. Personally I find this
> behavior somewhat suboptimal but since we already have it in certain
> cases I guess what you propose might
So, the update message generated by the last UPDATE would have NULL
> > for column 'b'.
> >
> > To avoid the issue, we can disallow UPDATE/DELETE on table with
> > unpublished generated column as REPLICA IDENTITY. I have attached a
> > patch for the same.
>
Hi Shlok,
> So, I think this behavior would be acceptable. Thoughts?
That's a fair point, thanks for sharing. Personally I find this
behavior somewhat suboptimal but since we already have it in certain
cases I guess what you propose might be acceptable.
I'm still not entirely happy about breakin
ated by the last UPDATE would have NULL
> for column 'b'.
>
> To avoid the issue, we can disallow UPDATE/DELETE on table with
> unpublished generated column as REPLICA IDENTITY. I have attached a
> patch for the same.
I don't think this would be a correct fix. Let's
ol' we do not
specify any column list, so column 'b' will not be published.
So, the update message generated by the last UPDATE would have NULL
for column 'b'.
To avoid the issue, we can disallow UPDATE/DELETE on table with
unpublished generated column as REPLICA IDENTITY.
55 matches
Mail list logo
