On Tues, Apr 4, 2023 at 23:48 PM Tom Lane wrote:
> Nathan Bossart writes:
> > On Wed, Feb 22, 2023 at 12:40:07PM +, wangw.f...@fujitsu.com wrote:
> >> After some rethinking, I think users can easily get exact value according
> >> to
> >> exact formula, an
On Tues, Mar 28, 2023 at 18:00 PM Wang, Wei/王 威 wrote:
> Attach the new patch.
Sorry, I attached the wrong patch.
Here is the correct new version patch which addressed all comments so far.
Regards,
Wang Wei
v25-0001-Avoid-syncing-data-twice-for-the-publish_via_par.patch
Description: v25-0001-
On Tues, Mar 28, 2023 at 7:02 AM Jacob Champion wrote:
> On Mon, Mar 20, 2023 at 11:22 PM Amit Kapila
> wrote:
> > If the tests you have in mind are only related to this patch set then
> > feel free to propose them here if you feel the current ones are not
> > sufficient.
>
> I think the new tes
On Mon, Mar 27, 2023 at 11:32 AM Amit Kapila wrote:
> On Mon, Mar 27, 2023 at 7:03 AM Peter Smith
> wrote:
> >
> >
> > 1.
> > +# two publications, one publishing through ancestor and another one
> > directly
> > +# publsihing the partition, with different row filters
> > +$node_publisher->safe_p
On Fri, Mar 24, 2023 at 9:49 AM Peter Smith wrote:
> On Thu, Mar 23, 2023 at 8:11 PM wangw.f...@fujitsu.com
> wrote:
> >
> > On Thu, Mar 23, 2023 at 12:27 PM Peter Smith
> wrote:
> > > Here are some review comments for patch v20-0001.
> >
> ...
>
On Fri, Mar 24, 2023 at 14:17 PM Amit Kapila wrote:
> > > > ==
> > > > src/test/subscription/t/028_row_filter.pl
> > > >
> > > > 7.
> > > > +# insert data into partitioned table.
> > > > +$node_publisher->safe_psql('postgres',
> > > > + "INSERT INTO tab_rowfilter_viaroot_part(a) VALUES(13), (1
On Fri, Mar 24, 2023 at 10:14 AM Peter Smith wrote:
>
Thanks for the information.
> BTW, since this patch changes the signature of the API
> pg_get_publication_tables, I assume the example in the CREATE
> SUBSCRIPTION Notes [1] may not work anymore.
The use case you mentioned is still work.
>
On Thu, Mar 23, 2023 at 12:27 PM Peter Smith wrote:
> Here are some review comments for patch v20-0001.
Thanks for your comments.
> ==
> src/backend/commands/subscriptioncmds.c
>
> 3. fetch_table_list
>
> + /* Get the list of tables from the publisher. */
> + if (server_version >= 16)
On Wed, Mar 22, 2023 at 14:32 PM Kuroda, Hayato/黒田 隼人
wrote:
> Dear Wang,
>
> Thank you for updating patch! Following are comments form v19-0001.
Thanks for your comments.
> 01. logical-replication.sgml
>
> I found a following statement in logical-replication.sgml. I think this may
> cause
>
On Wed, Mar 22, 2023 at 12:50 PM Peter Smith wrote:
> Here are some review comments for patch code of HEAD_v19-0001
Thanks for your comments.
> ==
> doc/src/sgml/ref/create_publication.sgml
>
> 1.
> +
> + There can be a case where a subscription combines multiple
> +
On Mon, Mar 20, 2023 at 21:18 PM Kuroda, Hayato/黒田 隼人
wrote:
> Dear Wang,
>
> I have tested about multilevel partitions, and it worked well.
> Followings are my comments for v18-0001.
Thanks for your comments and testing.
> 01. pg_get_publication_tables
>
> ```
> + ListCell *l
On Mon, Mar 20, 2023 at 15:32 PM Peter Smith wrote:
> Here are some review comments for v17-0001.
Thanks for your comments.
> ==
> src/backend/catalog/pg_publication.c
>
> 1. filter_partitions
>
> -static List *
> -filter_partitions(List *relids)
> +static void
> +filter_partitions(List *t
On Sat, Mar 18, 2023 at 7:37 AM Jacob Champion wrote:
> On Thu, Mar 16, 2023 at 11:28 PM wangw.f...@fujitsu.com
> wrote:
> > Attach the new patch set.
Thanks for your comments and testing.
> For example, the corner case mentioned in 0003, with multiple
> publications
On Mon, Mar 20, 2023 at 18:15 PM Amit Kapila wrote:
>
Thanks for your comments.
> On Mon, Mar 20, 2023 at 1:02 PM Peter Smith
> wrote:
> >
> >
> > ==
> > src/include/catalog/pg_proc.dat
> >
> > 4.
> > +{ oid => '6119',
> > + descr => 'get information of the tables in the given publication
On Fri, Mar 17, 2023 at 20:07 PM Amit Kapila wrote:
> On Fri, Mar 17, 2023 at 11:58 AM wangw.f...@fujitsu.com
> wrote:
> >
> > On Thu, Mar 16, 2023 at 20:25 PM Amit Kapila
> wrote:
> > >
> >
> > Thanks for your comments
On Thu, Mar 16, 2023 at 20:25 PM Amit Kapila wrote:
>
Thanks for your comments.
> + if (server_version >= 16)
> + {
> + appendStringInfo(&cmd, "SELECT DISTINCT N.nspname, C.relname,\n"
> + " ( SELECT array_agg(a.attname ORDER BY a.attnum)\n"
> + "FROM pg_attribut
On Tues, Mar 14, 2023 12:17 PM Ajin Cherian wrote:
> On Mon, Mar 13, 2023 at 2:24 AM Zheng Li wrote:
> >
> > Thanks for working on the test coverage for CREATE and ALTER TABLE.
> > I've made fixes for some of the failures in the v79 patch set (0002,
> > 0003 and 0004 are updated). The changes inc
On Fri, Mar 10, 2023 20:17 PM Osumi, Takamichi/大墨 昂道
wrote:
> Hi,
>
>
> On Friday, March 10, 2023 6:32 PM Wang, Wei/王 威
> wrote:
> > Attach the new patch set.
> Thanks for updating the patch ! One review comment on v7-0005.
Thanks for your comment.
> stream_start_cb_wrapper and stream_stop_c
On Mon, Mar 10, 2023 14:35 PM Amit Kapila wrote:
> On Fri, Mar 10, 2023 at 11:17 AM Peter Smith wrote:
> >
> > On Fri, Mar 10, 2023 at 3:32 PM Amit Kapila wrote:
> > >
> > > On Thu, Mar 9, 2023 at 10:56 AM Peter Smith
> wrote:
> > > >
> > > > 2. rollback_prepared_cb_wrapper
> > > >
> > > > /*
On Thur, Mar 9, 2023 13:26 PM Peter Smith wrote:
> Here are some review comments for v6-0001
Thanks for your comments.
> ==
> General.
>
> 1.
> There are lots of new comments saying:
> /* don't call update progress, we didn't really make any */
>
> but is the wording "call update progress"
On Wed, Mar 8, 2023 23:55 PM Osumi, Takamichi/大墨 昂道
wrote:
> Hi,
>
>
> On Wednesday, March 8, 2023 11:54 AM From: wangw.f...@fujitsu.com
> wrote:
> > Attach the new patch.
> Thanks for sharing v6 ! Few minor comments for the same.
Thanks for your comments.
> (1)
On Wed, Mar 8, 2023 19:06 PM Kuroda, Hayato/黒田 隼人
wrote:
> Dear Wang,
Thanks for your testing and comments.
> ---
> ```
> +/*
> + * Update progress tracking and send keep alive (if required).
> + */
> +static void
> +UpdateProgressAndKeepalive(LogicalDecodingContext *ctx, bool finished_xact)
>
On Mon, Mar 10, 2023 11:56 AM Amit Kapila wrote:
> On Wed, Mar 8, 2023 at 8:24 AM wangw.f...@fujitsu.com
> wrote:
> >
> > Attach the new patch.
> >
>
> I think this combines multiple improvements in one patch. We can
> consider all of them together or maybe it w
On Mon, Mar 6, 2023 18:17 PM Wang, Wei/王 威 wrote:
> On Mon, Mar 6, 2023 14:34 AM Ajin Cherian wrote:
> > Changes are in patch 1 and patch 2
>
> Thanks for updating the patch set.
>
> Here are some comments:
Here are some more comments for v-75-0002* patch:
1. In the function deparse_AlterRela
On Tue, Mar 7, 2023 15:55 PM Kuroda, Hayato/黒田 隼人
wrote:
> Dear Wang,
>
> Thank you for updating the patch! Followings are my comments.
Thanks for your comments.
> ---
> 01. missing comments
> You might miss the comment from Peter[1]. Or could you pin the related one?
Since I think the functi
On Fri, Mar 3, 2023 8:18 AM Peter Smith wrote:
>
Thanks for your comments.
> 1.
> +
> +static void UpdateProgressAndKeepalive(LogicalDecodingContext *ctx,
> +bool finished_xact);
> +
> +static bool is_keepalive_threshold_exceeded(LogicalDecodingContext *ctx);
>
> 1a.
> There is an unnecess
On Mon, Mar 6, 2023 14:34 AM Ajin Cherian wrote:
> Changes are in patch 1 and patch 2
Thanks for updating the patch set.
Here are some comments:
For v-75-0002* patch.
1. In the function deparse_AlterRelation.
+ if ((sub->address.objectId != relId &&
+sub->a
On Thu, Mar 2, 2023 at 4:19 AM Gregory Stark (as CFM)
wrote:
> On Wed, 8 Feb 2023 at 15:04, Andres Freund wrote:
> >
> > Attached is a current, quite rough, prototype. It addresses some of the
> > points
> > raised, but far from all. There's also several XXXs/FIXMEs in it. I changed
> >
On Tues, Feb 28, 2023 at 11:31 AM Osumi, Takamichi/大墨 昂道
wrote:
> Hi,
>
>
> On Monday, February 27, 2023 6:30 PM wangw.f...@fujitsu.com
> wrote:
> > Attach the new patch.
> Thanks for sharing v3. Minor review comments and question.
Thanks
On Tues, Feb 28, 2023 at 9:12 AM Peter Smith wrote:
> Here are some comments for the v2-0001 patch.
>
> (I haven't looked at the v3 that was posted overnight; maybe some of
> my comments have already been addressed.)
Thanks for your comments.
> ==
> General
>
> 1. (Info from the commit mes
On Thur, Feb 23, 2023 at 18:41 PM Kuroda, Hayato/�\田 隼人
wrote:
> Dear Wang,
>
> Thank you for making the patch. IIUC your patch basically can achieve that
> output plugin
> does not have to call UpdateProgress.
Thanks for your review and comments.
> I think the basic approach is as follows, is
On Wed, Feb 22, 2023 at 8:37 AM Nathan Bossart wrote:
> On Wed, Feb 15, 2023 at 08:16:43AM +0000, wangw.f...@fujitsu.com wrote:
> > When I refer to the GUC "max_locks_per_transaction", I find that the
> description
> > of the shared lock table size in pg-doc[1] i
On Sun, Feb 19, 2023 at 21:06 PM Wang, Wei/王 威 wrote:
> On Thur, Feb 14, 2023 at 2:03 AM Andres Freund wrote:
> > On 2023-02-13 14:06:57 +0530, Amit Kapila wrote:
> > > > > The patch calls update_progress in change_cb_wrapper and other
> > > > > wrappers which will miss the case of DDLs that gene
On Thur, Feb 14, 2023 at 2:03 AM Andres Freund wrote:
> On 2023-02-13 14:06:57 +0530, Amit Kapila wrote:
> > > > The patch calls update_progress in change_cb_wrapper and other
> > > > wrappers which will miss the case of DDLs that generates a lot of data
> > > > that is not processed by the plugin
Hi,
When I refer to the GUC "max_locks_per_transaction", I find that the description
of the shared lock table size in pg-doc[1] is inconsistent with the code
(guc_table.c). BTW, the GUC "max_predicate_locks_per_xact" has similar problems.
I think the descriptions in pg-doc are correct.
- GUC "max
On Thur, Feb 7, 2023 15:29 PM I wrote:
> On Wed, Feb 1, 2023 20:07 PM Melih Mutlu wrote:
> > Thanks for pointing out this review. I somehow skipped that, sorry.
> >
> > Please see attached patches.
>
> Thanks for updating the patch set.
> Here are some comments.
Hi, here are some more comments o
On Wed, Feb 8, 2023 4:29 AM Andres Freund wrote:
> Hi,
>
> On 2022-11-16 08:58:31 +, wangw.f...@fujitsu.com wrote:
> > Attach the new patch set.
>
> This patch causes several of the tests to fail. See e.g.:
>
> https://cirrus-ci.com/task/6587624765259776
>
&g
On Tue, Feb 7, 2023 15:37 PM Amit Kapila wrote:
> On Tue, Feb 7, 2023 at 12:41 PM Masahiko Sawada
> wrote:
> >
> > On Fri, Feb 3, 2023 at 6:44 PM Amit Kapila wrote:
> >
> > > We need to think of a predictable
> > > way to test this path which may not be difficult. But I guess it would
> > > be b
On Wed, Feb 1, 2023 20:07 PM Melih Mutlu wrote:
> Thanks for pointing out this review. I somehow skipped that, sorry.
>
> Please see attached patches.
Thanks for updating the patch set.
Here are some comments.
1. In the function ApplyWorkerMain.
+ /* This is main apply wor
On Thurs, Feb 2, 2023 16:04 PM Takamichi Osumi (Fujitsu)
wrote:
> Attached the updated patch v26 accordingly.
Thanks for your patch.
Here is a comment:
1. The checks in function AlterSubscription
+ /*
+* The combinat
On Tues, Jan 31, 2023 18:27 PM I wrote:
> I found one typo in v9-0002, but it seems already mentioned by Shi in [1].#5
> before. Maybe you can have a look at that email for this and some other
> comments.
Sorry, I forgot to add the link to the email. Please refer to [1].
[1] -
https://www.postgr
On Mon, Jan 23, 2023 21:00 PM Melih Mutlu wrote:
> Hi,
>
> Thanks for your review.
> Attached updated versions of the patches.
Thanks for updating the patch set.
> > 5. New member "created_slot" in structure LogicalRepWorker
> > + /*
> > +* Indicates if the sync worker created a r
On Mon, Jan 30, 2023 at 17:50 PM I wrote:
> Attach the new patch.
When invoking the function ReorderBufferProcessTXN, the threshold-related
counter "changes_count" may have some random value from the previous
transaction's processing. To fix this, I moved the definition of the counter
"changes_cou
On Mon, Jan 30, 2023 at 14:55 PM Amit Kapila wrote:
> On Mon, Jan 30, 2023 at 10:36 AM wangw.f...@fujitsu.com
> wrote:
> >
> > On Mon, Jan 30, 2023 11:37 AM Shi, Yu/侍 雨
> wrote:
> > > On Sun, Jan 29, 2023 3:41 PM wangw.f...@fujitsu.com
> > > wrote:
> &
On Mon, Jan 30, 2023 11:37 AM Shi, Yu/侍 雨 wrote:
> On Sun, Jan 29, 2023 3:41 PM wangw.f...@fujitsu.com
> wrote:
> >
> > I tested a mix transaction of transactional and non-transactional messages
> > on
> > the current HEAD and reproduced the timeout proble
On Fri, Jan 27, 2023 at 19:55 PM Amit Kapila wrote:
> On Fri, Jan 27, 2023 at 5:18 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Wednesday, January 25, 2023 7:26 PM Amit Kapila
>
> > >
> > > On Tue, Jan 24, 2023 at 8:15 AM wangw.f...@fujitsu.com
>
On Tues, Jan 24, 2023 at 8:28 AM Peter Smith wrote:
> Hi Hou-san, Here are my review comments for v5-0001.
Thanks for your comments.
> ==
> src/backend/replication/logical/reorderbuffer.c
>
> 1.
> @@ -2446,6 +2452,23 @@ ReorderBufferProcessTXN(ReorderBuffer *rb,
> ReorderBufferTXN *txn,
>
On Fri, Jan 20, 2023 at 10:10 AM Peter Smith wrote:
> Here are some review comments for patch v3-0001.
Thanks for your comments.
> ==
> Commit message
>
> 1.
> The problem is when there is a DDL in a transaction that generates lots of
> temporary data due to rewrite rules, these temporary d
On Fri, Jan 20, 2023 at 12:35 PM Amit Kapila wrote:
> On Fri, Jan 20, 2023 at 7:40 AM Peter Smith wrote:
> >
> > Here are some review comments for patch v3-0001.
> >
> > ==
> > src/backend/replication/logical/logical.c
> >
> > 3. forward declaration
> >
> > +/* update progress callback */
> >
On Thu, Jan 19, 2023 at 19:37 PM Amit Kapila wrote:
> On Thu, Jan 19, 2023 at 4:13 PM Ashutosh Bapat
> wrote:
> >
> > On Wed, Jan 18, 2023 at 6:00 PM Amit Kapila
> wrote:
> >
> > > + */
> > > + ReorderBufferUpdateProgressCB update_progress;
> > >
> > > Are you suggesting changing the name of the
On Thurs, Jan 19, 2023 at 19:18 PM Amit Kapila wrote:
> On Wed, Jan 11, 2023 at 4:20 PM wangw.f...@fujitsu.com
> wrote:
> >
> > When I was reading the "Logical Decoding Output Plugins" chapter in pg-doc
> [1],
> > I think in the summary section, only the call
On Wed, Jan 18, 2023 at 13:29 PM Amit Kapila wrote:
> On Tue, Jan 17, 2023 at 6:41 PM Ashutosh Bapat
> wrote:
> >
> > On Tue, Jan 17, 2023 at 3:34 PM Amit Kapila wrote:
> >
> > > >
> > > > I am a bit worried about the indirections that the wrappers and hooks
> > > > create. Output plugins call O
On Wed, Jan 18, 2023 12:36 PM Amit Kapila wrote:
> On Tue, Jan 17, 2023 at 8:07 PM Masahiko Sawada
> wrote:
> >
> > On Tue, Jan 17, 2023 at 6:14 PM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > On Tuesday, January 17, 2023 2:46 PM Masahiko Sawada
> wrote:
> > > >
> > > > On Tue, Jan 17, 2023 at
On Wed, Jan 11, 2023 4:31 PM Melih Mutlu wrote:
> Rebased the patch to resolve conflicts.
Thanks for your patch set.
Here are some comments:
v3-0001* patch
===
1. typedefs.list
I think we also need to add "walrcv_slot_snapshot_fn" to this file.
v7-0002* patch
===
1. Ab
Hi,
When I was reading the "Logical Decoding Output Plugins" chapter in pg-doc [1],
I think in the summary section, only the callback message_cb is not described
whether it is required or optional, and the description of callback
stream_prepare_cb seems inaccurate.
And after the summary section,
On Mon, Jan 9, 2023 at 13:04 PM Amit Kapila wrote:
>
Thanks for your comments.
> One more thing, I think it would be better to expose a new callback
> API via reorder buffer as suggested previously [2] similar to other
> reorder buffer APIs instead of directly using reorderbuffer API to
> invoke
On Fri, Jan 6, 2023 at 15:06 PM Ashutosh Bapat
wrote:
> Hi Wang,
> Thanks for working on this. One of our customer faced a similar
> situation when running BDR with PostgreSQL.
>
> I tested your patch and it solves the problem.
>
> Please find some review comments below
Thanks for your testing
On Mon, Dec 26, 2022 21:02 PM Masahiko Sawada wrote:
> Thank you for updating the patches. Here are some comments for 0001
> and 0002 patches:
Thanks for your comments.
> I think it'd be better to write logs when the leader enters the
> serialization mode. It would be helpful for investigating i
On Sat, Nov 26, 2022 at 19:50 PM Amit Kapila wrote:
> On Fri, Nov 25, 2022 at 5:30 PM Ashutosh Bapat
> wrote:
> >
> > Hi Hou,
> > Thanks for the patch. With a simple condition, we can eliminate the
> > need to queueing snapshot change in the current transaction and then
> > applying it. Saves som
On Thurs, Nov 17, 2022 at 13:58 PM vignesh C wrote:
> On Wed, 16 Nov 2022 at 14:28, wangw.f...@fujitsu.com
> wrote:
> >
> > On Mon, Nov 14, 2022 at 0:56 AM vignesh C wrote:
> > > >
> > > > Attach new patches.
> > >
> >
> > Thanks
On Mon, Nov 14, 2022 at 0:56 AM vignesh C wrote:
> >
> > Attach new patches.
>
Thanks for your comments.
> Here we are having tables list to store the relids and table_infos
> list which stores pubid along with relid. Here tables list acts as a
> temporary list to get filter_partitions and then
On Tues, Nov 8, 2022 at 12:12 PM Osumi, Takamichi/大墨 昂道
wrote:
> On Monday, October 17, 2022 2:49 PM Wang, Wei/王 威
> wrote:
> > Attach the new patch set.
> Hi, thank you for posting the new patches.
>
>
> Here are minor comments on the HEAD_v13-0002.
Thanks for your comments.
> (1) Suggestio
On Fri, Oct 21, 2022 at 17:02 PM Peter Smith wrote:
> Here are my review comments for HEAD patches v13*
Thanks for your comments.
> Patch HEAD_v13-0002
>
> 1. Commit message
>
> The following usage scenarios are not described in detail in the manual:
> If one subscription subscribes multiple
On Fri, Oct 21, 2022 at 17:02 PM Peter Smith wrote:
>
Thanks for your comments. Sorry for not replying in time.
> On Mon, Oct 17, 2022 at 4:49 PM wangw.f...@fujitsu.com
> wrote:
> >
> > On Wed, Oct 5, 2022 at 11:08 AM Peter Smith
> wrote:
> > > Hi Wang-san
On Fri, Nov 4, 2022 at 18:13 PM Fabrice Chapuis wrote:
> Hello Wang,
>
> I tested the draft patch in my lab for Postgres 14.4, the refresh of the
> materialized view ran without generating the timeout on the worker.
> Do you plan to propose this patch at the next commit fest.
Thanks for your con
On Thurs, Oct 20, 2022 at 13:47 PM Fabrice Chapuis
wrote:
> Yes the refresh of MV is on the Publisher Side.
> Thanks for your draft patch, I'll try it
> I'll back to you as soonas possible
Thanks a lot.
> One question: why the refresh of the MV is a DDL not a DML?
Since in the source, the type
On Tue, Oct 18, 2022 at 22:35 PM Fabrice Chapuis
wrote:
> Hello Amit,
>
> In version 14.4 the timeout problem for logical replication happens again
> despite
> the patch provided for this issue in this version. When bulky materialized
> views
> are reloaded it broke logical replication. It is p
On Fri, Sep 23, 2022 at 0:14 AM Önder Kalacı wrote:
> Hii Wang wei,
Thanks for updating the patch and your reply.
> > 1. In the function GetCheapestReplicaIdentityFullPath.
> > + if (rel->pathlist == NIL)
> > + {
> > + /*
> > +* A sequential scan could have been dominat
On Wed, Oct 5, 2022 at 23:05 PM Osumi, Takamichi/大墨 昂道
wrote:
> Hi, thank you for the updated patches!
>
>
> Here are my minor review comments for HEAD v12.
Thanks for your comments.
> (1) typo & suggestion to reword one comment
>
>
> +* Publications support partitio
On Wed, Oct 5, 2022 at 11:08 AM Peter Smith wrote:
> Hi Wang-san. Here are my review comments for HEAD_v12-0001 patch.
Thanks for your comments.
> ==
>
> 1. Missing documentation.
>
> In [1] you wrote:
> > I think the behaviour of multiple publications with parameter
> publish_via_partitio
On Tues, Sep 27, 2022 at 16:45 PM Peter Smith wrote:
> Here are my review comments for the HEAD_v11-0001 patch:
Thanks for your comments.
> ==
>
> 1. General - Another related bug?
>
> In [1] Hou-san wrote:
>
> For another case you mentioned (via_root used when publishing child)
> CREATE
On Mon, Sep 26, 2022 at 14:47 PM Andres Freund wrote:
> Hi,
>
> On 2022-09-26 06:24:42 +, wangw.f...@fujitsu.com wrote:
> > I tried to use meson and ninja and they are really efficient.
> > But when I tried to specify "c_args", it did not take effect.
>
&g
Hi,
I tried to use meson and ninja and they are really efficient.
But when I tried to specify "c_args", it did not take effect.
Attached my steps:
[In the HEAD (7d708093b7)]
$ meson setup build --prefix /home/wangw/install/parallel_apply/ -Dcassert=true
-Dtap_tests=enabled -Dicu=enabled -Dc_args
On Mon, Sep 26, 2022 at 10:31 AM Osumi, Takamichi/大墨 昂道
wrote:
> Hi, thank you for updating the patchset.
>
>
> FYI, I noticed that the patch for head is no longer applicable.
Thanks for your kindly reminder and comment.
> $ git apply --check HEAD_v10-0001-Fix-data-replicated-twice-when-speci
On Thur, Sep 22, 2022 at 18:12 PM Amit Kapila wrote:
> Few comments on v33-0001
> ===
Thanks for your comments.
> 1.
> + else if (data->streaming == SUBSTREAM_PARALLEL &&
> + data->protocol_version <
> LOGICALREP_PROTO_STREAM_PARALLEL_VERSION_NUM)
> + ereport(ERROR,
> + (errc
On Tues, Sep 20, 2022 at 18:30 PM Önder Kalacı wrote:
> Thanks for the reviews, attached v12.
Thanks for your patch. Here is a question and a comment:
1. In the function GetCheapestReplicaIdentityFullPath.
+ if (rel->pathlist == NIL)
+ {
+ /*
+* A sequen
> FYI -
>
> The latest patch 30-0001 fails to apply, it seems due to a recent commit [1].
>
> [postgres@CentOS7-x64 oss_postgres_misc]$ git apply
> ../patches_misc/v30-0001-Perform-streaming-logical-transactions-by-
> parall.patch
> error: patch failed: src/include/replication/logicalproto.h:246
On Mon, Sept 19, 2022 at 14:52 PM Peter Smith wrote:
> FYI, I'm not sure why the cfbot hasn't reported this, but the apply v9
> patch failed for me on HEAD as below:
>
> [postgres@CentOS7-x64 oss_postgres_misc]$ git apply
> ../patches_misc/HEAD_v9-0001-Fix-data-replicated-twice-when-specifying-
>
On Tues, Sep 13, 2022 at 20:02 PM Kuroda, Hayato/黒田 隼人
wrote:
> Dear Hou-san,
>
> > I will dig your patch more, but I send partially to keep the activity of
> > the thread.
>
> More minor comments about v28.
Thanks for your comments.
> ===
> About 0002
>
> For 015_stream.pl
>
> 14. check_p
On Wed, Sep 13, 2022 at 18:26 PM Amit Kapila wrote:
>
Thanks for your comments.
> On Fri, Sep 9, 2022 at 12:32 PM Peter Smith wrote:
> >
> > 29. src/backend/replication/logical/worker.c - TransactionApplyAction
> >
> > /*
> > * What action to take for the transaction.
> > *
> > * TA_APPLY_IN
On Tues, Sep 13, 2022 at 17:49 PM Amit Kapila wrote:
>
Thanks for your comments.
> On Fri, Sep 9, 2022 at 2:31 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Friday, September 9, 2022 3:02 PM Peter Smith
> wrote:
> > >
> >
> > > 3.
> > >
> > > max_logical_replication_workers (integer)
> > >
On Mon, Sep 12, 2022 at 18:58 PM Kuroda, Hayato/黒田 隼人
wrote:
> Dear Hou-san,
>
> Thank you for updating the patch! Followings are comments for v28-0001.
> I will dig your patch more, but I send partially to keep the activity of the
> thread.
Thanks for your comments.
> ===
> For applyparallel
On Fri, Sep 9, 2022 at 15:02 PM Peter Smith wrote:
> Here are my review comments for the v28-0001 patch:
>
> (There may be some overlap with other people's review comments and/or
> some fixes already made).
Thanks for your comments.
> 5. src/backend/libpq/pqmq.c
>
> + {
> + if (IsParallelWorke
On Thur, Sep 8, 2022 at 19:25 PM Amit Kapila wrote:
> On Thu, Sep 8, 2022 at 12:21 PM Amit Kapila wrote:
> >
> > On Mon, Sep 5, 2022 at 6:34 PM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > Attach the correct patch set this time.
> > >
> >
> > Few comments on v28-0001*:
> > =
On Tues, 6 Sept 2022 at 11:14, vignesh C wrote:
> Thanks for the comments, the attached patch has the changes for the same.
Thanks for updating the patch.
Here is one comment for 0001 patch.
1. The query in function check_publications_origin.
+ appendStringInfoString(&cmd,
+
On Tues, Aug 9, 2022 at 15:15 PM Peter Smith wrote:
> Here are some review comment for the HEAD_v8 patch:
Thanks for your comments.
> 1. Commit message
>
> If there are two publications, one of them publish a parent table with
> (publish_via_partition_root = true) and another publish child tabl
On Fri, August 12, 2022 17:22 PM Peter Smith wrote:
> Here are some review comments for v20-0004:
>
> (This completes my reviews of the v20* patch set. Sorry, the reviews
> are time consuming, so I am lagging slightly behind the latest posted
> version)
Thanks for your comments.
> 1. doc/src/sg
On Thur, Jul 28, 2022 at 17:17 PM Peter Smith wrote:
> Here are some review comments for the HEAD_v7-0001 patch:
Thanks for your comments.
> 2. Commit message.
>
> 2a.
>
> If there are two publications that publish the parent table and the child
> table
> separately, and both specify the opti
On Mon, Jul 25, 2022 at 21:50 PM Amit Kapila wrote:
> Few comments on 0001:
> ==
Thanks for your comments.
> 1.
> - substream bool
> + substream char
>
>
> - If true, the subscription will allow streaming of in-progress
> - transactions
On Wed, Jul 27, 2022 at 16:03 PM Peter Smith wrote:
> Here are some review comments for the patch v19-0004:
Thanks for your kindly review and comments.
To avoid making this thread too long, I will reply to all of your comments
(0001-patch ~ 0004-patch) in this email.
In addition, in order not to
On Thurs, Jul 28, 2022 at 13:20 PM Kuroda, Hayato/黒田 隼人
wrote:
>
> Dear Wang-san,
>
> Hi, I'm also interested in the patch and I started to review this.
> Followings are comments about 0001.
Thanks for your kindly review and comments.
To avoid making this thread too long, I will reply to all of
On Sun, Jul 24, 2022 1:28 AM vignesh C wrote:
> Added a note for the same and referred it to the conflicts section.
>
> Thanks for the comments, the attached v38 patch has the changes for the same.
Thanks for your patches.
Two slight comments on the below message in the 0001 patch:
The error
On Thur, Jul 14, 2022 at 12:46 PM Peter Smith wrote:
> Here are some review comments for the v6 patch (HEAD only):
Thanks for your comments.
> 1. Commit message
>
> If there are two publications that publish the parent table and the child
> table
> separately, and both specify the option PUBLI
On Mon, Jul 4, 2022 at 14:47 AM Peter Smith wrote:
> Below are some review comments for patch v14-0004:
Thanks for your comments.
> 4.0 General.
>
> This comment is an after-thought but as I write this mail I am
> wondering if most of this 0004 patch is even necessary at all? Instead
> of intro
On Mon, Jul 4, 2022 at 12:12 AM Peter Smith wrote:
> Below are some review comments for patch v14-0003:
Thanks for your comments.
> 3.1 Commit message
>
> If any of the following checks are violated, an error will be reported.
> 1. The unique columns between publisher and subscriber are differe
On Fri, Jul 1, 2022 at 17:44 PM Amit Kapila wrote:
>
Thanks for your comments.
> On Fri, Jul 1, 2022 at 12:13 PM Peter Smith wrote:
> >
> > ==
> >
> > 1.2 doc/src/sgml/protocol.sgml - Protocol constants
> >
> > Previously I wrote that since there are protocol changes here,
> > shouldn’t ther
On Wed, May 18, 2022 4:51 PM I wrote:
> Attach the new patch.
Since there are some new commits in HEAD (0ff20288, fd0b9dc and 52b5c53) that
improve the functions pg_get_publication_tables and fetch_table_list, we cannot
apply the patch cleanly. Therefore, I rebased the patch based on the changes i
On Tues, Jun 28, 2022 at 12:15 PM Amit Kapila wrote:
> On Tue, Jun 28, 2022 at 8:51 AM wangw.f...@fujitsu.com
> wrote:
> >
> > On Thu, Jun 23, 2022 at 16:44 PM Amit Kapila
> wrote:
> > > On Thu, Jun 23, 2022 at 12:51 PM wangw.f...@fujitsu.com
> > > wrot
On Thu, Jun 23, 2022 at 9:41 AM Peter Smith wrote:
> Here are some review comments for v12-0002
Thanks for your comments.
> 3. .../subscription/t/022_twophase_cascade.pl
>
> For every test file in this patch the new function is passed $is_apply
> = 0/1 to indicate to use 'on' or 'apply' paramet
On Mon, Jun 21, 2022 at 9:41 AM Peter Smith wrote:
> Here are some review comments for the v11-0001 patch.
>
> (I will review the remaining patches 0002-0005 and post any comments later)
>
Thanks for your comments.
> 6. doc/src/sgml/protocol.sgml
>
> Since there are protocol changes made here
1 - 100 of 164 matches
Mail list logo
