Hi,
On 2022-02-14 14:54:41 +0530, Amit Kapila wrote:
> On Fri, Feb 11, 2022 at 4:27 PM Amit Kapila wrote:
> >
> > On Fri, Feb 11, 2022 at 12:00 PM tanghy.f...@fujitsu.com
> > wrote:
> > >
> > > Attached the patches which fixed the above two comments and the first
> > > comment in
> > > my previ
On Mon, Feb 14, 2022 at 2:54 PM Amit Kapila wrote:
>
> On Fri, Feb 11, 2022 at 4:27 PM Amit Kapila wrote:
> >
> > On Fri, Feb 11, 2022 at 12:00 PM tanghy.f...@fujitsu.com
> > wrote:
> > >
> > > Attached the patches which fixed the above two comments and the first
> > > comment in
> > > my previ
On Fri, Feb 11, 2022 at 4:27 PM Amit Kapila wrote:
>
> On Fri, Feb 11, 2022 at 12:00 PM tanghy.f...@fujitsu.com
> wrote:
> >
> > Attached the patches which fixed the above two comments and the first
> > comment in
> > my previous mail [1], the rest is the same as before.
> > I ran the tests on a
On Fri, Feb 11, 2022 at 12:00 PM tanghy.f...@fujitsu.com
wrote:
>
> Attached the patches which fixed the above two comments and the first comment
> in
> my previous mail [1], the rest is the same as before.
> I ran the tests on all branches, they all passed as expected.
>
Thanks, these look good
On Thu, Feb 10, 2022 9:34 PM Amit Kapila wrote:
>
> On Mon, Feb 7, 2022 at 1:27 PM Dilip Kumar wrote:
> >
> > On Mon, Feb 7, 2022 at 12:25 PM Amit Kapila
> wrote:
> > >
> > > Attached please find the modified patches.
> >
> > I have looked into the latest modification and back branch patches an
On Mon, Feb 7, 2022 at 1:27 PM Dilip Kumar wrote:
>
> On Mon, Feb 7, 2022 at 12:25 PM Amit Kapila wrote:
> >
> > Attached please find the modified patches.
>
> I have looked into the latest modification and back branch patches and
> they look fine to me.
>
Today, while looking at this patch agai
On Wed, Feb 9, 2022 at 11:08 AM tanghy.f...@fujitsu.com
wrote:
>
> On Tue, Feb 8, 2022 3:18 AM Andres Freund wrote:
> >
> > On 2022-02-07 08:44:00 +0530, Amit Kapila wrote:
> > > Right, and it is getting changed. We are just printing the first 200
> > > characters (by using SQL [1]) from the deco
On Tue, Feb 8, 2022 3:18 AM Andres Freund wrote:
>
> On 2022-02-07 08:44:00 +0530, Amit Kapila wrote:
> > Right, and it is getting changed. We are just printing the first 200
> > characters (by using SQL [1]) from the decoded tuple so what is shown
> > in the results is the initial 200 bytes.
>
On Wed, Feb 9, 2022 at 7:16 AM Euler Taveira wrote:
>
> On Tue, Feb 8, 2022, at 10:18 PM, tanghy.f...@fujitsu.com wrote:
>
> 2)
> + /*
> + * Check if the old tuple's attribute is stored externally and is a
> + * member of external_cols.
> + */
> + if (VARATT_IS_EXTERNAL((struct varlena *) DatumGet
On Tue, Feb 8, 2022, at 10:18 PM, tanghy.f...@fujitsu.com wrote:
> 2)
> + /*
> + * Check if the old tuple's attribute is stored externally and is a
> + * member of external_cols.
> + */
> + if (VARATT_IS_EXTERNAL((struct varlena *) DatumGetPointer(value1)) &&
> + bms_is_member(attrnum - FirstLowInv
On Mon, Feb 7, 2022 2:55 PM Amit Kapila wrote:
>
> On Sat, Feb 5, 2022 at 6:10 AM Amit Kapila wrote:
> >
> > On Fri, Feb 4, 2022 at 9:06 PM Alvaro Herrera
> > wrote:
> > >
> > >
> > > I have some suggestions
> > > on the comments and docs though.
> > >
> >
> > Thanks, your suggestions look go
On Tue, Feb 8, 2022 at 12:48 AM Andres Freund wrote:
>
> On 2022-02-07 08:44:00 +0530, Amit Kapila wrote:
> > Right, and it is getting changed. We are just printing the first 200
> > characters (by using SQL [1]) from the decoded tuple so what is shown
> > in the results is the initial 200 bytes.
On Tue, Feb 8, 2022 at 12:48 AM Andres Freund wrote:
>
> Hi,
>
> On 2022-02-07 08:44:00 +0530, Amit Kapila wrote:
> > Right, and it is getting changed. We are just printing the first 200
> > characters (by using SQL [1]) from the decoded tuple so what is shown
> > in the results is the initial 200
Hi,
On 2022-02-07 08:44:00 +0530, Amit Kapila wrote:
> Right, and it is getting changed. We are just printing the first 200
> characters (by using SQL [1]) from the decoded tuple so what is shown
> in the results is the initial 200 bytes.
Ah, I knew I must have been missing something.
> The com
On Mon, Feb 7, 2022 at 12:25 PM Amit Kapila wrote:
>
> On Sat, Feb 5, 2022 at 6:10 AM Amit Kapila wrote:
> >
> > On Fri, Feb 4, 2022 at 9:06 PM Alvaro Herrera
> > wrote:
> > >
> > >
> > > I have some suggestions
> > > on the comments and docs though.
> > >
> >
> > Thanks, your suggestions look
On Sat, Feb 5, 2022 at 6:10 AM Amit Kapila wrote:
>
> On Fri, Feb 4, 2022 at 9:06 PM Alvaro Herrera wrote:
> >
> >
> > I have some suggestions
> > on the comments and docs though.
> >
>
> Thanks, your suggestions look good to me. I'll take care of these in
> the next version.
>
Attached please
On Sun, Feb 6, 2022 at 5:04 AM Andres Freund wrote:
>
> On 2022-02-04 17:45:36 +0530, Amit Kapila wrote:
> > diff --git a/contrib/test_decoding/expected/toast.out
> > b/contrib/test_decoding/expected/toast.out
> > index cd03e9d..a757e7d 100644
> > --- a/contrib/test_decoding/expected/toast.out
>
Hi,
On 2022-02-04 17:45:36 +0530, Amit Kapila wrote:
> diff --git a/contrib/test_decoding/expected/toast.out
> b/contrib/test_decoding/expected/toast.out
> index cd03e9d..a757e7d 100644
> --- a/contrib/test_decoding/expected/toast.out
> +++ b/contrib/test_decoding/expected/toast.out
> @@ -77,7 +7
On 2022-Feb-05, Amit Kapila wrote:
> On Fri, Feb 4, 2022 at 9:06 PM Alvaro Herrera wrote:
> >
> > I don't have a reason not to commit this patch.
> >
>
> It is not very clear to me from this so just checking again, are you
> fine with back-patching this as well?
Hmm, of course, I never thought
On Fri, Feb 4, 2022 at 9:06 PM Alvaro Herrera wrote:
>
> I don't have a reason not to commit this patch.
>
It is not very clear to me from this so just checking again, are you
fine with back-patching this as well?
>
> I have some suggestions
> on the comments and docs though.
>
Thanks, your su
I don't have a reason not to commit this patch. I have some suggestions
on the comments and docs though.
> @@ -8359,14 +8408,15 @@ log_heap_new_cid(Relation relation, HeapTuple tup)
> * Returns NULL if there's no need to log an identity or if there's no
> suitable
> * key defined.
> *
> -
On Mon, Jan 31, 2022 at 9:03 AM Dilip Kumar wrote:
>
> >
> > I have changed this and various other comments in the patch. I have
> > modified the docs as well to reflect the changes. I thought of adding
> > a test but I think the current test in toast.sql seems sufficient.
> > Kindly let me know w
On Sat, Jan 29, 2022 at 3:57 PM Amit Kapila wrote:
>
> On Fri, Jan 28, 2022 at 12:16 PM Dilip Kumar wrote:
> >
> + /*
> + * If it's a whole-tuple reference, say "not equal". It's not really
> + * worth supporting this case, since it could only succeed after a
> + * no-op update, which is hardly
On Fri, Jan 28, 2022 at 12:16 PM Dilip Kumar wrote:
>
> I think the best way is to do some refactoring and renaming of the
> function, because as part of HeapDetermineModifiedColumns we are
> already processing the tuple so we can not put extra overhead of
> reprocessing it again. In short I like
On Tue, Jan 25, 2022 at 11:59 AM Amit Kapila wrote:
>
> On Tue, Jan 25, 2022 at 12:26 AM Euler Taveira wrote:
> >
>
> I am not sure if your proposal is much different compared to v4 or how
> much it improves the situation? I see you didn't consider
> 'check_external_attr' parameter and I think th
On Tue, Jan 25, 2022 at 12:26 AM Euler Taveira wrote:
>
> On Mon, Jan 24, 2022, at 12:58 AM, Michael Paquier wrote:
>
> FWIW, I find the API changes of HeapDetermineModifiedColumns() and
> ExtractReplicaIdentity() a bit grotty. Shouldn't we try to flatten
> the old tuple instead? There are thing
On Mon, Jan 24, 2022 at 4:42 PM Andres Freund wrote:
> On 2022-01-24 16:31:08 -0500, Robert Haas wrote:
> > That seems consistent with what's been described on this thread so
> > far, but I still don't quite understand why the logic that reassembles
> > TOAST chunks doesn't solve it.
>
> There are
On 2022-01-24 16:31:08 -0500, Robert Haas wrote:
> That seems consistent with what's been described on this thread so
> far, but I still don't quite understand why the logic that reassembles
> TOAST chunks doesn't solve it.
There are no toast chunks to reassemble if the update didn't change the
pr
On Mon, Jan 24, 2022 at 4:17 PM Andres Freund wrote:
> Possibly the root of the problem is that we/I didn't think of cases where the
> primary key is an external toast datum - in moast scenarios you'd an error
> about a too wide index tuple. But of course that neglects cases where toasting
> happe
Hi,
On 2022-01-24 15:10:05 -0500, Robert Haas wrote:
> I think we realized when we were working on the logical decoding stuff
> that the key columns of the old tuple would have to be detoasted in
> order for the mechanism to work, because I remember worrying about
> whether it would potentially be
On Tue, Aug 10, 2021 at 1:20 AM Amit Kapila wrote:
> It seems to me this problem exists from the time we introduced
> wal_level = logical in the commit e55704d8b2 [1], or another
> possibility is that logical replication commit didn't consider
> something to make it work. Andres, Robert, Petr, can
On Mon, Jan 24, 2022, at 12:58 AM, Michael Paquier wrote:
> FWIW, I find the API changes of HeapDetermineModifiedColumns() and
> ExtractReplicaIdentity() a bit grotty. Shouldn't we try to flatten
> the old tuple instead? There are things like
> toast_flatten_tuple_to_datum() for this purpose if a
On Mon, Jan 24, 2022 at 9:28 AM Michael Paquier wrote:
>
> On Wed, Aug 11, 2021 at 06:14:55PM +0530, Dilip Kumar wrote:
> > Right
>
> Amit, are you planning to look more at this patch? It has been a
> couple of months since the last update, and this is still a bug as far
> as I understand.
>
> FW
On Wed, Aug 11, 2021 at 06:14:55PM +0530, Dilip Kumar wrote:
> Right
Amit, are you planning to look more at this patch? It has been a
couple of months since the last update, and this is still a bug as far
as I understand.
FWIW, I find the API changes of HeapDetermineModifiedColumns() and
Extract
On Wed, Sep 8, 2021 at 11:26 AM Ajin Cherian wrote:
> On Wed, Aug 11, 2021 at 10:45 PM Dilip Kumar
> wrote:
>
> > Yeah we can avoid that by detecting any toasted replica identity key
> > in HeapDetermineModifiedColumns, check the attached patch.
> >
>
> I had a second look at this, and I just ha
On Wed, Aug 11, 2021 at 10:45 PM Dilip Kumar wrote:
> Yeah we can avoid that by detecting any toasted replica identity key
> in HeapDetermineModifiedColumns, check the attached patch.
>
I had a second look at this, and I just had a small doubt. Since the
convention is that for UPDATES, the old t
On Wed, Aug 11, 2021 at 10:45 PM Dilip Kumar wrote:
>
> Yeah we can avoid that by detecting any toasted replica identity key
> in HeapDetermineModifiedColumns, check the attached patch.
>
The patch applies cleanly, all tests pass, I tried out a few toast
combination tests and they seem to be work
On Wed, Aug 11, 2021 at 10:30 AM Amit Kapila wrote:
>
> On Tue, Aug 10, 2021 at 8:08 PM Alvaro Herrera
> wrote:
> >
> > On 2021-Jul-30, Amit Kapila wrote:
> >
> > Reading Dilip's last posted patch that day, I had some reservations
> > about the API of ExtractReplicaIdentity. The new argument ma
On Tue, Aug 10, 2021 at 8:08 PM Alvaro Herrera wrote:
>
> On 2021-Jul-30, Amit Kapila wrote:
>
> Reading Dilip's last posted patch that day, I had some reservations
> about the API of ExtractReplicaIdentity. The new argument makes for a
> very strange to explain behavior "return the key values if
On 2021-Jul-30, Amit Kapila wrote:
> I was thinking of using toast pointer but that won't work because it
> can be different on the subscriber-side. I don't see any better ideas
> to fix this issue. This problem seems to be from the time Logical
> Replication has been introduced, so adding others
On Fri, Jul 30, 2021 at 10:21 AM Amit Kapila wrote:
>
> This problem seems to be from the time Logical
> Replication has been introduced, so adding others (who are generally
> involved in this area) to see what they think about this bug? I think
> people might not be using toasted columns for Repl
On Mon, Jul 26, 2021 at 10:45 AM Dilip Kumar wrote:
>
> I was thinking more about this idea, but IMHO, unless we send the key
> toasted tuple from the publisher how is the subscriber supposed to
> fetch it. Because that is the key value for finding the tuple on the
> subscriber side and if we hav
On Fri, Jul 23, 2021 at 8:58 AM Amit Kapila wrote:
>
> Okay, thanks. I think one point we need to consider here is that on
> the subscriber side, we use dirtysnapshot to search the key, so we
> need to ensure that we don't fetch the wrong data. I am not sure what
> will happen when by the time we
On Thu, Jul 22, 2021 at 8:02 PM Dilip Kumar wrote:
>
> On Thu, Jul 22, 2021 at 4:11 PM Amit Kapila wrote:
> >
> > On Thu, Jun 3, 2021 at 5:15 PM Dilip Kumar wrote:
> > >
> > > On Wed, Jun 2, 2021 at 7:23 PM Dilip Kumar wrote:
> > > >
> > > > On Wed, Jun 2, 2021 at 7:20 PM Kuntal Ghosh
> > > >
On Thu, Jul 22, 2021 at 4:11 PM Amit Kapila wrote:
>
> On Thu, Jun 3, 2021 at 5:15 PM Dilip Kumar wrote:
> >
> > On Wed, Jun 2, 2021 at 7:23 PM Dilip Kumar wrote:
> > >
> > > On Wed, Jun 2, 2021 at 7:20 PM Kuntal Ghosh
> > > wrote:
> > > >
> > > > On Wed, Jun 2, 2021 at 3:10 PM Dilip Kumar
>
On Thu, Jun 3, 2021 at 5:15 PM Dilip Kumar wrote:
>
> On Wed, Jun 2, 2021 at 7:23 PM Dilip Kumar wrote:
> >
> > On Wed, Jun 2, 2021 at 7:20 PM Kuntal Ghosh
> > wrote:
> > >
> > > On Wed, Jun 2, 2021 at 3:10 PM Dilip Kumar wrote:
> > > >
> > > > Yes, you are right. I will change it in the next
On Fri, Jun 4, 2021 at 8:25 AM tanghy.f...@fujitsu.com
wrote:
>
> On Thu, Jun 3, 2021 7:45 PMDilip Kumar wrote:
> >
> > I have fixed all the pending issues, I see there is already a test
> > case for this so I have changed the output for that.
>
> Thanks for your patch. I tested it for all branch
On Thu, Jun 3, 2021 7:45 PMDilip Kumar wrote:
>
> I have fixed all the pending issues, I see there is already a test
> case for this so I have changed the output for that.
Thanks for your patch. I tested it for all branches(10,11,12,13,HEAD) and all
of them passed.(This bug was introduced in PG
On Wed, Jun 2, 2021 at 7:23 PM Dilip Kumar wrote:
>
> On Wed, Jun 2, 2021 at 7:20 PM Kuntal Ghosh
> wrote:
> >
> > On Wed, Jun 2, 2021 at 3:10 PM Dilip Kumar wrote:
> > >
> > > Yes, you are right. I will change it in the next version, along with
> > > the test case.
> > >
> > +/*
> > +
On Wed, Jun 2, 2021 at 7:20 PM Kuntal Ghosh wrote:
>
> On Wed, Jun 2, 2021 at 3:10 PM Dilip Kumar wrote:
> >
> > Yes, you are right. I will change it in the next version, along with
> > the test case.
> >
> +/*
> + * if the key hasn't changed and we're only logging the key, we're done.
>
On Wed, Jun 2, 2021 at 3:10 PM Dilip Kumar wrote:
>
> Yes, you are right. I will change it in the next version, along with
> the test case.
>
+/*
+ * if the key hasn't changed and we're only logging the key, we're done.
+ * But if tuple has external data then we might have to detoast
On Wed, Jun 2, 2021 at 2:37 PM tanghy.f...@fujitsu.com
wrote:
>
> On Wed, Jun 2, 2021 2:44 PM Dilip Kumar wrote:
> > Attached patch fixes that, I haven't yet added the test case. Once
> > someone confirms on the approach then I will add a test case to the
> > patch.
>
> key_tuple = heap_
On Wed, Jun 2, 2021 2:44 PM Dilip Kumar wrote:
> Attached patch fixes that, I haven't yet added the test case. Once
> someone confirms on the approach then I will add a test case to the
> patch.
key_tuple = heap_form_tuple(desc, values, nulls);
*copy = true;
...
On Tue, Jun 1, 2021 at 3:39 PM Dilip Kumar wrote:
>
> On Tue, Jun 1, 2021 at 12:29 PM tanghy.f...@fujitsu.com
> > I noticed that in case1, ExtractReplicaIdentity function returned NULL on
> > HEAD. But after your fix, it didn’t return NULL. There is no problem with
> > this case on HEAD, but th
On Tue, Jun 1, 2021 at 12:29 PM tanghy.f...@fujitsu.com
wrote:
>
> Hi
>
>
>
> I have some questions with your patch. Here are two cases I used to check the
> bug.
>
>
>
> Case1(PK toasted_key is short), data could be synchronized on HEAD.
>
> ---
>
> INSERT INTO toasted_key(toasted_ke
Hi
I have some questions with your patch. Here are two cases I used to check the
bug.
Case1(PK toasted_key is short), data could be synchronized on HEAD.
---
INSERT INTO toasted_key(toasted_key, toasted_col1) VALUES('111',
repeat('9876543210', 200));
UPDATE toasted_key SET toasted_c
On Mon, 31 May 2021 at 3:33 PM, tanghy.f...@fujitsu.com <
tanghy.f...@fujitsu.com> wrote:
> On Mon, May 31, 2021 5:12 PM Dilip Kumar wrote:
> >
> > The problem is if the key attribute is not changed we don't log it as
> > it should get logged along with the updated tuple, but if it is
> > externa
On Mon, May 31, 2021 5:12 PM Dilip Kumar wrote:
>
> The problem is if the key attribute is not changed we don't log it as
> it should get logged along with the updated tuple, but if it is
> externally stored then the complete key will never be logged because
> there is no log from the toast table
On Mon, May 31, 2021 at 12:20 PM Dilip Kumar wrote:
>
> On Mon, May 31, 2021 at 8:04 AM tanghy.f...@fujitsu.com
> wrote:
> >
> > On Friday, May 28, 2021 6:51 PM, Dilip Kumar wrote:
> > > Seems you did not set the replica identity for updating the tuple.
> > > Try this before updating, and it sho
On Mon, May 31, 2021 at 8:04 AM tanghy.f...@fujitsu.com
wrote:
>
> On Friday, May 28, 2021 6:51 PM, Dilip Kumar wrote:
> > Seems you did not set the replica identity for updating the tuple.
> > Try this before updating, and it should work.
>
> Thanks for your reply. I tried it.
>
> > ALTER TABLE
On Friday, May 28, 2021 6:51 PM, Dilip Kumar wrote:
> Seems you did not set the replica identity for updating the tuple.
> Try this before updating, and it should work.
Thanks for your reply. I tried it.
> ALTER TABLE toasted_key REPLICA IDENTITY USING INDEX toasted_key_pkey;
This didn't work.
On Fri, May 28, 2021 at 12:31 PM tanghy.f...@fujitsu.com
wrote:
>
> On Friday, May 28, 2021 3:02 PM, tanghy.f...@fujitsu.com wrote:
> > FYI. The problem also occurs in PG-13. I will try to check from which
> > version it
> > got introduced.
>
> I reproduced it in PG-10,11,12,13.
> I think the pro
On Friday, May 28, 2021 3:02 PM, tanghy.f...@fujitsu.com wrote:
> FYI. The problem also occurs in PG-13. I will try to check from which version
> it
> got introduced.
I reproduced it in PG-10,11,12,13.
I think the problem has been existing since Logical replication introduced in
PG-10.
Regards
On Friday, May 28, 2021 2:16 PM, tanghy.f...@fujitsu.com wrote:
>I think I just found a bug in logical replication. Data couldn't be
>synchronized while updating toast data.
>Could anyone take a look at it?
FYI. The problem also occurs in PG-13. I will try to check from which version
it got in
64 matches
Mail list logo