On Tue, Aug 10, 2021 at 11:01 AM Masahiko Sawada wrote:
>
> On Mon, Aug 9, 2021 at 1:01 PM Peter Smith wrote:
> >
> > On Mon, Aug 9, 2021 at 12:46 PM Amit Kapila wrote:
> > >
> > > On Sun, Aug 8, 2021 at 10:21 AM Peter Smith wrote:
> > > >
&g
12. src/tools/pgindent/typedefs.list - PGOutputTxnData
PGOutputData
+PGOutputTxnData
PGPROC
=>
This change looks good, but I think it should have been done in
v12-0001 and not here in v12-0002.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Mon, Aug 9, 2021 at 12:46 PM Amit Kapila wrote:
>
> On Sun, Aug 8, 2021 at 10:21 AM Peter Smith wrote:
> >
> > On Sat, Aug 7, 2021 at 4:33 PM Amit Kapila wrote:
> > >
> > > On Thu, Jul 8, 2021 at 6:31 PM Masahiko Sawada
> > > wrote:
> &g
v1 -> v2
Rebased.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v2-0001-create-subscription-options-list-order.patch
Description: Binary data
v11 -> v12
* A rebase was needed due to some recent pgindent changes on HEAD.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v12-0001-Simplify-recent-parse_subscription_option-change.patch
Description: Binary data
PSA a patch to fix trivial comment typo in newly committed TAP test.
"amd" -> "and"
------
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-0001-Fix-typo-in-TAP-test-comment.patch
Description: Binary data
v22 --> v23
Changes:
* A rebase was needed (due to commit [1]) to keep the patch working with cfbot.
--
[1]
https://github.com/postgres/postgres/commit/93d573d86571d148e2d14415166ec6981d34ea9d
Kind Regards,
Peter Smith.
Fujitsu Australia
v23-0001-Row-filter-for-logical-replication.pa
r REFRESH
> PUBLICATION (refresh_option) may
> be specified,.."? I think keeping "refresh options" in the text would
> be good because there could be multiple such options.
>
I feel like it would be better to reword it in some way that avoids
using parentheses because they look like part of the syntax instead of
just part of the sentence.
--
Kind Regards,
Peter Smith.
Fujitsu Australia.
[1]
https://github.com/postgres/postgres/commit/63cf61cdeb7b0450dcf3b2f719c553177bac85a2
Kind Regards,
Peter Smith.
Fujitsu Australia
v22-0001-Row-filter-for-logical-replication.patch
Description: Binary data
On Tue, Aug 3, 2021 at 5:02 PM Amit Kapila wrote:
>
> On Tue, Aug 3, 2021 at 6:17 AM Peter Smith wrote:
> >
> > Please find attached the latest patch set v102*
> >
>
> I have made minor modifications in the comments and docs, please see
> attached. Can y
ind Regards,
Peter Smith.
Fujitsu Australia
v21-0001-Row-filter-for-logical-replication.patch
Description: Binary data
3dcadfbb1204a196681313
>
Earlier today the other documentation patch mentioned above was
committed by Tom Lane.
The 2PC patch v102 now fixes your review comments 2 and 3 by matching
the same datatype annotation style of that commit.
--
Kind Regards,
Peter Smith
Fujitsu Australia
/CALDaNm3U4fGxTnQfaT1TqUkgX5c0CSDvmW12Bfksis8zB_XinA%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
v102-0001-Add-prepare-API-support-for-streaming-transacti.patch
Description: Binary data
On Fri, Jul 30, 2021 at 4:33 PM Greg Nancarrow wrote:
>
> On Fri, Jul 30, 2021 at 2:02 PM Peter Smith wrote:
> >
> > Please find attached the latest patch set v100*
> >
> > v99-0002 --> v100-0001
> >
>
> A few minor comments:
>
> (1) doc/src/
On Fri, Jul 30, 2021 at 3:18 PM vignesh C wrote:
>
> On Fri, Jul 30, 2021 at 9:32 AM Peter Smith wrote:
> >
> > Please find attached the latest patch set v100*
> >
> > v99-0002 --> v100-0001
> >
> > Differences:
> >
> > * Rebased to HEAD
On Fri, Jul 30, 2021 at 6:25 PM tanghy.f...@fujitsu.com
wrote:
>
> On Friday, July 30, 2021 12:02 PM Peter Smith wrote:
> >
> > Please find attached the latest patch set v100*
> >
> > v99-0002 --> v100-0001
> >
>
> Thanks for your patch. A f
On Sat, Jul 31, 2021 at 9:36 PM Amit Kapila wrote:
>
> On Fri, Jul 30, 2021 at 9:32 AM Peter Smith wrote:
> >
> > Please find attached the latest patch set v100*
> >
>
> Few minor comments:
> 1.
> CREATE SUBSCRIPTION regress_testsub CONNECTION
> 'dbname=re
%2BVcNDUYSZVm3xNg4YLzaMqcZHqxznfbAYvJWoVzvLqFQ%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
v101-0001-Add-prepare-API-support-for-streaming-transacti.patch
Description: Binary data
On Mon, Aug 2, 2021 at 1:32 AM vignesh C wrote:
>
> On Sun, Aug 1, 2021 at 4:11 PM Peter Smith wrote:
> >
> > On Sat, Jul 31, 2021 at 7:00 AM Tom Lane wrote:
> > >
> > > vignesh C writes:
> > > [ v6-0001-Included-the-actual-datatype-used-in-logical
bly settle on some other markup.
> Or just ignore the ambiguity.
+1 to change it like suggested above.
The specific value for the flags might then look like below, but that
does not look too bad to me.
Int8 (uint8) (0)
--
Kind Regards,
Peter Smith.
Fujitsu Australia
for both
> > subscriptions to catch up.
> > Attached is a patch that addresses this issue.
>
> The changes look good to me.
>
The patch to the test code posted by Ajin LGTM also.
I applied the patch and re-ran the TAP subscription tests. All OK.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Thu, Jul 29, 2021 at 9:56 PM Amit Kapila wrote:
>
> On Tue, Jul 27, 2021 at 11:41 AM Peter Smith wrote:
> >
> > Please find attached the latest patch set v99*
> >
> > v98-0001 --> split into v99-0001 + v99-0002
> >
>
> Pushed the first refactoring
mit/201a76183e2056c2217129e12d68c25ec9c559c8
[2]
https://github.com/postgres/postgres/commit/91f9861242cd7dcf28fae216b1d8b47551c9159d
[1]
https://www.postgresql.org/message-id/CAA4eK1%2BNzcz%3DhzZJO16ZcqA7hZRa4RzGRwL_XXM%2Bdin8ehROaQ%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
v100-0001-Add-prepare-
thing, or is there some other reason?
For example:
if (msg)
res->errMsg = msg;
else
res->errMsg = libpq_gettext("out of memory\n");
VERSUS:
res->errMsg = msg ? msg : libpq_gettext("out of memory\n");
------
Kind Regards,
Peter Smith.
Fujitsu Australia
On Wed, Jul 28, 2021 at 11:32 AM Michael Paquier wrote:
>
> On Wed, Jul 28, 2021 at 09:39:02AM +1000, Peter Smith wrote:
> > IMO the PG code comments are not an appropriate place for leetspeak
> > creativity.
> >
> > PSA a patch to replace a few examples that I rece
IMO the PG code comments are not an appropriate place for leetspeak creativity.
PSA a patch to replace a few examples that I recently noticed.
"up2date" --> "up-to-date"
------
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-0001-Replace-leetspeak-comments-with-En
P test numbered 021.
--
[1]
https://github.com/postgres/postgres/commit/2b00db4fb0c7f02f000276bfadaab65a14059168
[2]
https://github.com/postgres/postgres/commit/a8fd13cab0ba815e9925dc9676e6309f699b5f72
Kind Regards,
Peter Smith.
Fujitsu Australia
v20-0001-Row-filter-for-logical-replication
BRx%3DC1Z1H_XLcRod_WYjBRv2Rn%2BDO2w%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
v99-0001-Refactor-to-make-common-functions.patch
Description: Binary data
v99-0002-Add-prepare-API-support-for-streaming-transactio.patch
Description: Binary data
On Fri, Jul 23, 2021 at 8:08 PM Amit Kapila wrote:
>
> On Tue, Jul 20, 2021 at 9:24 AM Peter Smith wrote:
> >
> > Please find attached the latest patch set v98*
> >
>
> Review comments:
>
All the following review comments are addressed in v99* p
On Fri, Jul 23, 2021 at 8:08 PM Amit Kapila wrote:
>
> On Tue, Jul 20, 2021 at 9:24 AM Peter Smith wrote:
> >
> > Please find attached the latest patch set v98*
> >
>
> Review comments:
>
[...]
> With Regards,
> Amit Kapila.
Thanks f
FYI - I have checked the v11 patch. Everything applies, builds, and
tests OK for me, and I have no more review comments. So v11 LGTM.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
the
+ * transaction has sent BEGIN or BEGIN PREPARE. BEGIN or BEGIN PREPARE
+ * is only sent when the first change in a transaction is processed.
+ * This make it possible to skip transactions that are empty.
+ */
=>
typo: "make it possible" --> "makes it possible"
--
Kind Reg
ans there were no
+ * relevant changes encountered, so we can skip the COMMIT PREPARED
+ * messsage too.
+ */
Typo: "messsage" --> "message"
(NOTE this same typo is in 2 places)
--
Kind Regards,
Peter Smith.
Fujitsu Australia
AFTER
Int8(0)
Flags (uint8); currently unused.
--
[1] https://www.postgresql.org/docs/devel/protocol-message-types.html
Kind Regards,
Peter Smith.
Fujitsu Australia.
hould also start with
uppercase.
(NOTE: This same comment was in couple of places in the patch)
--
Kind Regards,
Peter Smith.
Fujitsu Australia
>
Modified in v98. I felt it would be too verbose to add another full
example since it would be 90% the same as the current example. So I
have combined the information.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Mon, Jul 19, 2021 at 3:28 PM Greg Nancarrow wrote:
>
> On Wed, Jul 14, 2021 at 6:33 PM Peter Smith wrote:
> >
> > Please find attached the latest patch set v97*
> >
>
> I couldn't spot spot any significant issues in the v97-0001 patch, but
> do have the fo
-id/CALDaNm0LVY5A98xrgaodynnj6c%3DWQ5%3DZMpauC44aRio7-jWBYQ%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
v98-0001-Add-prepare-API-support-for-streaming-transactio.patch
Description: Binary data
On Mon, Jul 19, 2021 at 4:41 PM Amit Kapila wrote:
>
> On Mon, Jul 19, 2021 at 9:19 AM Peter Smith wrote:
> >
> > On Mon, Jul 19, 2021 at 12:43 PM Amit Kapila
> > wrote:
> > >
> > > On Mon, Jul 19, 2021 at 1:55 AM Tom Lane wrote:
> > &
utData *data = (PGOutputData *) ctx->output_plugin_private;
+ PGOutputTxnData *txndata;
TransactionId xid = InvalidTransactionId;
=>
This variable should be declared in the block where it is used,
similar to the suggestion 13a.
Also is it just an accidental omission that you did Assert(txndata)
for all the other places but not here?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
9923%40sss.pgh.pa.us
[2] https://www.postgresql.org/docs/devel/sql-prepare-transaction.html
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-0001-Fix-potential-buffer-overruns.patch
Description: Binary data
improvement within the
noise when each run is varying from 57 to 138 ms.
IMO the only conclusion you can draw from your results is that any
performance gain is too small to be observable.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
-331c33fd11c3ed85f9dbfead93f139c20ff3a25176651fc2ed37c486b97630e6
Kind Regards,
Peter Smith.
Fujitsu Australia
u want to, then you
could Assert that every list element has a consistent kind as the
initial kind, but maybe that is overkill too?
PSA a small tmp patch to demonstrate what this comment is about.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v19-0001-PS-tmp-OpenTableList-IsA-logic.patch
Description: Binary data
On Wed, Jul 14, 2021 at 4:23 PM Amit Kapila wrote:
>
> On Mon, Jul 12, 2021 at 9:14 AM Peter Smith wrote:
> >
> > On Sun, Jul 11, 2021 at 8:20 PM Amit Kapila wrote:
> > >
> > > On Fri, Jul 9, 2021 at 4:43 AM Peter Smith wrote:
> > > >
> &
On Wed, Jul 14, 2021 at 12:15 AM Tom Lane wrote:
>
> Michael Paquier writes:
> > On Tue, Jul 13, 2021 at 07:02:27PM +1000, Peter Smith wrote:
> >> I found a few functions making unnecessary repeated calls to
> >> PQserverVersion(conn); instead of just calling once
Hi,
I found a few functions making unnecessary repeated calls to
PQserverVersion(conn); instead of just calling once and assigning to a
local variable.
PSA a little patch which culls those extra calls.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-0001-Avoid-unnecessary-calls
=bfodqwgbntzofk0q1l...@mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
On Tue, Jul 13, 2021 at 1:25 PM Peter Smith wrote:
>
> On Mon, Jul 12, 2021 at 5:39 AM Euler Taveira wrote:
> >
> > On Fri, Jul 2, 2021, at 4:29 AM, Peter Smith wrote:
> >
> > Hi.
> >
&
On Mon, Jul 12, 2021 at 5:39 AM Euler Taveira wrote:
>
> On Fri, Jul 2, 2021, at 4:29 AM, Peter Smith wrote:
>
> Hi.
>
> I have been looking at the latest patch set (v16). Below are my review
> comments and some patches.
>
> Peter, thanks for your detailed review. Comme
On Sun, Jul 11, 2021 at 8:20 PM Amit Kapila wrote:
>
> On Fri, Jul 9, 2021 at 4:43 AM Peter Smith wrote:
> >
> > > The patch looks good to me, I don't have any comments.
> >
> > I tried the v95-0001 patch.
> >
> > - The patch a
mypub for all tables with ( "
"create publication mypub for table mytable " TAB --> "create
publication mypub for table mytable WITH ( "
--
[1] https://www.postgresql.org/docs/devel/sql-createpublication.html
Kind Regards,
Peter Smith
Fujitsu Australia
v1-0001-more-auto-complete-for-CREATE-PUBLICATION.patch
Description: Binary data
On Thu, Jul 8, 2021 at 10:08 PM vignesh C wrote:
>
> On Thu, Jul 8, 2021 at 11:37 AM Amit Kapila wrote:
> >
> > On Tue, Jul 6, 2021 at 9:58 AM Peter Smith wrote:
> > >
> > > Please find attached the latest patch set v93*
> > >
> >
> > Tha
t code either.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Wed, Jul 7, 2021 at 1:35 PM Bharath Rupireddy
wrote:
>
> On Wed, Jul 7, 2021 at 5:33 AM Peter Smith wrote:
> > PSA my patch which includes all the fixes mentioned above.
>
> I agree with Amit to start a separate thread to discuss these points.
> IMO, we can close thi
alue" */
errmsg("subscription with %s must also set %s",
"slot_name = NONE", "create_slot = false")));
This code can be simplified even more than the others mentioned,
because here the "specified_opts" checks were already done in the code
that precedes this.
It can be simplified like this:
- if (enabled && !*enabled_given && *enabled)
+ if (opts->enabled)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
/*- translator: both %s are strings of the form "option = value" */
errmsg("subscription with %s must also set %s",
"slot_name = NONE", "enabled = false")));
- if (create_slot && !create_slot_given && *create_slot)
+ if (opts->create_slot)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
+ /*- translator: both %s are strings of the form "option = value" */
errmsg("subscription with %s must also set %s",
"slot_name = NONE", "create_slot = false")));
//
PSA my patch which includes all the fixes mentioned above.
(Make check, and TAP subscription tests are tested and pass OK)
--
[1]
https://github.com/postgres/postgres/commit/8aafb02616753f5c6c90bbc567636b73c0cbb9d4
Kind Regards,
Peter Smith.
Fujitsu Australia
v11-0001-PS-Fix-v11.patch
Description: Binary data
t;option = value" */
errmsg("subscription with %s must also set %s",
"slot_name = NONE", "create_slot = false")));
This code can be simplified even more than the others mentioned,
because here the "specified_opts" checks were already done in the code
that precedes this.
It can be simplified like this:
- if (enabled && !*enabled_given && *enabled)
+ if (opts->enabled)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
/*- translator: both %s are strings of the form "option = value" */
errmsg("subscription with %s must also set %s",
"slot_name = NONE", "enabled = false")));
- if (create_slot && !create_slot_given && *create_slot)
+ if (opts->create_slot)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
+ /*- translator: both %s are strings of the form "option = value" */
errmsg("subscription with %s must also set %s",
"slot_name = NONE", "create_slot = false")));
//
PSA my patch which includes all the fixes mentioned above.
(Make check, and TAP subscription tests are tested and pass OK)
--
[1]
https://github.com/postgres/postgres/commit/8aafb02616753f5c6c90bbc567636b73c0cbb9d4
Kind Regards,
Peter Smith.
Fujitsu Australia
v11-0001-PS-Fix-v11.patch
Description: Binary data
Unless there are millions of rows the speedup may be barely noticeable.
[1]
https://www.postgresql.org/message-id/20210128022032.eq2qqc6zxkqn5syt%40alap3.anarazel.de
[2]
https://www.postgresql.org/message-id/CACawEhW_iMnY9XK2tEb1ig%2BA%2BgKeB4cxdJcxMsoCU0SaKPExxg%40mail.gmail.com
Ki
me it is passed at
AlterSubscription_refresh(sub, copy_data);
--
Kind Regards,
Peter Smith.
Fujitsu Australia.
can affect the ASRP
copy_data [link].
Thoughts?
-
[1] https://www.postgresql.org/docs/devel/sql-altersubscription.html
[2] https://www.postgresql.org/docs/devel/sql-alterpublication.html
[3] https://www.postgresql.org/docs/devel/sql-createpublication.html
Kind Regards,
Peter Smith.
Fujitsu Australia
one in worker.c.
Thanks for the recent rebase.
- The v15 patch applies OK (albeit with whitespace warning)
- make check is passing OK
- the new TAP tests 020_row_filter is passing OK.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
ge-id/CAA4eK1Kyx6U9yxC7OXoBD7pHC3bJ4LuNGd%3DOiABmiW6%2BqG%2BvEQ%40mail.gmail.com
[Amit-4]
https://www.postgresql.org/message-id/CAA4eK1Kyx6U9yxC7OXoBD7pHC3bJ4LuNGd%3DOiABmiW6%2BqG%2BvEQ%40mail.gmail.com
[Amit-5]
https://www.postgresql.org/message-id/CAA4eK1Kyx6U9yxC7OXoBD7pHC3bJ4LuNGd%3DOiABmiW6%2BqG%2BvEQ%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
AEST [18521] CONTEXT: COPY test_tab, line 1
2021-06-18 15:38:29.766 AEST [19924] LOG: background worker "logical
replication worker" (PID 18521) exited with exit code 1
etc...
-[ RECORD 1 ]-+
oid | 16399
subdbid | 16384
subname | tap_sub
subowner | 10
subenabled| t
disable_on_error | f
disabled_by_error | f
subbinary | f
substream | f
subconninfo | host=localhost dbname=test_pub application_name=tap_sub
subslotname | tap_sub
subsynccommit | off
suberrmsg |
subpublications | {tap_pub}
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Thu, Jun 17, 2021 at 6:22 PM Greg Nancarrow wrote:
>
> On Wed, Jun 16, 2021 at 9:08 AM Peter Smith wrote:
> >
> >
> > Please find attached the latest patch set v86*
> >
>
> A couple of comments:
>
> (1) I think one of my suggested changes was missed
On Thu, Jun 10, 2021 at 1:28 AM Bharath Rupireddy
wrote:
>
> On Wed, Jun 9, 2021 at 10:37 AM Peter Smith wrote:
> >
[...]
I checked the v4* patches.
Everything applies and builds and tests OK for me.
> > 2.
> > + /* If connect option is supported, the others also nee
On Wed, Jun 2, 2021 at 10:41 PM Bharath Rupireddy
wrote:
>
> On Wed, Jun 2, 2021 at 11:43 AM Peter Smith wrote:
> > Yes, it looks better, but (since the masks are all 1 bit) I was only
> > asking why not do like:
> >
> > if (supported_opts & SUBO
On Mon, May 10, 2021 at 11:42 PM Euler Taveira wrote:
>
> On Mon, May 10, 2021, at 5:19 AM, Peter Smith wrote:
>
> AFAIK this is the latest patch available, but FYI it no longer applies
> cleanly on HEAD.
>
> Peter, the last patch is broken since f3b141c48
On Thu, Jun 3, 2021 at 7:56 PM Amit Kapila wrote:
>
> On Wed, Jun 2, 2021 at 4:34 AM Peter Smith wrote:
> >
> > Please find attached the latest patch set v82*
> >
>
> Few comments on 0001:
>
> 1.
> + /*
> + * Beg
; Is that a deliberate functionality, or is it a quirk / bug?
>
> I don't think it's a bug
...
OK. Thanks to both of you for sharing your thoughts about it.
--
Kind Regards,
Peter Smith
Fujitsu Australia
f (supported_opts & SUBOPT_ENABLED)
if (supported_opts & SUBOPT_SLOT_NAME)
if (supported_opts & SUBOPT_COPY_DATA)
>
> Can we implement a similar idea for the parse_publication_options
> although there are only two options right now. Option parsing code
> will be consistent for logical replication DDLs and is extensible.
> Thoughts?
I have no strong opinion about it. It seems a trade off between having
a goal of "code consistency", versus "if it aint broke don't fix it".
--
Kind Regards,
Peter Smith.
Fujitsu Australia
it is just a style thing,
but since there are so many of them I felt it contributed to clutter
and made the code less readable. This pattern was in many places, not
just the example above.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
when doing the REFRESH
PUBLICATION.
Is that a deliberate functionality, or is it a quirk / bug?
--
[1] https://www.postgresql.org/docs/devel/sql-altersubscription.html
Kind Regards,
Peter Smith.
Fujitsu Australia
is function starts the
> transaction then the caller is responsible to commit it?
>
> 8.
> (errmsg("logical replication apply worker for subscription \"%s\" will
> restart so two_phase can be enabled",
> + MySubscription->name)));
>
> Can we slightl
Hi.
The attached PG docs patch about catalog deadlocks was previously
implemented in another thread [1], but it seems more relevant to this
one.
PSA.
--
[1]
https://www.postgresql.org/message-id/CAA4eK1K%2BSeT31pxwL5iTvXq%3DJhZpG_cUJLFhiz-eD%2BJr-WAPeg%40mail.gmail.com
Kind Regards,
Peter
On Fri, May 21, 2021 at 9:21 PM Peter Smith wrote:
>
> On Thu, May 20, 2021 at 2:11 PM Bharath Rupireddy
> wrote:
> >
> > On Wed, May 19, 2021 at 6:13 PM Bharath Rupireddy
> > wrote:
> > > Thanks. I will work on the new structure ParseSubOption only for
>
isadd ? _data : NULL;
The opts struct is already zapped 0/NULL so this code maybe should be:
if (isadd)
opts.copy_data = _data;
==
10.
Since the new typedef ParseSubOptions was added by this patch
shouldn't the src/tools/pgindent/typedefs.list file be updated also?
--
Kind Regards,
Peter Smith.
Fujitsu Australia.
On Tue, May 18, 2021 at 9:32 PM Amit Kapila wrote:
>
> On Thu, May 13, 2021 at 3:20 PM Peter Smith wrote:
> >
> > Please find attached the latest patch set v75*
> >
>
> Review comments for v75-0001-Add-support-for-pr
On Sun, May 16, 2021 at 12:07 AM Ajin Cherian wrote:
>
> On Thu, May 13, 2021 at 7:50 PM Peter Smith wrote:
> >
> > Please find attached the latest patch set v75*
> >
> > Differences from v74* are:
> >
> > * Rebased to HEAD @ today.
> >
> > *
On Fri, May 14, 2021 at 11:16 AM David Rowley wrote:
>
> On Fri, 14 May 2021 at 12:00, Peter Smith wrote:
> > That bug led me to wonder if similar problems might be going
> > undetected elsewhere in the code. There is a gcc compiler option [3]
> > -Wshadow which i
On Mon, May 17, 2021 at 6:47 PM Amit Kapila wrote:
>
> On Mon, May 17, 2021 at 12:30 PM Peter Smith wrote:
> >
[...]
> > The essence of the trouble seems to be that the apply_handle_truncate
> > function never anticipated it may end up truncating the same table
>
On Mon, May 17, 2021 at 8:13 PM Amit Kapila wrote:
>
> On Mon, May 17, 2021 at 3:05 PM Dilip Kumar wrote:
> >
> > On Mon, May 17, 2021 at 12:30 PM Peter Smith wrote:
> >
> > > PSA a patch adding a test for this scenario.
> >
> > I am not sure this t
) but the
ExecuteTruncate is using a more powerful AcccessExclusiveLock than the
apply_handle_truncate was using.
~~
PSA a patch to make the apply_handle_truncate use AccessExclusiveLock
same as the ExecuteTruncate function does.
PSA a patch adding a test for this scenario.
Kind Regards,
Peter Smith.
Fujitsu
On Mon, May 10, 2021 at 1:31 PM vignesh C wrote:
>
> On Thu, Apr 29, 2021 at 2:23 PM Peter Smith wrote:
> >
> > Please find attached the latest patch set v74*
> >
> > Differences from v73* are:
> >
> > * Rebased to HEAD @ 2 days ago.
> >
> &
On Wed, May 12, 2021 at 11:09 PM vignesh C wrote:
...
>
> Thanks for the comments. Attached v4 patch has the fix for the same.
>
I have not tried this patch so I cannot confirm whether it applies or
renders OK, but just going by the v4 content this now LGTM.
Kind Regards,
Pe
On Wed, May 12, 2021 at 5:52 PM Michael Paquier wrote:
>
> On Tue, May 11, 2021 at 04:50:46PM +0900, Michael Paquier wrote:
> > And that makes the code slightly easier to follow.
>
> Yeah, that's better this way, so applied.
Thanks!
------
Kind Regards,
Peter Smith.
Fujitsu Australia
quot; already means.
e.g. consider change to just say "Flags (uint8); currently unused."
--
Kind Regards,
Peter Smith.
Fujitsu Australia
sage. e.g. maybe you could say what C type the bytes
represent when they come off the wire at the other end - something
like below.
BEFORE
Int64
The final LSN of the transaction.
AFTER
Int64
The final LSN (XLogRecPtr) of the transaction
--
[1] https://www.postgresql.org/docs/devel/protocol-message-types.html
[2] https://linux.die.net/man/3/ntohl
Kind Regards,
Peter Smith.
Fujitsu Australia.
On Mon, May 10, 2021 at 6:09 PM Bharath Rupireddy
wrote:
>
> On Mon, May 10, 2021 at 12:36 PM Peter Smith wrote:
> >
> > The function GetSubscriptionRelations was declaring ScanKeyData
> > skey[2]; but actually
> > only uses 1 scan key. It seems like the code was c
plication.patch:518:
trailing whitespace.
error: patch failed: src/backend/parser/gram.y:426
error: src/backend/parser/gram.y: patch does not apply
error: patch failed: src/backend/replication/logical/worker.c:340
error: src/backend/replication/logical/worker.c: patch does not apply
Kind Regards,
Peter Smith.
Fujitsu Australia.
Regards,
Peter Smith.
Fujitsu Australia
v1-0001-GetSubscriptionRelations-declare-only-1-scan-key.patch
Description: Binary data
PSA v5 of the patch. It is the same as v4 but with the v4-0001 part
omitted because that was already pushed.
(reposted to keep cfbot happy).
--
Kind Regards,
Peter Smith
Fujitsu Australia
v5-0001-Rename-the-logical-replication-global-wrconn.patch
Description: Binary data
> +
>
Can you please provide more details so I can be sure of the context of
this feedback, e.g. there are multiple places that match that patch
fragment provided. So was this suggestion to change all of them ( 'b',
'P', 'K' , 'r' of patch 0001; and also 'p' of patch 0002) ?
--
Kind Regards,
Peter Smith.
Fujitsu Australia.
On Fri, May 7, 2021 at 6:09 PM Peter Smith wrote:
>
> On Fri, May 7, 2021 at 7:08 AM Tom Lane wrote:
> >
> > Alvaro Herrera writes:
> > > On 2021-May-06, Peter Smith wrote:
> > >> PSA v3 of the patch. Same as before, but now also renames
On Sun, May 9, 2021 at 11:13 PM Peter Smith wrote:
>
> On Sun, May 9, 2021 at 10:38 PM vignesh C wrote:
> >
> > Hi,
> >
> > For some of the logical replication messages the data type documented
> > was not correct, especially for lsn and xid. For ls
e
> used is uint32 but documented as int32.
> Attached is a patch which has the fix for the same.
> Thoughts?
If you want to do this then there are more - e.g. Flags should be
Uint8 instead of Int8.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Fri, May 7, 2021 at 7:08 AM Tom Lane wrote:
>
> Alvaro Herrera writes:
> > On 2021-May-06, Peter Smith wrote:
> >> PSA v3 of the patch. Same as before, but now also renames the global
> >> variable from "wrconn" to "lrep_worker_wrconn".
>
On Thu, May 6, 2021 at 7:18 PM Japin Li wrote:
>
>
> On Thu, 06 May 2021 at 17:08, Peter Smith wrote:
> > On Wed, May 5, 2021 at 12:35 PM Tom Lane wrote:
> >>
> >> Peter Smith writes:
> >> > This patch replaces the global "wrconn" in
On Wed, May 5, 2021 at 12:35 PM Tom Lane wrote:
>
> Peter Smith writes:
> > This patch replaces the global "wrconn" in AlterSubscription_refresh with a
> > local variable of the same name, making it consistent with other functions
> > in subscri
On Wed, May 5, 2021 at 3:20 PM Bharath Rupireddy
wrote:
>
> On Wed, May 5, 2021 at 8:05 AM Tom Lane wrote:
> >
> > Peter Smith writes:
> > > This patch replaces the global "wrconn" in AlterSubscription_refresh with
> > > a local variable of
PSA v2 of this patch - it has the same content, but an improved commit comment.
I have also added a commitfest entry, https://commitfest.postgresql.org/33/3109/
--
Kind Regards,
Peter Smith
Fujitsu Australia
v2-0001-Fix-wrconn.-Use-stack-variable.patch
Description: Binary data
On Tue, May 4, 2021 at 2:31 PM Andres Freund wrote:
>
> Hi,
>
> On 2021-05-04 09:29:42 +1000, Peter Smith wrote:
> > While reviewing some logical replication code I stumbled across a
> > variable usage that looks suspicious to me.
>
> > Note that the AlterSubs
1201 - 1300 of 1496 matches
Mail list logo
