Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 19 Sep 2024 13:08:33 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: "David G. Johnston" <david.g.johnston@gmail.com>,
	Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Partitioned tables and [un]loggedness
Message-ID: <ZuujwbHFor1zLufJ@paquier.xyz>
References: <ZtCKiJZaoYBHBv05@nathan>
 <ZtZW4gEJOPmeP-nT@paquier.xyz>
 <ZtZn0-JVQuBUbpWw@nathan>
 <Zta_CFeFCx7bMBL8@paquier.xyz>
 <ZtcsLnUSuALqEyE-@nathan>
 <Zt6cDnwSvnuLLnak@paquier.xyz>
 <Zt-V90bS4diq4e0Q@paquier.xyz>
 <ZurvG45F3lQHLw4G@nathan>
 <Zur4qqhABOXtZS8h@nathan>
 <Zutc67q6my4AXOt2@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="mMUwqX7lahztDAh4"
Content-Disposition: inline
In-Reply-To: <Zutc67q6my4AXOt2@paquier.xyz>
Archived-At: <https://www.postgresql.org/message-id/ZuujwbHFor1zLufJ%40paquier.xyz>
Precedence: bulk


--mMUwqX7lahztDAh4
Content-Type: multipart/mixed; boundary="bDLrf5ynSGHLFDUx"
Content-Disposition: inline


--bDLrf5ynSGHLFDUx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Sep 19, 2024 at 08:06:19AM +0900, Michael Paquier wrote:
> I did not notice this one.  I have to admit that the error message
> consistency is welcome, not the extra checks required at
> transformation.

I have applied 0001 for now to add ATT_PARTITIONED_TABLE.  Attached is
the remaining piece.
--
Michael

--bDLrf5ynSGHLFDUx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v5-0002-Remove-support-for-ALTER-TABLE-.-SET-UN-LOGGED-on.patch"
Content-Transfer-Encoding: quoted-printable

=46rom f29099559497e05419bece407fb0be17e35d52e8 Mon Sep 17 00:00:00 2001
=46rom: Michael Paquier <michael@paquier.xyz>
Date: Thu, 19 Sep 2024 13:07:45 +0900
Subject: [PATCH v5] Remove support for ALTER TABLE .. SET [UN]LOGGED on
 partitioned tables

---
 src/backend/commands/tablecmds.c           |  9 +++++++--
 src/bin/pg_dump/pg_dump.c                  |  7 ++++++-
 src/bin/pgbench/pgbench.c                  |  2 +-
 src/test/regress/expected/create_table.out | 10 ++++++++++
 src/test/regress/sql/create_table.sql      |  6 ++++++
 doc/src/sgml/ref/alter_table.sgml          |  4 ++++
 doc/src/sgml/ref/create_table.sgml         |  4 ++++
 7 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablec=
mds.c
index 2d703aa22e..5deeb6490c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -735,6 +735,12 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid own=
erId,
 	else
 		partitioned =3D false;
=20
+	if (relkind =3D=3D RELKIND_PARTITIONED_TABLE &&
+		stmt->relation->relpersistence =3D=3D RELPERSISTENCE_UNLOGGED)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("partitioned tables cannot be unlogged")));
+
 	/*
 	 * Look up the namespace in which we are supposed to create the relation,
 	 * check we have permission to create there, lock it against concurrent
@@ -4989,8 +4995,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd =
*cmd,
 			break;
 		case AT_SetLogged:		/* SET LOGGED */
 		case AT_SetUnLogged:	/* SET UNLOGGED */
-			ATSimplePermissions(cmd->subtype, rel,
-								ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_SEQUENCE);
+			ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_SEQUENCE);
 			if (tab->chgPersistence)
 				ereport(ERROR,
 						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 130b80775d..24a8067ed6 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -15909,8 +15909,13 @@ dumpTableSchema(Archive *fout, const TableInfo *tb=
info)
 			binary_upgrade_set_pg_class_oids(fout, q,
 											 tbinfo->dobj.catId.oid);
=20
+		/*
+		 * PostgreSQL 18 has disabled UNLOGGED for partitioned tables, so
+		 * ignore it when dumping if it was set in this case.
+		 */
 		appendPQExpBuffer(q, "CREATE %s%s %s",
-						  tbinfo->relpersistence =3D=3D RELPERSISTENCE_UNLOGGED ?
+						  tbinfo->relpersistence =3D=3D RELPERSISTENCE_UNLOGGED &&
+						  tbinfo->relkind !=3D RELKIND_PARTITIONED_TABLE ?
 						  "UNLOGGED " : "",
 						  reltypename,
 						  qualrelname);
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 61618f2e18..dc4d7408cd 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -4865,7 +4865,7 @@ initCreateTables(PGconn *con)
=20
 		/* Construct new create table statement. */
 		printfPQExpBuffer(&query, "create%s table %s(%s)",
-						  unlogged_tables ? " unlogged" : "",
+						  unlogged_tables && partition_method =3D=3D PART_NONE ? " unlogged"=
 : "",
 						  ddl->table,
 						  (scale >=3D SCALE_32BIT_THRESHOLD) ? ddl->bigcols : ddl->smcols);
=20
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/=
expected/create_table.out
index 284a7fb85c..c45e02d42f 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -50,6 +50,16 @@ ERROR:  cannot create temporary relation in non-temporar=
y schema
 LINE 1: CREATE TEMP TABLE public.temp_to_perm (a int primary key);
                           ^
 DROP TABLE unlogged1, public.unlogged2;
+CREATE UNLOGGED TABLE unlogged1 (a int) PARTITION BY RANGE (a); -- fail
+ERROR:  partitioned tables cannot be unlogged
+CREATE TABLE unlogged1 (a int) PARTITION BY RANGE (a); -- ok
+ALTER TABLE unlogged1 SET LOGGED; -- fails
+ERROR:  ALTER action SET LOGGED cannot be performed on relation "unlogged1"
+DETAIL:  This operation is not supported for partitioned tables.
+ALTER TABLE unlogged1 SET UNLOGGED; -- fails
+ERROR:  ALTER action SET UNLOGGED cannot be performed on relation "unlogge=
d1"
+DETAIL:  This operation is not supported for partitioned tables.
+DROP TABLE unlogged1;
 CREATE TABLE as_select1 AS SELECT * FROM pg_class WHERE relkind =3D 'r';
 CREATE TABLE as_select1 AS SELECT * FROM pg_class WHERE relkind =3D 'r';
 ERROR:  relation "as_select1" already exists
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/c=
reate_table.sql
index 1fd4cbfa7e..37a227148e 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -30,6 +30,12 @@ CREATE TEMP TABLE pg_temp.doubly_temp (a int primary key=
);		-- also OK
 CREATE TEMP TABLE public.temp_to_perm (a int primary key);		-- not OK
 DROP TABLE unlogged1, public.unlogged2;
=20
+CREATE UNLOGGED TABLE unlogged1 (a int) PARTITION BY RANGE (a); -- fail
+CREATE TABLE unlogged1 (a int) PARTITION BY RANGE (a); -- ok
+ALTER TABLE unlogged1 SET LOGGED; -- fails
+ALTER TABLE unlogged1 SET UNLOGGED; -- fails
+DROP TABLE unlogged1;
+
 CREATE TABLE as_select1 AS SELECT * FROM pg_class WHERE relkind =3D 'r';
 CREATE TABLE as_select1 AS SELECT * FROM pg_class WHERE relkind =3D 'r';
 CREATE TABLE IF NOT EXISTS as_select1 AS SELECT * FROM pg_class WHERE relk=
ind =3D 'r';
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_tab=
le.sgml
index 1a49f321cf..19f29a0470 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -795,6 +795,10 @@ WITH ( MODULUS <replaceable class=3D"parameter">numeri=
c_literal</replaceable>, REM
       (for identity or serial columns).  However, it is also possible to
       change the persistence of such sequences separately.
      </para>
+
+     <para>
+      This form is not supported for partitioned tables.
+     </para>
     </listitem>
    </varlistentry>
=20
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_t=
able.sgml
index c1855b8d82..83859bac76 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -220,6 +220,10 @@ WITH ( MODULUS <replaceable class=3D"parameter">numeri=
c_literal</replaceable>, REM
       If this is specified, any sequences created together with the unlogg=
ed
       table (for identity or serial columns) are also created as unlogged.
      </para>
+
+     <para>
+      This form is not supported for partitioned tables.
+     </para>
     </listitem>
    </varlistentry>
=20
--=20
2.45.2


--bDLrf5ynSGHLFDUx--

--mMUwqX7lahztDAh4
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmbro8EACgkQnvQgOdby
QH2n+g/8CzgW9EWbt8d0q8xbHgMnS37Twpr9b//A6ALDO2h/iXlPxJ+IXcgPbYkO
8ZbP5mM+lrpKDwwsrqayagZXbBYuf997WNNXRR9mDYT41WeZ0Srf7PiL195bV3pC
+qfpqZMesRbjVaFKQ1sqdqhR8b8vlit4XSnvtVJbdJWWBdjn5934PJaKSlUuPQF/
rWwtb2oWRjATakIB+LrStJGiNU+UB9xiUtjghagfx8sKRZkYfMPkZWqKvr1SkQTo
hZ4yHWe53WMRW9D7TWglVqBDbqeBfKd+HI+5NPJ0W41qaTPDCw8dMoVqThhcYR/E
IKCd54m/dh9kxxIeox6wDN82WtPQe530xhGjNsr9eO6GbgvhFZTACgjDp0gY/Nqy
J815xRkflQgcKAcFYlRY5q1BagXVOHJsdCeJbs9kYg2BYzg8ciOAz30OLU3+KOB4
nk/WuHKTcGQcS7sI9OawoSqaeqsIq4d0gcFZ2QRWvTRMf34PZEd+60HEHqc8vZDB
kp3P/dha3c/WLeiucCPXUSpyeTMXiVFnAGv599LLcPSQeJkKZQTgNhZikIACKAO+
VdUqv0TfTJbuLA0Zt4lNkHnShuPAazYeyvaNpHgsLlpWZe67N4yacKBsHZCNVsls
2I616eRKNNQSsxHW6WEEYxsj1WuWP2T+3VK0ak91LReBr3vnifA=
=6Kq/
-----END PGP SIGNATURE-----

--mMUwqX7lahztDAh4--