From: Michael Paquier <michael@paquier.xyz>
To: Peter Smith <smithpb2250@gmail.com>
Cc: Julien Rouhaud <rjuju123@gmail.com>
Cc: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Subject: Re: pg_upgrade and logical replication
Date: Wed, 19 Jul 2023 16:17:32 +0900
Message-ID: <ZLeODIhuoClIQTPR@paquier.xyz> (raw)
In-Reply-To: <CAHut+PvYt0yVJ+eAg0AdTHyTmgoZVY9znk3uxe98jOytmeCwLA@mail.gmail.com>
References: <CAA4eK1+4ORS7C+j3NS9yfoKdjfpdat0XrhUy1opBvw8WN58g4Q@mail.gmail.com>
<CAOBaU_YavOL0AhS0uU07Ea_0ZyR7youtJAvSHDnhmADGDw+VwA@mail.gmail.com>
<CAA4eK1+pWXqDSO+QpGtLFqcCP5+qCXkvLN-YviRqdXV+d7Fdow@mail.gmail.com>
<20230309083456.xpjcsuycyn7d2yvr@jrouhaud>
<20230327084955.m624nwfaf7mbzo6j@jrouhaud>
<TYAPR01MB58667340763002108CEB5EB8F5919@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<20230407022802.jqmdhgjhrnj5lpcl@jrouhaud>
<TYAPR01MB586682CF17623F6E71B9D3ADF59D9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<20230424072224.qd3awkfrjcanw2pa@jrouhaud>
<CAHut+PvYt0yVJ+eAg0AdTHyTmgoZVY9znk3uxe98jOytmeCwLA@mail.gmail.com>
On Wed, May 10, 2023 at 05:59:24PM +1000, Peter Smith wrote:
> 1. ALTER SUBSCRIPTION name ADD TABLE (relid = XYZ, state = 'x' [, lsn = 'X/Y'])
>
> I was a bit confused by this relation 'state' mentioned in multiple
> places. IIUC the pg_upgrade logic is going to reject anything with a
> non-READY (not 'r') state anyhow, so what is the point of having all
> the extra grammar/parse_subscription_options etc to handle setting the
> state when only possible value must be 'r'?
We are just talking about the handling of an extra DefElem in an
extensible grammar pattern, so adding the state field does not
represent much maintenance work. I'm OK with the addition of this
field in the data set dumped, FWIW, on the ground that it can be
useful for debugging purposes when looking at --binary-upgrade dumps,
and because we aim at copying catalog contents from one cluster to
another.
Anyway, I am not convinced that we have any need for a parse-able
grammar at all, because anything that's presented on this thread is
aimed at being used only for the internal purpose of an upgrade in a
--binary-upgrade dump with a direct catalog copy in mind, and having a
grammar would encourage abuses of it outside of this context. I think
that we should aim for simpler than what's proposed by the patch,
actually, with either a single SQL function à-la-binary_upgrade() that
adds the contents of a relation. Or we can be crazier and just create
INSERT queries for pg_subscription_rel to provide an exact copy of the
catalog contents. A SQL function would be more consistent with other
objects types that use similar tricks, see
binary_upgrade_create_empty_extension() that does something similar
for some pg_extension records. So, this function would require in
input 4 arguments:
- The subscription name or OID.
- The relation OID.
- Its LSN.
- Its sync state.
> 2. state V relstate
>
> I still feel code readbility suffers a bit by calling some fields/vars
> a generic 'state' instead of the more descriptive 'relstate'. Maybe
> it's just me.
>
> Previously commented same (see [1]#3, #4, #5)
Agreed to be more careful with the naming here.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: michael@paquier.xyz, smithpb2250@gmail.com, rjuju123@gmail.com, kuroda.hayato@fujitsu.com, pgsql-hackers@lists.postgresql.org, amit.kapila16@gmail.com
Subject: Re: pg_upgrade and logical replication
In-Reply-To: <ZLeODIhuoClIQTPR@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox