From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Julien Rouhaud' <rjuju123@gmail.com>
CC: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Amit Kapila
	<amit.kapila16@gmail.com>
Subject: RE: pg_upgrade and logical replication
Thread-Topic: pg_upgrade and logical replication
Thread-Index: 
 AQHZQqUkO5/8autRm0eitS78AwCVZq7S8+MAgABRnoCAANCRAIAAHqEAgABRgwCAAN8JgIAB8F0AgAAZIICAAx3eAIAEqU2AgAFjHgCAAgjLAIABENUAgAAQ/oCAABbdgIABrGuAgAAJZoCAAcrkgIAACTwAgALXMICABlU6AIABjJEAgAAhVwCAHE4ngIAPcLtggAFuNwCACFY4wA==
Date: Wed, 12 Apr 2023 09:48:15 +0000
Message-ID: 
 <TYAPR01MB58667233ECDB40B7FFDDB22CF59B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
References: 
 <CAA4eK1+MnYn4OGzobTycbvtVNpwvbdxCa9ykY4j9YF-N_jW2Mw@mail.gmail.com>
 <20230301065527.gk7pdtz5kcjy7rc5@jrouhaud>
 <CAA4eK1Jbqd28Z-KXTV6OHd_mpLLFECRAqmdqT9Sy4R6mqg6eTA@mail.gmail.com>
 <20230302105056.cgu2lb5pryazdnxm@jrouhaud>
 <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>
In-Reply-To: <20230407022802.jqmdhgjhrnj5lpcl@jrouhaud>
Accept-Language: ja-JP, en-US
Content-Language: ja-JP
msip_labels: MSIP_Label_a7295cc1-d279-42ac-ab4d-3b0f4fece050_Enabled=true;
 MSIP_Label_a7295cc1-d279-42ac-ab4d-3b0f4fece050_SetDate=2023-04-12T09:48:13Z;
 MSIP_Label_a7295cc1-d279-42ac-ab4d-3b0f4fece050_Method=Standard;
 MSIP_Label_a7295cc1-d279-42ac-ab4d-3b0f4fece050_Name=FUJITSU-RESTRICTED?;
 MSIP_Label_a7295cc1-d279-42ac-ab4d-3b0f4fece050_SiteId=a19f121d-81e1-4858-a9d8-736e267fd4c7;
 MSIP_Label_a7295cc1-d279-42ac-ab4d-3b0f4fece050_ActionId=f891a6fa-39df-43bc-b0da-9542c91afaf0;
 MSIP_Label_a7295cc1-d279-42ac-ab4d-3b0f4fece050_ContentBits=0
authentication-results: dkim=none (message not signed)
 header.d=none;dmarc=none action=none header.from=fujitsu.com;
x-ms-publictraffictype: Email
x-ms-traffictypediagnostic: TYAPR01MB5866:EE_|OS7PR01MB11648:EE_
x-ms-office365-filtering-correlation-id: 7f9541f1-988a-4b19-a640-08db3b3b0981
x-ms-exchange-senderadcheck: 1
x-ms-exchange-antispam-relay: 0
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: 
 3hPrKqFzofjQpa0D47EQIMup2I0XsVQ4f8KxgZ6ArahuLdxoBJAOeYOrKV4XO4OJswaoMG4BDatK9BrUX1hvdeO8u0DsgGk/wg3bC4oPB6LAJj4jfMsSAl6z4k8lFlPlBr/eJ5suSjAKchyFkGfQ28s+J9w0ZdmUi7OfAD3GLjTghSCQAQGRCueG3z7ykaQ5wQjZDHnC/DLr4Dsm+gZ/lg/EgpN54YbPwcp+iSbPLYJpUv1bK0LiIyb6Zqygu5mxFvDhAOwu3UlwOL+wa3p7xIvSDJ2gJS318O0Ka8aAC/WPdWO6l6ik3sRDi5qtm1DgHFvkSgjWeZravwF4gLWWmw2Gl/FFp9ldL/fLbcdY9qeA1+R8N1cS+rOCb0zxFsJLXHCx6nZ0BTlM/hYv3Y8osTfuuJe74oKyHKeMsfkeZ6r7ZejRb0L6ssXmkwGoOEzDbkXyiDULCDjNAMmq31+zaBl+1HOnbHxygt8MMEVdEI0ZsEke9wXrkBw8Jz5hr7pnZA6VCiAUL3BzjGJfAMF94iJX9Hi21zdFnRrxXufpmBTpmkmpsEpYbFw4y+fMg5V39d0Tva3QmFFmJeM8pvgoJLinS4HZqL40R8WB4wabrp63hIaOhLXpRfjc0v9pglg5kmBoW9VgjIxjduu8dqT+Iw==
x-forefront-antispam-report: 
 CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:TYAPR01MB5866.jpnprd01.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(4636009)(376002)(346002)(39860400002)(366004)(396003)(136003)(1590799018)(451199021)(38070700005)(5660300002)(1580799015)(52536014)(8936002)(9686003)(26005)(6506007)(55016003)(2906002)(83380400001)(186003)(85182001)(122000001)(33656002)(82960400001)(38100700002)(86362001)(478600001)(76116006)(41300700001)(7696005)(66556008)(6916009)(66946007)(4326008)(8676002)(64756008)(66476007)(66446008)(316002)(71200400001)(54906003);DIR:OUT;SFP:1101;
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?iso-2022-jp?B?dGZKRGhkNXZRSTZCTzFwMFlGKzkxMXh0U2s5YUxZMWJ6WDNKK1JRN2Fm?=
 =?iso-2022-jp?B?MUh5QUNsQ2VRaERiV3MwNEJxeEljV3prYjhsRkZXQTFxcFk4dmVQdWhl?=
 =?iso-2022-jp?B?bDZFaHkxTmVvMXhaWXhjTkJ5RzhuZGtmbndMcGhIOVNwL3lEeHFabEhv?=
 =?iso-2022-jp?B?UmZuTUoycEtJMVlGYmllR0pIYUpDRVRYTE16cWZRWHBYMGd2N253MWUr?=
 =?iso-2022-jp?B?U2hvcUsxaHpxbitHTnpxTlVaV1R4QXBEcXgyci92aFR6ZEdlSUEyQUhF?=
 =?iso-2022-jp?B?Y1Zsc2o2M1VrMzFFNHIwVXkyVWZkWXBQclp0N2J0QVg2c2h3T1ZnalZX?=
 =?iso-2022-jp?B?WThmY0VDYmxadTZYZ09PcTZQaFczN09EM3owTlYrNnI1cGY4S1pxRmtV?=
 =?iso-2022-jp?B?UDVKSkhaNzdHUERFZzdKU2pJZlF6Rk5CZGdxdzNzVHBSYmVtSUc0NkxD?=
 =?iso-2022-jp?B?MVVYS3YzdFRaWmVzdklGeHI0TDI2WGYzcVNPM0RkRnFEWi9PU0phbUZH?=
 =?iso-2022-jp?B?bHJGVWJqbXJlQXlRTkdkaDIyM2hRYUFJY08zckJKU0VYOHdwWkxHcXNP?=
 =?iso-2022-jp?B?bnkrbWw4T0g4NTJnOENRUHplN0NFUUdiaWpIZnloM2k0MDNHSTFnZVVQ?=
 =?iso-2022-jp?B?TGpVTUVwNG1jYmFwRTBLcWVEQWQwVkJxVDhJS2gvNzdoTVA0cW83bkE4?=
 =?iso-2022-jp?B?U2xpM0gycnhDWGhWSnROT3lENE1GMFRyZGN0bHl6Ymdxd1gxdk1LTnky?=
 =?iso-2022-jp?B?VXJERTVEY1hnN2swTVdCaG5aQm1iaVoyNkxsZGlEWUVjcWlqWEpodU5a?=
 =?iso-2022-jp?B?TlVLbW53SjJGN3hkQ2UyenJnUGtqYWFuNnlmSGx1OG1WOVN4bjBOQzI3?=
 =?iso-2022-jp?B?Vk9LSjcvYUZ5L1psNm1oZVdISFpHemVnUVNEcXRZaTJFWnZTRW5XUWIy?=
 =?iso-2022-jp?B?RlZzNW9RalBkbkM3UFkxNzJYdHdyRFA5L1ZMMG1jVjNTSjZvL1Q0TWN1?=
 =?iso-2022-jp?B?bE5iVGh0eEhEKzQ3U0JPVkFSVUJ4L0tWWDZmTlo5ZjZGS1kyWHlGR0JD?=
 =?iso-2022-jp?B?bXEyUitpTDVRU0NncC8xRmV3QkVEMGxCYjJUYjBaU3Z3MDdCbUZIRjE5?=
 =?iso-2022-jp?B?dTNHdFhNQWt4TTVwcGs2ZmZOY3prMkFOZWVtUkdNMGp6R1E1V1VvRHJj?=
 =?iso-2022-jp?B?MUJaTnNYdDMySkd6cnQ0c0hYQWRsUGYySTZLVFVVbWV3dFhla0pTdDRL?=
 =?iso-2022-jp?B?Z3N5UlMvKzM0L2RWVnVkTEQvTDBaNkRlSXQvK0NmMUo3VWdFeFpBc0h6?=
 =?iso-2022-jp?B?cms2NzUxa3hEeElmc1FoREFKS3hIdzZKMHpwZlppMUQxRlNrSnAxdHFN?=
 =?iso-2022-jp?B?M3EwM0cxWk5WeDNQQWg3YWRyYUhKbkFoMk5DRjlqWW9MZkpCelViRlFE?=
 =?iso-2022-jp?B?Q3AzWmI5SlZacWtZRlhLd2R3Sk9iUWRlSXVSYU52WHdrNENCU25HaUw4?=
 =?iso-2022-jp?B?WmNiNTBqRGdPN2xiMGZnWE5zbXN4akJpMkloTFlzc0VRMnpHelcwVnp1?=
 =?iso-2022-jp?B?bGV1dWxJNUE1UHZ5MGQ2UnRITFVLOW9CemtnbEtreWZ3V09ObEUrUE5k?=
 =?iso-2022-jp?B?bFd5T0JTZlB0SkZBQUx6NUxJODhmeE80ODFYMXpWOU9aajVWMm94d1FW?=
 =?iso-2022-jp?B?d3l4dmNkbFZvSlFMdDZoOXNvbm1sUUVOVDdtTUhwelBOSmxPUkF3WHR4?=
 =?iso-2022-jp?B?dU41WmdTam9nK3hWY2NEaVlBN0dCSG9BYk0vdTQ3aytzaFB5ek50K2Ux?=
 =?iso-2022-jp?B?emxVdDJNbkQ0OEVsUHAvSDkwN3FsYkl0TktrZTF1YWlvYnpoNHZEZDQx?=
 =?iso-2022-jp?B?U1JrcmJxQVh1eE5mUEpmN2VNZENuckVTbjRnSUdoNzNoSktoZHZWRDB3?=
 =?iso-2022-jp?B?T3JNM2hJa0VBaFdLRmV6SFhxVldrRjFGNlh4bnh2cEFUYzMzQ0lyT05P?=
 =?iso-2022-jp?B?YjVkbGtUNGpTUXlmaWdvNXNaTkMxamFMVW5Edlk3Qk9zdlN3WG5jU055?=
 =?iso-2022-jp?B?Z0kwNnFSUm10MWlEUzN6T3kwanlERjZ3WU5Tc3EzVkJ0anc1eFh1Y2g5?=
 =?iso-2022-jp?B?c05SWXZ1ZmJLVk9sRUtPQVk4bTBKbDFCeTBGN1ZDa2RMRllvaGdzZmtp?=
 =?iso-2022-jp?B?Rk1pclNid2RZRXdGRENaODFFaHNBSUVZWWtiOWJLN3NLRE1PTkQyL3B2?=
 =?iso-2022-jp?B?d2hCbTczNzJHVnl2bnBiTHptVGlpVjNOc29PYUZHYmRwaWw2RFJFU1Bw?=
 =?iso-2022-jp?B?NjBESk1OSWdrOHFnTDRFYzMyTmc4ZVhlbUE9PQ==?=
Content-Type: text/plain; charset="iso-2022-jp"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	wpMtrovSr/eaniJRtnLeXES8kOaxNCF9K7iBk3HbqliJxy75d44zpTLo/UA8nt7EXGz525F0ryehoFT4S+uJ2G05Hf7+re873PklGcsudOQYblrG8JTgly3sYxWJiS10QOnTd82MR5jTR4W5PnazxAU/CjLhSXmdo+i7zaJ291mDXTGATClAl+FGOpyGEVhBeUz6BKhqtVAERgocGyeqzUuYesv4WFyx6RKUd0h+7pJAZtkAeQhass+iTvRJfFBZLRabbOqbSr8kGhuuYiYslKJcgaDFFZ/k+QRiJ1x59kwRz8YOMXKq2NIxLutBM8djPpU9Yc4uwzIHAu9zrCq27Nd9aI3/iGQj9qZUvxLs/l7AHDUY/yZe55QVbodrYxONMBQhmr0ckIL67mJfnqu4hf+bbob4aWvvjjBazjVzMyNz5dynXxzOsd0SZGEJzS7gpCpQRFyFeowQ/gsz9n4CpkxSUdEAKNru3mUNhXflv6vzWsMfXRTo8RQ298jLHF4Y7bZUwJ0IlcctJ4g4w1OArPgHzOwL2SJAh8DjFl/CGC7ddbmTXcruTJV9SqTBNiOmf1TRjFK8kj9ynp3voWmfpmxRTdRzk70vgiUEbZgq/Gc7MVd4k2LAea5E9EaLbtsoUz98v9fmeXmG9mFNq9XVWtdbvmEnIQbGop8jAjllWTfwIobyDdLeWKB33snr9udH5Z7NOYw6B98FocRHtVgxsPUfgirPyswjjjtsHZMzcJji0aR15QFV6VsHfrVTZ51monIuUUjUALOJoe4nphmnMFqy7mgwtGvl9KTJcqt08zs2+L4/kzJgHo25+f8YB2MB
X-OriginatorOrg: fujitsu.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: TYAPR01MB5866.jpnprd01.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 
 7f9541f1-988a-4b19-a640-08db3b3b0981
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Apr 2023 09:48:15.1137
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: a19f121d-81e1-4858-a9d8-736e267fd4c7
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 
 ceT4yxD7CUyjNv+xAYALxSoEpN6RC9dIhMpkhjYQTFIdOcG9u20wYTbZBfmuE/GYCaUPVnvJK/u0d5wzY+lPUZgzaqw8Muto/mrNJkZEuQQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: OS7PR01MB11648
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Archived-At: 
 <https://www.postgresql.org/message-id/TYAPR01MB58667233ECDB40B7FFDDB22CF59B9%40TYAPR01MB5866.jpnprd01.prod.outlook.com>
Precedence: bulk

Dear Julien,

Thank you for updating the patch. I checked yours.
Followings are general or non-minor questions:

1.
Feature freeze for PG16 has already come. So I think there is no reason to =
rush
making the patch. Based on above, could you allow to upgrade while synchron=
izing
data? Personally it can be added as 0002 patch which extends the feature. O=
r
have you already found any problem?

2.
I have a questions about the SQL interface:

ALTER SUBSCRIPTION name ADD TABLE (relid =3D XYZ, state =3D 'x' [, lsn =3D =
'X/Y'])

Here the oid of the table is directly specified, but is it really kept betw=
een
old and new node? Similar command ALTER PUBLICATION requires the name of ta=
ble,
not the oid.

3.
Currently getSubscriptionRels() is called from the getSubscriptions(), but =
I could
not find the reason why we must do like that. Other functions like
getPublicationTables() is directly called from getSchemaData(), so they sho=
uld
be followed. Additionaly, I found two problems.

* Only tables that to be dumped should be included. See getPublicationTable=
s().
* dropStmt for subscription relations seems not to be needed.
* Maybe security label and comments should be also dumped.

Followings are minor comments.


4. parse_subscription_options

```
+                       opts->state =3D defGetString(defel)[0];
```

[0] is not needed.

5. AlterSubscription

```
+                               supported_opts =3D SUBOPT_RELID | SUBOPT_ST=
ATE | SUBOPT_LSN;
+                               parse_subscription_options(pstate, stmt->op=
tions,
+                                                                          =
        supported_opts, &opts);
+
+                               /* relid and state should always be provide=
d. */
+                               Assert(IsSet(opts.specified_opts, SUBOPT_RE=
LID));
+                               Assert(IsSet(opts.specified_opts, SUBOPT_ST=
ATE));
+
```

SUBOPT_LSN accepts "none" string, which means InvalidLSN. Isn't it better t=
o
reject it?

6. dumpSubscription()

```
+       if (dopt->binary_upgrade && dopt->preserve_subscriptions &&
+               subinfo->suboriginremotelsn)
+       {
+               appendPQExpBuffer(query, ", lsn =3D '%s'", subinfo->suborig=
inremotelsn);
+       }
```

{} is not needed.

7. pg_dump.h

```
+/*
+ * The SubRelInfo struct is used to represent subscription relation.
+ */
+typedef struct _SubRelInfo
+{
+       Oid             srrelid;
+       char    srsubstate;
+       char   *srsublsn;
+} SubRelInfo;
```

This typedef must be added to typedefs.list.

8. check_for_subscription_state

```
			nb =3D atooid(PQgetvalue(res, 0, 0));
			if (nb !=3D 0)
			{
				is_error =3D true;
				pg_log(PG_WARNING,
					   "\nWARNING:  %d subscription have invalid remote_lsn",
					   nb);
			}
```

I think no need to use atooid. Additionaly, isn't it better to show the nam=
e of
subscriptions which have invalid remote_lsn?

```
		nb =3D atooid(PQgetvalue(res, 0, 0));
		if (nb !=3D 0)
		{
			is_error =3D true;
			pg_log(PG_WARNING,
				   "\nWARNING: database \"%s\" has %d subscription "
				   "relations(s) in non-ready state", active_db->db_name, nb);
		}
```

Same as above.

9. parseCommandLine

```
+       user_opts.preserve_subscriptions =3D false;
```

I think this initialization is not needed because it is default.

And maybe you missed to run pgindent.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED