Thread: extensible options syntax for replication parser?
Hi, It seems to me that we're making the same mistake with the replication parser that we've made in various placesin the regular parser: using a syntax for options that requires that every potential option be a keyword, and every potential option requires modification of the grammar. In particular, the syntax for the BASE_BACKUP command has accreted a whole lot of cruft already and I think that trend is likely to continue. I don't think that trying to keep people from adding options is a good strategy, so instead I'd like to have a better way to do it. Attached is v1 of a patch to refactor things so that parts of the BASE_BACKUP and CREATE_REPLICATION_SLOT are replaced with a flexible options syntax. There are some debatable decisions here, so I'd be happy to get some feedback on whether to go further with this, or less far, or maybe even just abandon the idea altogether. I doubt the last one is the right course, though: ISTM something's got to be done about the BASE_BACKUP case, at least. This version of the patch does not include documentation, but hopefully it's fairly clear from reading the code what it's all about. If people agree with the basic approach, I'll write docs. The intention is that we'd continue to support the existing syntax for the existing options, but the client tools would be adjusted to use the new syntax if the server's new enough, and any new options would be supported only through the new syntax. At some point in the distant future we could retire the old syntax, when we've stopped caring about compatibility with pre-14 versions. Thoughts? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
Hello Robert, My 0.02 €: > It seems to me that we're making the same mistake with the replication > parser that we've made in various placesin the regular parser: using a > syntax for options that requires that every potential option be a > keyword, and every potential option requires modification of the > grammar. In particular, the syntax for the BASE_BACKUP command has > accreted a whole lot of cruft already and I think that trend is likely > to continue. I don't think that trying to keep people from adding > options is a good strategy, Indeed. > so instead I'd like to have a better way to do it. > Attached is v1 of a patch to refactor things so that parts of the > BASE_BACKUP and CREATE_REPLICATION_SLOT are replaced with a flexible > options syntax. Patch applies cleanly, however compilation fails on: repl_gram.y:271:106: error: expected ‘;’ before ‘}’ Getting rid of "ident_or_keyword", some day, would be a relief. For boolean options, you are generating (EXPORT_SNAPSHOT TRUE) where (EXPORT_SNAPSHOT) would do. Maybe allowing (!EXPORT_SNAPSHOT) for (FOO FALSE) would be nice, if it is not allowed yet. That would also allow to get rid of FOO/NOFOO variants if any for FOO & !FOO, so one keyword is enough for a concept. > There are some debatable decisions here, so I'd be happy to get some > feedback on whether to go further with this, or less far, or maybe even > just abandon the idea altogether. I doubt the last one is the right > course, though: ISTM something's got to be done about the BASE_BACKUP > case, at least. ISTM that it would be better to generalize the approach to all commands which accept options, so that the syntax is homogeneous. > If people agree with the basic approach, I'll write docs. The > intention is that we'd continue to support the existing syntax for the > existing options, but the client tools would be adjusted to use the > new syntax if the server's new enough, and any new options would be > supported only through the new syntax. Yes. > At some point in the distant future we could retire the old syntax, when > we've stopped caring about compatibility with pre-14 versions. Just wondering: ISTM that the patch implies that dumping a v14 db generates the new syntax, which makes sense. Now I see 4 use cases wrt to version. # source target comment 1 v < 14 v < 14 probably the dump would use one of the older version 2 v < 14 v >= 14 upgrade 3 v >= 14 v < 14 downgrade: oops, the output uses the new syntax 4 v >= 14 v >= 14 ok Both cross version usages may be legitimate. In particular, 3 (oops, hardware issue, I have to move the db to a server where pg has not been upgraded) seems not possible because the generated syntax uses the new approach. Should/could there be some option to tell "please generate vXXX syntax" to allow that? -- Fabien.
On Sun, Jun 14, 2020 at 3:15 AM Fabien COELHO <coelho@cri.ensmp.fr> wrote: > > so instead I'd like to have a better way to do it. > > > Attached is v1 of a patch to refactor things so that parts of the > > BASE_BACKUP and CREATE_REPLICATION_SLOT are replaced with a flexible > > options syntax. > > Patch applies cleanly, however compilation fails on: > > repl_gram.y:271:106: error: expected ‘;’ before ‘}’ Oops. I'm surprised my compiler didn't complain. > Getting rid of "ident_or_keyword", some day, would be a relief. Actually, I think that particular thing is a sign that things are being done correctly. You need something like that if you have contexts where you want to treat keywords and non-keywords the same way, and it's generally good to have such places. In fact, this could probably be profitably used in more places in the replication grammar. > For boolean options, you are generating (EXPORT_SNAPSHOT TRUE) where > (EXPORT_SNAPSHOT) would do. True, but it doesn't seem to matter much one way or the other. I thought this way looked a little clearer. > Maybe allowing (!EXPORT_SNAPSHOT) for (FOO FALSE) would be nice, if it is > not allowed yet. That would also allow to get rid of FOO/NOFOO variants if > any for FOO & !FOO, so one keyword is enough for a concept. Well, the goal for this is not to need ANY new keywords for a new concept, but !FOO would be inconsistent with other places where we do similar things (e.g. EXPLAIN, VACUUM, COPY) so I don't think that's the way to go. > > There are some debatable decisions here, so I'd be happy to get some > > feedback on whether to go further with this, or less far, or maybe even > > just abandon the idea altogether. I doubt the last one is the right > > course, though: ISTM something's got to be done about the BASE_BACKUP > > case, at least. > > ISTM that it would be better to generalize the approach to all commands > which accept options, so that the syntax is homogeneous. As a general principle, sure, but it's always debatable how far to take things in particular cases. For instance, in the cases of EXPLAIN, VACUUM, and COPY, the relation name is given as a dedicated piece of syntax, not an option. It could be given as an option, but since it's mandatory and important, we didn't. In the case of COPY, the source file is also specified via dedicated syntax, rather than an option. So we have to make the same kinds of decisions here. For example, for CREATE_REPLICATION_SLOT, one could argue that PHYSICAL and LOGICAL should be moved to the extensible options list instead of being kept as separate syntax. However, that seems somewhat inconsistent with the long-standing syntax for START_REPLICATION, which already does use extensible options: START_REPLICATION SLOT slot_name LOGICAL XXX/XXX [ ( option_name [option_value] [, ... ] ) ] On balance I am comfortable with what the patch does, but other people might have a different take. > Just wondering: ISTM that the patch implies that dumping a v14 db > generates the new syntax, which makes sense. Now I see 4 use cases wrt to > version. > > # source target comment > 1 v < 14 v < 14 probably the dump would use one of the older version > 2 v < 14 v >= 14 upgrade > 3 v >= 14 v < 14 downgrade: oops, the output uses the new syntax > 4 v >= 14 v >= 14 ok > > Both cross version usages may be legitimate. In particular, 3 (oops, > hardware issue, I have to move the db to a server where pg has not been > upgraded) seems not possible because the generated syntax uses the new > approach. Should/could there be some option to tell "please generate vXXX > syntax" to allow that? I don't think dumping a DB is really affected by any of this. AFAIK, replication commands aren't used in pg_dump output. It just affects pg_basebackup and the server, and you'll notice that I have taken pains to allow the server to continue to accept the current format, and to allow pg_basebackup to generate that format when talking to an older server. Thanks for the review. v2 attached, hopefully fixing the compilation issue you mentioned. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
On Wed, Jun 24, 2020 at 11:51 AM Robert Haas <robertmhaas@gmail.com> wrote: > Thanks for the review. v2 attached, hopefully fixing the compilation > issue you mentioned. Tushar Ahuja reported to me off-list that my basebackup refactoring patch set was changing whether or not the following message appeared: NOTICE: WAL archiving is not enabled; you must ensure that all required WAL segments are copied through other means to complete the backup That patch set includes this patch, and the reason for the behavior difference turned out to be that I had gotten an if-test that is part of this patch backwards. Here is v3, fixing that. It is a little disappointing that this mistake didn't cause any existing regression tests to fail. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
On Thu, Oct 8, 2020 at 10:33 AM Robert Haas <robertmhaas@gmail.com> wrote:
> That patch set includes this patch, and the reason for the behavior
> difference turned out to be that I had gotten an if-test that is part
> of this patch backwards. Here is v3, fixing that. It is a little
> disappointing that this mistake didn't cause any existing regression
> tests to fail.
I'm returning to this topic ~11 months later with a more definite
intent to get something committed, since my "refactoring basebackup.c"
patch set - that also adds server-side compression and server-side
backup - needs to add more options to BASE_BACKUP, and doubling down
on the present options-parsing strategy seems like a horrible idea.
I've now split this into two patches, one for BASE_BACKUP, and the
other for CREATE_REPLICATION_SLOT. I've rebased the patches and added
documentation as well. The CREATE_REPLICATION_SLOT patch now unifies
EXPORT_SNAPSHOT, NOEXPORT_SNAPSHOT, and USE_SNAPSHOT, which are
mutually exclusive choices, into SNAPSHOT { 'export' | 'use' |
'nothing' } which IMHO is clearer.
Last call for complaints about either the overall direction or the
specific implementation choices...
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachment
On Fri, Sep 10, 2021 at 3:44 PM Robert Haas <robertmhaas@gmail.com> wrote: > Last call for complaints about either the overall direction or the > specific implementation choices... A complaint showed up over at http://postgr.es/m/979131631633278@mail.yandex.ru and pursuant to that complaint I have made the new syntax for controlling the checkpoint type look like CHECKPOINT { 'fast' | 'spread' } rather than just having an option called FAST. It was suggested over there to also rename WAIT to WAIT_WAL_ARCHIVED, but I don't like that for reasons explained on that thread and so have not adopted that proposal. Sergei also helpfully pointed out that I'd accidentally deleted a version check in one place, so this version is also updated to not do that. -- Robert Haas EDB: http://www.enterprisedb.com
Attachment
Hello
Thanks, I missed this thread.
> + <term><literal>CHECKPOINT { 'fast' | 'spread' }</replaceable></literal></term>
Unpaired </replaceable> tag in docs.
That was all I noticed in 0001. Still not sure where is the difference between "change NOWAIT to WAIT" and "change
NOWAITto something else descriptive". But fine, I can live with WAIT. (one note: the exact command is visible to the
userwhen log_replication_commands is enabled, not completely hidden)
0002 breaks "create subscription (with create_slot true)" when the publish server is an earlier version:
postgres=# create subscription test CONNECTION 'host=127.0.0.1 user=postgres' PUBLICATION test with (create_slot =
true);
ERROR: could not create replication slot "test": ERROR: syntax error
regards, Sergei
On Wed, Sep 22, 2021 at 8:11 AM Sergei Kornilov <sk@zsrv.org> wrote:
> > + <term><literal>CHECKPOINT { 'fast' | 'spread' }</replaceable></literal></term>
>
> Unpaired </replaceable> tag in docs.
>
> That was all I noticed in 0001. Still not sure where is the difference between "change NOWAIT to WAIT" and "change
NOWAITto something else descriptive". But fine, I can live with WAIT. (one note: the exact command is visible to the
userwhen log_replication_commands is enabled, not completely hidden)
>
> 0002 breaks "create subscription (with create_slot true)" when the publish server is an earlier version:
>
> postgres=# create subscription test CONNECTION 'host=127.0.0.1 user=postgres' PUBLICATION test with (create_slot =
true);
> ERROR: could not create replication slot "test": ERROR: syntax error
Thanks. I have attempted to fix these problems in the attached version.
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachment
On 9/23/21 1:25 AM, Robert Haas wrote:
postgres=# create subscription test CONNECTION 'host=127.0.0.1 user=postgres' PUBLICATION test with (create_slot = true); ERROR: could not create replication slot "test": ERROR: syntax errorThanks. I have attempted to fix these problems in the attached version.
l checked and look like the issue is still not fixed against v7-* patches -
postgres=# create subscription test CONNECTION 'host=127.0.0.1 user=edb dbname=postgres' PUBLICATION p with (create_slot = true);
ERROR: could not create replication slot "test": ERROR: syntax error
-- regards,tushar EnterpriseDB https://www.enterprisedb.com/ The Enterprise PostgreSQL Company
On Thu, Sep 23, 2021 at 2:55 AM tushar <tushar.ahuja@enterprisedb.com> wrote: > l checked and look like the issue is still not fixed against v7-* patches - > > postgres=# create subscription test CONNECTION 'host=127.0.0.1 user=edb dbname=postgres' PUBLICATION p with (create_slot= true); > ERROR: could not create replication slot "test": ERROR: syntax error Thanks. Looks like that version had some stupid mistakes. Here's a new one. -- Robert Haas EDB: http://www.enterprisedb.com
Attachment
On 2021/09/24 0:05, Robert Haas wrote:
> On Thu, Sep 23, 2021 at 2:55 AM tushar <tushar.ahuja@enterprisedb.com> wrote:
>> l checked and look like the issue is still not fixed against v7-* patches -
>>
>> postgres=# create subscription test CONNECTION 'host=127.0.0.1 user=edb dbname=postgres' PUBLICATION p with
(create_slot= true);
>> ERROR: could not create replication slot "test": ERROR: syntax error
>
> Thanks. Looks like that version had some stupid mistakes. Here's a new one.
- <indexterm><primary>BASE_BACKUP</primary></indexterm>
+ <term><literal>BASE_BACKUP</literal> [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ]
You seem to accidentally remove the index term for BASE_BACKUP.
+ident_or_keyword:
+ IDENT { $$ = $1; }
ident_or_keyword seems to be used only for generic options,
but it includes the keywords for legacy options like "FAST".
Isn't it better to remove the keywords for legacy options from
ident_or_keyword?
OTOH, the keywords for newly-introduced generic options like
CHECKPOINT should be defined in repl_scanner.l and repl_gram.y?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
On 9/23/21 8:35 PM, Robert Haas wrote: > Thanks. Looks like that version had some stupid mistakes. Here's a new one. Thanks, the reported issue seems to be fixed now for HEAD w/patch (publication) to HEAD w/patch (subscription) but still getting the same error if we try to perform v12(publication) to HEAD w/patch(subscription) . I checked there is no such issue for v12(publication) to v14 RC1 (subscription) postgres=# create subscription sub123s CONNECTION 'host=127.0.0.1 user=edb port=4444 dbname=postgres' PUBLICATION pp with (slot_name = from_v14); ERROR: could not create replication slot "from_v14": ERROR: syntax error postgres=# -- regards,tushar EnterpriseDB https://www.enterprisedb.com/ The Enterprise PostgreSQL Company
On Fri, Sep 24, 2021 at 12:01 AM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:
> You seem to accidentally remove the index term for BASE_BACKUP.
Good catch.
> +ident_or_keyword:
> + IDENT { $$ = $1; }
>
> ident_or_keyword seems to be used only for generic options,
> but it includes the keywords for legacy options like "FAST".
> Isn't it better to remove the keywords for legacy options from
> ident_or_keyword?
I don't think so. I mean, if we do, then it's not really an
ident_or_keyword production any more, because it would only allow some
keywords, not all. Now if the keywords that are not included aren't
used as options anywhere then it won't matter, but it seems cleaner to
me to make the list complete.
> OTOH, the keywords for newly-introduced generic options like
> CHECKPOINT should be defined in repl_scanner.l and repl_gram.y?
One big advantage of this approach is that we don't need to make
changes to those files in order to add new options, so I feel like
that would be missing the point completely.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Fri, Sep 24, 2021 at 7:28 AM tushar <tushar.ahuja@enterprisedb.com> wrote:
> On 9/23/21 8:35 PM, Robert Haas wrote:
> > Thanks. Looks like that version had some stupid mistakes. Here's a new one.
>
> Thanks, the reported issue seems to be fixed now for HEAD w/patch
> (publication) to HEAD w/patch (subscription) but still getting the same
> error if we try to perform v12(publication) to HEAD
> w/patch(subscription) . I checked there is no such issue for
> v12(publication) to v14 RC1 (subscription)
>
> postgres=# create subscription sub123s CONNECTION 'host=127.0.0.1
> user=edb port=4444 dbname=postgres' PUBLICATION pp with (slot_name =
> from_v14);
> ERROR: could not create replication slot "from_v14": ERROR: syntax error
> postgres=#
I am not able to reproduce this failure. I suspect you made a mistake
in testing, because my test case before sending the patch was
basically the same as yours, except that I was testing with v13. But I
tried again with v12 and it seems fine:
[rhaas pgsql]$ createdb -p 5412
[rhaas pgsql]$ psql -c 'select version()' -p 5412
version
----------------------------------------------------------------------------------------------------------------
PostgreSQL 12.3 on x86_64-apple-darwin19.4.0, compiled by clang
version 5.0.2 (tags/RELEASE_502/final), 64-bit
(1 row)
[rhaas pgsql]$ psql
psql (15devel)
Type "help" for help.
rhaas=# create subscription sub123s CONNECTION 'port=5412' PUBLICATION
pp with (slot_name =
from_v14);
NOTICE: created replication slot "from_v14" on publisher
CREATE SUBSCRIPTION
Here's v9, fixing the issue reported by Fujii Masao.
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachment
On 9/24/21 10:36 PM, Robert Haas wrote: > I am not able to reproduce this failure. I suspect you made a mistake > in testing, because my test case before sending the patch was > basically the same as yours, except that I was testing with v13. But I > tried again with v12 and it seems fine: Right, on a clean setup -I am not also not able to reproduce this issue. Thanks for checking at your end. -- regards,tushar EnterpriseDB https://www.enterprisedb.com/ The Enterprise PostgreSQL Company
On 9/24/21 10:36 PM, Robert Haas wrote: > Here's v9, fixing the issue reported by Fujii Masao. Please refer this scenario where publication on v14RC1 and subscription on HEAD (w/patch) --create a subscription with parameter two_phase=1 on HEAD postgres=# CREATE SUBSCRIPTION r1015 CONNECTION 'dbname=postgres host=localhost port=5454' PUBLICATION p WITH (two_phase=1); NOTICE: created replication slot "r1015" on publisher CREATE SUBSCRIPTION postgres=# --check on 14RC1 postgres=# select two_phase from pg_replication_slots where slot_name='r105'; two_phase ----------- f (1 row) so are we silently ignoring this parameter as it is not supported on v14RC/HEAD ? and if yes then why not we just throw an message like ERROR: unrecognized subscription parameter: "two_phase" -- regards,tushar EnterpriseDB https://www.enterprisedb.com/ The Enterprise PostgreSQL Company
On 9/24/21 11:57 PM, tushar wrote: > postgres=# select two_phase from pg_replication_slots where > slot_name='r105'; Correction -Please read 'r105' to 'r1015' -- regards,tushar EnterpriseDB https://www.enterprisedb.com/ The Enterprise PostgreSQL Company
On Fri, Sep 24, 2021 at 2:28 PM tushar <tushar.ahuja@enterprisedb.com> wrote: > Please refer this scenario where publication on v14RC1 and subscription > on HEAD (w/patch) > > --create a subscription with parameter two_phase=1 on HEAD > > postgres=# CREATE SUBSCRIPTION r1015 CONNECTION 'dbname=postgres > host=localhost port=5454' PUBLICATION p WITH (two_phase=1); > NOTICE: created replication slot "r1015" on publisher > CREATE SUBSCRIPTION > postgres=# > > --check on 14RC1 > > postgres=# select two_phase from pg_replication_slots where > slot_name='r105'; > two_phase > ----------- > f > (1 row) > > so are we silently ignoring this parameter as it is not supported on > v14RC/HEAD ? and if yes then why not we just throw an message like > ERROR: unrecognized subscription parameter: "two_phase" two_phase is new in v15, something you could also find out by checking the documentation. Now if the patch changes the way two_phase interacts with older versions, that's a bug in the patch and we should fix it. But if the same issue exists without the patch then I'm not sure why you are raising it here rather than on the thread where that feature was developed. -- Robert Haas EDB: http://www.enterprisedb.com
On Sat, Sep 25, 2021 at 4:28 AM tushar <tushar.ahuja@enterprisedb.com> wrote: > > On 9/24/21 10:36 PM, Robert Haas wrote: > > Here's v9, fixing the issue reported by Fujii Masao. > > Please refer this scenario where publication on v14RC1 and subscription > on HEAD (w/patch) > > --create a subscription with parameter two_phase=1 on HEAD > > postgres=# CREATE SUBSCRIPTION r1015 CONNECTION 'dbname=postgres > host=localhost port=5454' PUBLICATION p WITH (two_phase=1); > NOTICE: created replication slot "r1015" on publisher > CREATE SUBSCRIPTION > postgres=# > > --check on 14RC1 > > postgres=# select two_phase from pg_replication_slots where > slot_name='r105'; > two_phase > ----------- > f > (1 row) > > so are we silently ignoring this parameter as it is not supported on > v14RC/HEAD ? and if yes then why not we just throw an message like > ERROR: unrecognized subscription parameter: "two_phase" > > -- There is usually a time lag between a subscription created with two_phase on and the slot on the publisher enabling two_phase. It only happens after a tablesync is completed and the apply worker is restarted. There are logs which indicate that this has happened. If you could share the logs (on publisher and subscriber) when this happens, I could have a look. regards, Ajin Cherian Fujitsu Australia
On Mon, Sep 27, 2021 at 11:20 AM Ajin Cherian <itsajin@gmail.com> wrote: > > On Sat, Sep 25, 2021 at 4:28 AM tushar <tushar.ahuja@enterprisedb.com> wrote: > > > > On 9/24/21 10:36 PM, Robert Haas wrote: > > > Here's v9, fixing the issue reported by Fujii Masao. > > > > Please refer this scenario where publication on v14RC1 and subscription > > on HEAD (w/patch) > > > > --create a subscription with parameter two_phase=1 on HEAD > > > > postgres=# CREATE SUBSCRIPTION r1015 CONNECTION 'dbname=postgres > > host=localhost port=5454' PUBLICATION p WITH (two_phase=1); > > NOTICE: created replication slot "r1015" on publisher > > CREATE SUBSCRIPTION > > postgres=# > > > > --check on 14RC1 > > > > postgres=# select two_phase from pg_replication_slots where > > slot_name='r105'; > > two_phase > > ----------- > > f > > (1 row) > > > > so are we silently ignoring this parameter as it is not supported on > > v14RC/HEAD ? and if yes then why not we just throw an message like > > ERROR: unrecognized subscription parameter: "two_phase" > > > > -- > > There is usually a time lag between a subscription created with two_phase on and > the slot on the publisher enabling two_phase. It only happens after a > tablesync is completed and > the apply worker is restarted. There are logs which indicate that this > has happened. If you could share the > logs (on publisher and subscriber) when this happens, I could have a look. > And in case you do see a problem, I request you create a seperate thread for this. I didn't want to derail this patch. regards, Ajin Cherian Fujitsu Australia
On 9/25/21 12:08 AM, Robert Haas wrote: > two_phase is new in v15, something you could also find out by checking > the documentation. Now if the patch changes the way two_phase > interacts with older versions, that's a bug in the patch and we should > fix it. But if the same issue exists without the patch then I'm not > sure why you are raising it here rather than on the thread where that > feature was developed. Right, issue is reproducible on HEAD as well. I should have checked that, sorry about it. -- regards,tushar EnterpriseDB https://www.enterprisedb.com/ The Enterprise PostgreSQL Company
On 9/27/21 9:29 AM, Ajin Cherian wrote: > And in case you do see a problem, I request you create a seperate > thread for this. I didn't want to derail this patch. It would be great if we throw an error rather than silently ignoring this parameter , I opened a separate email for this https://www.postgresql.org/message-id/CAC6VRoY3SAFeO7kZ0EOVC6mX%3D1ZyTocaecTDTh209W20KCC_aQ%40mail.gmail.com -- regards,tushar EnterpriseDB https://www.enterprisedb.com/ The Enterprise PostgreSQL Company
On Mon, Sep 27, 2021 at 3:15 AM tushar <tushar.ahuja@enterprisedb.com> wrote: > It would be great if we throw an error rather than silently ignoring > this parameter , I opened a separate email for this > > https://www.postgresql.org/message-id/CAC6VRoY3SAFeO7kZ0EOVC6mX%3D1ZyTocaecTDTh209W20KCC_aQ%40mail.gmail.com Hearing no further comments, I've gone ahead and committed these patches. I'm still slightly nervous that I may have missed some issue, but I think at this point having the patches in the tree is more likely to turn it up than any other course of action. -- Robert Haas EDB: http://www.enterprisedb.com
On 10/5/21 10:26 PM, Robert Haas wrote: > Hearing no further comments, I've gone ahead and committed these > patches. I'm still slightly nervous that I may have missed some issue, > but I think at this point having the patches in the tree is more > likely to turn it up than any other course of action. I have tested couple of scenarios of pg_basebackup / pg_receivewal /pg_recvlogical / Publication(wal_level=logical) and Subscription e.t.c against HEAD (with patches) and cross-version testing. Things look good to me and no breakage was found. -- regards,tushar EnterpriseDB https://www.enterprisedb.com/ The Enterprise PostgreSQL Company