Hi,
Amit Kapila , 23 Mar 2023 Per, 08:48 tarihinde
şunu yazdı:
> Pushed the 0001. It may be better to start a separate thread for 0002.
>
Great! Thanks.
Best,
--
Melih Mutlu
Microsoft
Dear Amit,
> Pushed the 0001. It may be better to start a separate thread for 0002.
Good job! I have started new thread [1] for 0002.
[1]:
https://www.postgresql.org/message-id/TYAPR01MB58667AE04D291924671E2051F5879%40TYAPR01MB5866.jpnprd01.prod.outlook.com
Best Regards,
Hayato Kuroda
FUJITSU
On Wed, Mar 22, 2023 at 4:06 PM Hayato Kuroda (Fujitsu)
wrote:
>
> > The patch looks mostly good to me. However, I have one
> > question/comment as follows:
> >
> > -
> > +
> > binary (boolean)
> >
> >
> > To allow references to the binary option, we add the
Dear Amit, hackers,
> The patch looks mostly good to me. However, I have one
> question/comment as follows:
>
> -
> +
> binary (boolean)
>
>
> To allow references to the binary option, we add the varlistentry id
> here. It looks slightly odd to me to add id for
On Wed, Mar 22, 2023 at 9:00 AM shiy.f...@fujitsu.com
wrote:
>
> On Wed Mar 22, 2023 7:29 AM Peter Smith wrote:
> >
> > Thanks for all the patch updates. Patch v19 LGTM.
> >
>
> +1
>
The patch looks mostly good to me. However, I have one
question/comment as follows:
-
+
On Wed Mar 22, 2023 7:29 AM Peter Smith wrote:
>
> Thanks for all the patch updates. Patch v19 LGTM.
>
+1
Regards,
Shi Yu
Thanks for all the patch updates. Patch v19 LGTM.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
Hi,
Peter Smith , 21 Mar 2023 Sal, 04:33 tarihinde şunu
yazdı:
> Here are my review comments for v18-0001
>
> ==
> doc/src/sgml/logical-replication.sgml
>
> 1.
> + target table. However, logical replication in binary format is more
> + restrictive. See the binary option of
> + CREATE
>
On Tue, Mar 21, 2023 at 2:16 PM Melih Mutlu wrote:
>
> Amit Kapila , 21 Mar 2023 Sal, 09:03 tarihinde şunu
> yazdı:
>>
>> On Tue, Mar 21, 2023 at 7:03 AM Peter Smith wrote:
>> >
>> >
>> > ==
>> > src/test/subscription/t/014_binary.pl
>> >
>> > 4.
>> > #
Amit Kapila , 21 Mar 2023 Sal, 09:03 tarihinde
şunu yazdı:
> On Tue, Mar 21, 2023 at 7:03 AM Peter Smith wrote:
> >
> >
> > ==
> > src/test/subscription/t/014_binary.pl
> >
> > 4.
> > # -
> > # Test mismatched column types with/without
On Tue, Mar 21, 2023 at 7:03 AM Peter Smith wrote:
>
>
> ==
> src/test/subscription/t/014_binary.pl
>
> 4.
> # -
> # Test mismatched column types with/without binary mode
> # -
>
> # Test
Here are my review comments for v18-0001
==
doc/src/sgml/logical-replication.sgml
1.
+ target table. However, logical replication in binary format is more
+ restrictive. See the binary option of
+ CREATE
SUBSCRIPTION
+ for details.
Because you've changed the linkend to be the
Hi,
Please see the attached patch.
vignesh C , 18 Mar 2023 Cmt, 12:03 tarihinde şunu
yazdı:
> On Fri, 17 Mar 2023 at 17:55, Melih Mutlu wrote:
> 1) Currently we refer the link to the beginning of create subscription
> page, this can be changed to refer to binary option contents in create
>
Dear Melih,
Thank you for updating the patch.
I checked your added description about initial data sync and I think it's OK.
Few minor comments:
01. copy_table
```
+ List *options = NIL;
```
I found a unnecessary blank just after "List". You can remove it and align
definition.
On Mon, Mar 20, 2023 at 3:37 AM Peter Smith wrote:
>
> There are a couple of TAP tests where the copy binary is expected to
> fail. And when it fails, you do binary=false (changing the format back
> to 'text') so the test is then expected to be able to proceed.
>
> I don't know if this happens in
There are a couple of TAP tests where the copy binary is expected to
fail. And when it fails, you do binary=false (changing the format back
to 'text') so the test is then expected to be able to proceed.
I don't know if this happens in practice, but IIUC in theory, if the
timing is extremely bad,
On Sat, Mar 18, 2023 at 3:11 PM Peter Smith wrote:
>
> On Fri, Mar 17, 2023 at 11:25 PM Melih Mutlu wrote:
> ==
> src/backend/replication/logical/tablesync.c
>
> 2.
> @@ -1168,6 +1170,15 @@ copy_table(Relation rel)
>
> appendStringInfoString(, ") TO STDOUT");
> }
> +
> + /* The binary
On Fri, Mar 17, 2023 at 11:25 PM Melih Mutlu wrote:
>
> Hi,
>
> Sharing v17.
>
> Amit Kapila , 17 Mar 2023 Cum, 03:02 tarihinde şunu
> yazdı:
>>
>> I think to reduce the risk of breakage, let's change the check to
>> >=v16. Also, accordingly, update the doc and commit message.
>
>
> Done.
>
On Fri, 17 Mar 2023 at 17:55, Melih Mutlu wrote:
>
> Hi,
>
> Sharing v17.
>
> Amit Kapila , 17 Mar 2023 Cum, 03:02 tarihinde şunu
> yazdı:
>>
>> I think to reduce the risk of breakage, let's change the check to
>> >=v16. Also, accordingly, update the doc and commit message.
>
>
> Done.
>
> Peter
Hi,
Sharing v17.
Amit Kapila , 17 Mar 2023 Cum, 03:02 tarihinde
şunu yazdı:
> I think to reduce the risk of breakage, let's change the check to
> >=v16. Also, accordingly, update the doc and commit message.
>
Done.
Peter Smith , 17 Mar 2023 Cum, 04:58 tarihinde şunu
yazdı:
> IMO the sentence
On Fri, Mar 17, 2023 at 6:42 AM Peter Smith wrote:
>
> On Thu, Mar 16, 2023 at 1:55 PM Amit Kapila wrote:
> >
> > On Wed, Mar 15, 2023 at 3:33 PM Melih Mutlu wrote:
> > >
> > > Amit Kapila , 15 Mar 2023 Çar, 12:31 tarihinde
> > > şunu yazdı:
> > >>
> > >> On Tue, Mar 14, 2023 at 4:32 PM Melih
On Thu, Mar 16, 2023 9:20 PM Melih Mutlu wrote:
>
> Hi,
>
> Please see the attached v16.
>
Thanks for updating the patch.
+# Cannot sync due to type mismatch
+$node_subscriber->wait_for_log(qr/ERROR: ( [A-Z0-9]+:)? incorrect binary data
format/);
+# Ensure the COPY command is executed in
On Fri, Mar 17, 2023 at 12:20 AM Melih Mutlu wrote:
>
> Hi,
>
> Please see the attached v16.
>
> Peter Smith , 16 Mar 2023 Per, 03:03 tarihinde şunu
> yazdı:
>>
>> Here are some review comments for v15-0001
>
>
> I applied your comments in the updated patch.
Thanks.
I checked patchv16-0001 and
On Thu, Mar 16, 2023 at 1:55 PM Amit Kapila wrote:
>
> On Wed, Mar 15, 2023 at 3:33 PM Melih Mutlu wrote:
> >
> > Amit Kapila , 15 Mar 2023 Çar, 12:31 tarihinde
> > şunu yazdı:
> >>
> >> On Tue, Mar 14, 2023 at 4:32 PM Melih Mutlu wrote:
> >
> >
> >>
> >> What purpose does this test serve
On Thu, Mar 16, 2023 at 6:59 PM Melih Mutlu wrote:
>
> Amit Kapila , 16 Mar 2023 Per, 06:25 tarihinde şunu
> yazdı:
>>
>> On Thu, Mar 16, 2023 at 8:27 AM Euler Taveira wrote:
>> >
>> > On Wed, Mar 8, 2023, at 11:50 PM, Amit Kapila wrote:
>> >
>> > It is not clear to me which version check you
Amit Kapila , 16 Mar 2023 Per, 06:25 tarihinde
şunu yazdı:
> On Thu, Mar 16, 2023 at 8:27 AM Euler Taveira wrote:
> >
> > On Wed, Mar 8, 2023, at 11:50 PM, Amit Kapila wrote:
> >
> > It is not clear to me which version check you wanted to add because we
> > seem to have a binary option in COPY
Hi,
Please see the attached v16.
Peter Smith , 16 Mar 2023 Per, 03:03 tarihinde şunu
yazdı:
> Here are some review comments for v15-0001
>
I applied your comments in the updated patch.
shiy.f...@fujitsu.com , 16 Mar 2023 Per, 05:35
tarihinde şunu yazdı:
> On Thu, Mar 16, 2023 2:26 AM Melih
On Thu, Mar 16, 2023 at 8:27 AM Euler Taveira wrote:
>
> On Wed, Mar 8, 2023, at 11:50 PM, Amit Kapila wrote:
>
> It is not clear to me which version check you wanted to add because we
> seem to have a binary option in COPY from the time prior to logical
> replication. I feel we need a publisher
On Wed, Mar 8, 2023, at 11:50 PM, Amit Kapila wrote:
> On Wed, Mar 8, 2023 at 6:17 PM Melih Mutlu wrote:
> >
> > On 7 Mar 2023 Tue at 04:10 Amit Kapila wrote:
> >>
> >> As per what I could read in this thread, most people prefer to use the
> >> existing binary option rather than inventing a new
On Wed, Mar 15, 2023 at 3:33 PM Melih Mutlu wrote:
>
> Amit Kapila , 15 Mar 2023 Çar, 12:31 tarihinde şunu
> yazdı:
>>
>> On Tue, Mar 14, 2023 at 4:32 PM Melih Mutlu wrote:
>
>
>>
>> What purpose does this test serve w.r.t this patch? Before checking
>> the sync for different column orders, the
Hi,
Thanks for updating the patch, I think it is a useful feature.
I looked at the v15 patch and the patch looks mostly good to me.
Here are few comments:
1.
+ {
+ appendStringInfo(, " WITH (FORMAT binary)");
We could use appendStringInfoString here.
2.
+# It should fail
On Thu, Mar 16, 2023 at 5:59 AM Peter Smith wrote:
>
> On Wed, Mar 15, 2023 at 5:31 PM Amit Kapila wrote:
> >
> > On Wed, Mar 15, 2023 at 11:52 AM Peter Smith wrote:
> > >
> > > ==
> > > src/backend/replication/logical/tablesync.c
> > >
> > > 5.
> > > +
> > > + /*
> > > + * If the publisher
On Thu, Mar 16, 2023 2:26 AM Melih Mutlu wrote:
>
> Right, it needs to be ordered. Fixed.
>
Hi,
Thanks for updating the patch. I tested some cases like toast data, combination
of row filter and column lists, and it works well.
Here is a comment:
+# Ensure the COPY command is executed in
On Wed, Mar 15, 2023 at 5:31 PM Amit Kapila wrote:
>
> On Wed, Mar 15, 2023 at 11:52 AM Peter Smith wrote:
> >
> > ==
> > src/backend/replication/logical/tablesync.c
> >
> > 5.
> > +
> > + /*
> > + * If the publisher version is earlier than v14, it COPY command doesn't
> > + * support the
Here are some review comments for v15-0001
==
doc/src/sgml/logical-replication.sgml
1.
+ target table. However, logical replication in binary format is
more restrictive,
+ see binary option of
+ CREATE
SUBSCRIPTION
+ for more details.
IMO (and Chat-GPT agrees) the new text should
Hi,
vignesh C , 15 Mar 2023 Çar, 18:12 tarihinde şunu
yazdı:
> One comment:
> 1) There might be a chance the result order of select may vary as
> "ORDER BY" is not specified, Should we add "ORDER BY" as the table
> has multiple rows:
> +# Check the synced data on the subscriber
> +$result =
On Wed, 15 Mar 2023 at 15:30, Melih Mutlu wrote:
>
> Hi,
>
> Please see the attached patch.
One comment:
1) There might be a chance the result order of select may vary as
"ORDER BY" is not specified, Should we add "ORDER BY" as the table
has multiple rows:
+# Check the synced data on the
Amit Kapila , 15 Mar 2023 Çar, 12:31 tarihinde
şunu yazdı:
> On Tue, Mar 14, 2023 at 4:32 PM Melih Mutlu
> wrote:
>
> What purpose does this test serve w.r.t this patch? Before checking
> the sync for different column orders, the patch has already changed
> binary to false, so it doesn't seem
Hi,
Please see v14 [1].
Peter Smith , 15 Mar 2023 Çar, 09:22 tarihinde şunu
yazdı:
> Here are some review comments for v13-0001
>
> ==
> doc/src/sgml/logical-replication.sgml
>
> 1.
> That's why in the previous v12 review [1] (comment #3) I suggested
> that this page should just say
Hi,
Please see the attached patch.
Takamichi Osumi (Fujitsu) , 14 Mar 2023 Sal,
18:20 tarihinde şunu yazdı:
> (1) create_subscription.sgml
>
> + column types as opposed to text format. Even when this option
> is enabled,
> + only data types having binary send and receive
On Tue, Mar 14, 2023 at 4:32 PM Melih Mutlu wrote:
>
> Attached v13.
>
I have a question related to the below test in the patch:
+# Setting binary to false should allow syncing
+$node_subscriber->safe_psql(
+'postgres', qq(
+ALTER SUBSCRIPTION tsub SET (binary = false);));
+
+# Ensure
Hi,
On Wednesday, March 15, 2023 2:34 PM Amit Kapila
wrote:
> On Tue, Mar 14, 2023 at 8:50 PM Takamichi Osumi (Fujitsu)
> wrote:
> >
> > On Tuesday, March 14, 2023 8:02 PM Melih Mutlu
> wrote:
> > (3) copy_table()
> >
> > + /*
> > +* If the publisher version is earlier than
On Wed, Mar 15, 2023 at 11:52 AM Peter Smith wrote:
>
> ==
> src/backend/replication/logical/tablesync.c
>
> 5.
> +
> + /*
> + * If the publisher version is earlier than v14, it COPY command doesn't
> + * support the binary option.
> + */
> + if (walrcv_server_version(LogRepWorkerWalRcvConn)
Here are some review comments for v13-0001
==
doc/src/sgml/logical-replication.sgml
1.
@@ -241,10 +241,13 @@
types of the columns do not need to match, as long as the text
representation of the data can be converted to the target type. For
example, you can replicate from a
On Tue, Mar 14, 2023 at 8:50 PM Takamichi Osumi (Fujitsu)
wrote:
>
> On Tuesday, March 14, 2023 8:02 PM Melih Mutlu wrote:
> (3) copy_table()
>
> + /*
> +* If the publisher version is earlier than v14, it COPY command
> doesn't
> +* support the binary option.
> +*/
On Tuesday, March 14, 2023 8:02 PM Melih Mutlu wrote:
> Attached v13.
Hi,
Thanks for sharing v13. Few minor review comments.
(1) create_subscription.sgml
+ column types as opposed to text format. Even when this option is
enabled,
+ only data types having binary send and
Hi,
Attached v13.
Peter Smith , 14 Mar 2023 Sal, 03:07 tarihinde şunu
yazdı:
> Here are some review comments for patch v12-0001
>
Thanks for reviewing. I tried to make explanations in docs better
according to your comments.
What do you think?
Amit Kapila , 14 Mar 2023 Sal, 06:17 tarihinde
On Tue, Mar 14, 2023 at 6:18 AM Peter Smith wrote:
>
> On Tue, Mar 14, 2023 at 11:06 AM Peter Smith wrote:
> >
> > Here are some review comments for patch v12-0001
> >
> > ==
> > General
> >
> > 1.
> > There is no new test code. Are we sure that there are already
> > sufficient TAP tests
Here are some review comments for patch v12-0001 (test code only)
==
src/test/subscription/t/014_binary.pl
# Check the synced data on subscribers
~
There are a couple of comments like the above that say: "on
subscribers" instead of "on subscriber".
~~~
I wondered if it might be useful to
On Tue, Mar 14, 2023 at 11:06 AM Peter Smith wrote:
>
> Here are some review comments for patch v12-0001
>
> ==
> General
>
> 1.
> There is no new test code. Are we sure that there are already
> sufficient TAP tests doing binary testing with/without copy_data and
> covering all the necessary
Here are some review comments for patch v12-0001
==
General
1.
There is no new test code. Are we sure that there are already
sufficient TAP tests doing binary testing with/without copy_data and
covering all the necessary combinations?
==
Commit Message
2.
Without this patch, table are
Hi,
Attached v12 with a unified option.
Setting binary = true now allows the initial sync to happen in binary
format.
Thanks,
--
Melih Mutlu
Microsoft
v12-0001-Allow-logical-replication-to-copy-table-in-binary.patch
Description: Binary data
On Wed, Mar 8, 2023 at 6:17 PM Melih Mutlu wrote:
>
> On 7 Mar 2023 Tue at 04:10 Amit Kapila wrote:
>>
>> As per what I could read in this thread, most people prefer to use the
>> existing binary option rather than inventing a new way (option) to
>> binary copy in the initial sync phase. Do you
Hi,
On 7 Mar 2023 Tue at 04:10 Amit Kapila wrote:
> As per what I could read in this thread, most people prefer to use the
> existing binary option rather than inventing a new way (option) to
> binary copy in the initial sync phase. Do you agree?
I agree.
What do you think about the version
Dear Melih,
>> I think we should add description to doc that it is more likely happen to
>> fail
>> the initial copy user should enable binary format after synchronization if
>> tables have original datatype.
>
> I tried to explain when binary copy can cause failures in the doc. What
> exactly
On Wed, Mar 1, 2023 at 7:58 PM Melih Mutlu wrote:
>
> Bharath Rupireddy , 1 Mar 2023 Çar,
> 15:02 tarihinde şunu yazdı:
>>
>
> That was my intention in the beginning with this patch. Then the new option
> also made some sense at some point, and I added copy_binary option according
> to
On Thu, Mar 2, 2023 at 4:00 PM Amit Kapila wrote:
>
> On Thu, Mar 2, 2023 at 7:27 AM Peter Smith wrote:
> >
...
> > IIUC most people seem to be coming down in favour of there being a
> > single unified option (the existing 'binary==true/false) which would
> > apply to both the COPY and the data
On Thu, Mar 2, 2023 at 10:30 AM Amit Kapila wrote:
>
> > TBH I am not sure anymore if the complications justify the patch.
> >
> > It seems we have to choose from 2 bad choices:
> > - separate options = this works but would be more confusing for the user
> > - unified option = this would be
On Thu, Mar 2, 2023 at 7:27 AM Peter Smith wrote:
>
> On Thu, Mar 2, 2023 at 5:10 AM Melih Mutlu wrote:
> >
> > Hi,
> >
> > Hayato Kuroda (Fujitsu) , 1 Mar 2023 Çar, 18:40
> > tarihinde şunu yazdı:
> >>
> >> Dear Melih,
> >>
> >> If we do not have to treat the case Shi pointed out[1] as
On Thu, Mar 2, 2023 at 5:10 AM Melih Mutlu wrote:
>
> Hi,
>
> Hayato Kuroda (Fujitsu) , 1 Mar 2023 Çar, 18:40
> tarihinde şunu yazdı:
>>
>> Dear Melih,
>>
>> If we do not have to treat the case Shi pointed out[1] as code-level, I
>> agreed to
>> same option binary because it is simpler.
>
>
>
Hi,
Hayato Kuroda (Fujitsu) , 1 Mar 2023 Çar, 18:40
tarihinde şunu yazdı:
> Dear Melih,
>
> If we do not have to treat the case Shi pointed out[1] as code-level, I
> agreed to
> same option binary because it is simpler.
How is this an issue if we let the binary option do binary copy and not an
Dear Melih,
If we do not have to treat the case Shi pointed out[1] as code-level, I agreed
to
same option binary because it is simpler. I read the use-cases addressed by
Bharath[2]
and I cannot find advantage for case 1 and 3 expect the case that binary
functions
are not implemented.
Hi,
Bharath Rupireddy , 1 Mar 2023 Çar,
15:02 tarihinde şunu yazdı:
> On Wed, Mar 1, 2023 at 4:47 PM Dilip Kumar wrote:
> > I agree with this thought, basically adding an extra option will
> > always complicate things for the user. And logically it doesn't make
> > much sense to copy data in
On Wed, Mar 1, 2023 at 4:47 PM Dilip Kumar wrote:
>
> > > walsender ERROR: no binary output function available for type
> > > public.myvarchar
> > > walsender STATEMENT: COPY public.tbl1 (a) TO STDOUT WITH (FORMAT binary)
> > >
> >
> > Thanks for sharing the example. I think to address this
On Mon, Feb 27, 2023 at 2:31 PM Amit Kapila wrote:
>
> On Mon, Feb 20, 2023 at 3:37 PM shiy.f...@fujitsu.com
> wrote:
> >
> > On Thu, Feb 16, 2023 8:48 PM Amit Kapila wrote:
> > >
> > > So, doesn't this mean that there is no separate failure mode during
> > > the initial copy? I am clarifying
Hi,
Attached v11.
vignesh C , 28 Şub 2023 Sal, 13:59 tarihinde şunu
yazdı:
> Thanks for the patch, Few comments:
> 1) Are primary key required for the tables, if not required we could
> remove the primary key which will speed up the test by not creating
> the indexes and inserting in the
> 3. I think the newly added tests must verify if the binary COPY is
> picked up when enabled. Perhaps, looking at the publisher's server log
> for 'COPY ... WITH BINARY format'? Maybe it's an overkill, otherwise,
> we have no way of testing that the option took effect.
Another way to test that
On Tue, Feb 28, 2023 at 1:22 AM Melih Mutlu wrote:
>
> Hi,
>
> Thanks for all of your reviews!
>
> So, I made some changes in the v10 according to your comments.
Thanks. Some quick comments on v10:
1.
+
+ If true, initial data synchronization will be performed in binary format
+
On Tue, 28 Feb 2023 at 01:22, Melih Mutlu wrote:
>
> Hi,
>
> Thanks for all of your reviews!
>
> So, I made some changes in the v10 according to your comments.
>
> 1- copy_format is changed to a boolean parameter copy_binary.
> It sounds easier to use a boolean to enable/disable binary copy.
Hi,
Thanks for all of your reviews!
So, I made some changes in the v10 according to your comments.
1- copy_format is changed to a boolean parameter copy_binary.
It sounds easier to use a boolean to enable/disable binary copy. Default
value is false, so nothing changes in the current behaviour
On Tue, Feb 21, 2023 at 7:18 PM Bharath Rupireddy
wrote:
>
> On Mon, Feb 20, 2023 at 5:17 PM Melih Mutlu wrote:
> >
> > Thanks for letting me know.
> > Attached the fixed version of the patch.
>
> Thanks. I have few comments on v9 patch:
>
> 1.
> +/* Do not allow binary =
On Fri, Feb 24, 2023 at 8:32 AM Peter Smith wrote:
>
> Here is my summary of this thread so far, plus some other thoughts.
Thanks.
> 1. Initial Goal
> --
>
> Currently, the CREATE SUBSCRIPTION ... WITH(binary=true) mode does
> data replication in binary mode, but tablesync COPY
On Mon, Feb 20, 2023 at 3:37 PM shiy.f...@fujitsu.com
wrote:
>
> On Thu, Feb 16, 2023 8:48 PM Amit Kapila wrote:
> >
> > So, doesn't this mean that there is no separate failure mode during
> > the initial copy? I am clarifying this to see if the patch really
> > needs a separate copy_format
On Monday, February 20, 2023 8:47 PM Melih Mutlu wrote:
> Thanks for letting me know.
> Attached the fixed version of the patch.
Hi, Melih
Thanks for updating the patch. Minor comments on v9.
(1) commit message
"The patch introduces a new parameter, copy_format, to CREATE SUBSCRIPTION to
Here is my summary of this thread so far, plus some other thoughts.
(I wrote this mostly for my own internal notes/understanding, but
maybe it is a helpful summary for others so I am posting it here too).
~~
1. Initial Goal
--
Currently, the CREATE SUBSCRIPTION ...
> This is because copy_data is not something stored in pg_subscription
> or another catalog. But this is not an issue for copy_fornat since its
> value will be stored in the catalog. This can allow users to set the
> format even if copy_data=false and no initial sync is needed at that
> moment.
shiy.f...@fujitsu.com , 23 Şub 2023 Per, 12:29
tarihinde şunu yazdı:
> On Thu, Feb 23, 2023 12:40 PM Kuroda, Hayato/黒田 隼人 <
> kuroda.hay...@fujitsu.com> wrote:
> >
> > > > I'm not sure the combination of "copy_format = binary" and
> "copy_data =
> > false"
> > > > should be accepted or not. How
On Thu, Feb 23, 2023 12:40 PM Kuroda, Hayato/黒田 隼人
wrote:
>
> > > I'm not sure the combination of "copy_format = binary" and "copy_data =
> false"
> > > should be accepted or not. How do you think?
> >
> > It seems quite useless indeed to specify the format of a copy that won't
> happen.
>
> I
Dear Jelte,
> I don't think it's necessary to check versions. Yes, there are
> situations where binary will fail across major versions. But in many
> cases it does not. To me it seems the responsibility of the operator
> to evaluate this risk. And if the operator chooses wrong and uses
> binary
> If in future version the general data type is added, the copy command in
> binary
> format will not work well, right? It is because the inferior version does not
> have
> recv/send functions for added type. It means that there is a possibility that
> replication between different versions may
Dear Melih,
Thank you for updating the patch! Followings are my comments.
01. catalogs.sgml
```
If true, the subscription will request that the publisher send data
- in binary format
```
I'm not clear here. subbinary does not directly mean that whether the worker
requests to send
On Mon, Feb 20, 2023 at 5:17 PM Melih Mutlu wrote:
>
> Thanks for letting me know.
> Attached the fixed version of the patch.
Thanks. I have few comments on v9 patch:
1.
+/* Do not allow binary = false with copy_format = binary */
+if (!opts.binary &&
+
Amit Kapila , 16 Şub 2023 Per, 15:47 tarihinde
şunu yazdı:
> On Mon, Jan 30, 2023 at 4:19 PM Melih Mutlu
> wrote:
> >> 8. Note that the COPY binary format isn't portable across platforms
> >> (Windows to Linux for instance) or major versions
> >>
Hi,
Hayato Kuroda (Fujitsu) , 20 Şub 2023 Pzt, 10:12
tarihinde şunu yazdı:
> Dear Melih,
>
> Thank you for updating the patch. Before reviewing, I found that
> cfbot have not accepted v8 patch [1].
>
Thanks for letting me know.
Attached the fixed version of the patch.
Best,
--
Melih Mutlu
On Thu, Feb 16, 2023 8:48 PM Amit Kapila wrote:
>
> On Mon, Jan 30, 2023 at 4:19 PM Melih Mutlu
> wrote:
> >
> > Logical replication between different types like int and smallint is
> > already not
> working properly on HEAD too.
> > Yes, the scenario you shared looks like working. But you
Dear Melih,
Thank you for updating the patch. Before reviewing, I found that
cfbot have not accepted v8 patch [1].
IIUC src/psql/describe.c has been modified in v8, but
src/test/regress/expected/subscription.out
has not been changed accordingly.
[1]:
On Mon, Jan 30, 2023 at 4:19 PM Melih Mutlu wrote:
>
> Hi Bharath,
>
> Thanks for reviewing.
>
> Bharath Rupireddy , 18 Oca 2023 Çar,
> 10:17 tarihinde şunu yazdı:
>>
>> On Thu, Jan 12, 2023 at 1:53 PM Melih Mutlu wrote:
>> 1. The performance numbers posted upthread [1] look impressive for the
Hi,
Thanks for reviewing. Please see the v8 here [1]
Takamichi Osumi (Fujitsu) , 1 Şub 2023 Çar,
06:05 tarihinde şunu yazdı:
> (1) general comment
>
> I wondered if the addition of the new option/parameter can introduce some
> confusion to the users.
>
> case 1. When binary = true and
Hi,
Please see the attached patch for following changes.
Bharath Rupireddy , 30 Oca 2023
Pzt, 15:34 tarihinde şunu yazdı:
> On Mon, Jan 30, 2023 at 4:19 PM Melih Mutlu
> wrote:
It'd be better and clearer to have a separate TAP test file IMO since
> the focus of the feature here isn't to just
Hi, Melih
On Monday, January 30, 2023 7:50 PM Melih Mutlu wrote:
> Thanks for reviewing.
...
> Well, with this patch, it will begin to fail in the table copy phase...
The latest patch doesn't get updated for more than two weeks
after some review comments. If you don't have time,
I would like
On Monday, January 30, 2023 7:50 PM Melih Mutlu wrote:
> Thanks for reviewing.
...
> Please see [1] and you'll get the following error in your case:
> "ERROR: incorrect binary data format in logical replication column 1"
Hi, thanks for sharing v7.
(1) general comment
I wondered if the
On Mon, Jan 30, 2023 at 4:19 PM Melih Mutlu wrote:
>
Thanks for providing an updated patch.
>> On Thu, Jan 12, 2023 at 1:53 PM Melih Mutlu wrote:
>> 1. The performance numbers posted upthread [1] look impressive for the
>> use-case tried, that's a 2.25X improvement or 55.6% reduction in
>>
Hi Bharath,
Thanks for reviewing.
Bharath Rupireddy , 18 Oca 2023
Çar, 10:17 tarihinde şunu yazdı:
> On Thu, Jan 12, 2023 at 1:53 PM Melih Mutlu
> wrote:
> 1. The performance numbers posted upthread [1] look impressive for the
> use-case tried, that's a 2.25X improvement or 55.6% reduction in
On Thu, Jan 12, 2023 at 1:53 PM Melih Mutlu wrote:
>
> Hi,
>
> Thanks for your reviews.
Thanks. I have some comments:
1. The performance numbers posted upthread [1] look impressive for the
use-case tried, that's a 2.25X improvement or 55.6% reduction in
execution times. However, it'll be great
Hi,
Thanks for your reviews.
Takamichi Osumi (Fujitsu) , 12 Oca 2023 Per,
06:07 tarihinde şunu yazdı:
> On Wednesday, January 11, 2023 7:45 PM Melih Mutlu
> wrote:
> (1-1) There is no need to log the version and the query by elog here.
> (1-2) Also, I suggest we can remove the server_version
On Wednesday, January 11, 2023 7:45 PM Melih Mutlu
wrote:
> Thanks for your review.
> Done.
Hi, minor comments on v5.
(1) publisher's version check
+ /* If the publisher is v16 or later, copy in binary format.*/
+ server_version = walrcv_server_version(LogRepWorkerWalRcvConn);
+
On Wed, 11 Jan 2023 at 16:14, Melih Mutlu wrote:
>
> Hi,
>
> Thanks for your review.
>
> shiy.f...@fujitsu.com , 11 Oca 2023 Çar, 11:56
> tarihinde şunu yazdı:
>>
>> On Mon, Nov 14, 2022 8:08 PM Melih Mutlu wrote:
>> 1.
>> +# Binary enabled subscription should fail
>>
Hi,
Thanks for your review.
shiy.f...@fujitsu.com , 11 Oca 2023 Çar, 11:56
tarihinde şunu yazdı:
> On Mon, Nov 14, 2022 8:08 PM Melih Mutlu wrote:
> 1.
> +# Binary enabled subscription should fail
> +$node_subscriber_binary->wait_for_log("ERROR: insufficient data left in
> message");
>
>
On Mon, Nov 14, 2022 8:08 PM Melih Mutlu wrote:
>
> Attached patch with updated version of this patch.
Thanks for your patch.
I tried to do a performance test for this patch, the result looks good to me.
(The steps are similar to what Melih shared [1].)
The time to synchronize about 1GB data
Hello,
osumi.takami...@fujitsu.com , 12 Eki 2022 Çar,
04:36 tarihinde şunu yazdı:
> > I agree with the direction to support binary copy for v16 and
> later.
> >
> > IIUC, the binary format replication with different data types
> fails even
> > during apply phase on HEAD.
> > I
1 - 100 of 113 matches
Mail list logo
