public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v7 7/7] pg_dump: partitioned index depend on its partitions
7+ messages / 3 participants
[nested] [flat]

* [PATCH v7 7/7] pg_dump: partitioned index depend on its partitions
@ 2020-11-25 23:34  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Justin Pryzby @ 2020-11-25 23:34 UTC (permalink / raw)

This is required for restoring clustered parent index, which is marked INVALID
until indexes have been built on all its child tables, and it's prohibited to
CLUSTER ON an INVALID index

See also: 8cff4f5348d075e063100071013f00a900c32b0f
---
 src/backend/commands/tablecmds.c | 6 +++---
 src/bin/pg_dump/common.c         | 8 ++++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 420991e315..a478c13990 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -17521,6 +17521,9 @@ validatePartitionedIndex(Relation partedIdx, Relation partedTbl)
 		table_close(idxRel, RowExclusiveLock);
 	}
 
+	/* make sure we see the validation we just did */
+	CommandCounterIncrement();
+
 	/*
 	 * If this index is in turn a partition of a larger index, validating it
 	 * might cause the parent to become valid also.  Try that.
@@ -17532,9 +17535,6 @@ validatePartitionedIndex(Relation partedIdx, Relation partedTbl)
 		Relation	parentIdx,
 					parentTbl;
 
-		/* make sure we see the validation we just did */
-		CommandCounterIncrement();
-
 		parentIdxId = get_partition_parent(RelationGetRelid(partedIdx));
 		parentTblId = get_partition_parent(RelationGetRelid(partedTbl));
 		parentIdx = relation_open(parentIdxId, AccessExclusiveLock);
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 1a261a5545..cdfba058fc 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -429,6 +429,12 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
 			attachinfo[k].parentIdx = parentidx;
 			attachinfo[k].partitionIdx = index;
 
+			/*
+			 * We want dependencies from parent to partition (so that the
+			 * partition index is created first)
+			 */
+			addObjectDependency(&parentidx->dobj, index->dobj.dumpId);
+
 			/*
 			 * We must state the DO_INDEX_ATTACH object's dependencies
 			 * explicitly, since it will not match anything in pg_depend.
@@ -446,6 +452,8 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
 			 */
 			addObjectDependency(&attachinfo[k].dobj, index->dobj.dumpId);
 			addObjectDependency(&attachinfo[k].dobj, parentidx->dobj.dumpId);
+			// addObjectDependency(&parentidx->dobj, attachinfo[k].dobj.dumpId);
+
 			addObjectDependency(&attachinfo[k].dobj,
 								index->indextable->dobj.dumpId);
 			addObjectDependency(&attachinfo[k].dobj,
-- 
2.17.0


--AA9g+nFNFPYNJKiL--





^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
@ 2024-04-15 01:46  Michael Paquier <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2024-04-15 01:46 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Alexander Lakhin <[email protected]>; Alvaro Herrera <[email protected]>; Peter Eisentraut <[email protected]>; Soumyadeep Chakraborty <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers; Ashwin Agrawal <[email protected]>; [email protected]

On Tue, Apr 02, 2024 at 01:06:06AM -0400, Tom Lane wrote:
> AFAICS, e2395cdbe posits that taking exclusive lock on pg_am in the
> middle of a bunch of concurrent regression scripts couldn't possibly
> cause any problems.  Really?

There is no need for a catalog here to trigger the failure, and it
would have happened as long as a foreign table is used.  The problem
introduced in 374c7a229042 fixed by e2395cdbe83a comes from a thinko
on my side, my apologies for that and the delay in replying.  Thanks
for the extra fix done in 13b3b62746ec, Alvaro.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
@ 2024-04-16 05:14  Michael Paquier <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2024-04-16 05:14 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Alexander Lakhin <[email protected]>; Alvaro Herrera <[email protected]>; Peter Eisentraut <[email protected]>; Soumyadeep Chakraborty <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers; Ashwin Agrawal <[email protected]>; [email protected]

On Mon, Apr 15, 2024 at 10:46:00AM +0900, Michael Paquier wrote:
> There is no need for a catalog here to trigger the failure, and it
> would have happened as long as a foreign table is used.  The problem
> introduced in 374c7a229042 fixed by e2395cdbe83a comes from a thinko
> on my side, my apologies for that and the delay in replying.  Thanks
> for the extra fix done in 13b3b62746ec, Alvaro.

While doing more tests with this feature, among other things, I've
spotted an incorrect behavior with dump/restore with the handling of
the GUC default_table_access_method when it comes to partitions.
Imagine the following in database "a":
CREATE TABLE parent_tab (id int) PARTITION BY RANGE (id);
CREATE TABLE parent_tab_2 (id int) PARTITION BY RANGE (id) USING heap;
CREATE TABLE parent_tab_3 (id int) PARTITION BY RANGE (id);

This leads to the following in pg_class:
=# SELECT relname, relam FROM pg_class WHERE oid > 16000;
   relname    | relam 
--------------+-------
 parent_tab   |     0
 parent_tab_2 |     2
 parent_tab_3 |     0
(3 rows)

Now, let's do the following:
$ createdb b
$ pg_dump | psql b
$ psql b
=# SELECT relname, relam FROM pg_class WHERE oid > 16000;
   relname    | relam 
--------------+-------
 parent_tab   |     0
 parent_tab_2 |     0
 parent_tab_3 |     0
(3 rows)

And parent_tab_2 would now rely on the default GUC when creating new
partitions rather than enforce heap.

It seems to me that we are going to extend the GUC
default_table_access_method with a "default" mode to be able to force
relam to 0 and make a difference with the non-0 case, in the same way
as ALTER TABLE SET ACCESS METHOD DEFAULT.  The thing is that, like
tablespaces, we have to rely on a GUC and not a USING clause to be
able to handle --no-table-access-method.

An interesting point comes to what we should do for
default_table_access_method set to "default" when dealing with
something else than a partitioned table, where an error may be
adapted.  Still, I'm wondering if there are more flavors I lack
imagination for.  This requires more careful design work.

Perhaps somebody has a good idea?
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
@ 2024-04-16 05:19  Michael Paquier <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2024-04-16 05:19 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Alexander Lakhin <[email protected]>; Alvaro Herrera <[email protected]>; Peter Eisentraut <[email protected]>; Soumyadeep Chakraborty <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers; Ashwin Agrawal <[email protected]>; [email protected]

On Tue, Apr 16, 2024 at 02:14:21PM +0900, Michael Paquier wrote:
> It seems to me that we are going to extend the GUC
> default_table_access_method with a "default" mode to be able to force
> relam to 0 and make a difference with the non-0 case, in the same way
> as ALTER TABLE SET ACCESS METHOD DEFAULT.  The thing is that, like
> tablespaces, we have to rely on a GUC and not a USING clause to be
> able to handle --no-table-access-method.
> 
> An interesting point comes to what we should do for
> default_table_access_method set to "default" when dealing with
> something else than a partitioned table, where an error may be
> adapted.  Still, I'm wondering if there are more flavors I lack
> imagination for.  This requires more careful design work.
> 
> Perhaps somebody has a good idea?

Actually, I've come up with an idea just after hitting the send
button: let's use an extra ALTER TABLE SET ACCESS METHOD rather than
rely on the GUC to set the AM of the partitioned table correctly.
This extra command should be optional, depending on
--no-table-access-method.  If a partitioned table has 0 as relam,
let's not add this extra ALTER TABLE at all.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
@ 2024-04-17 05:31  Michael Paquier <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2024-04-17 05:31 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Alexander Lakhin <[email protected]>; Alvaro Herrera <[email protected]>; Peter Eisentraut <[email protected]>; Soumyadeep Chakraborty <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers; Ashwin Agrawal <[email protected]>; [email protected]

On Tue, Apr 16, 2024 at 02:19:56PM +0900, Michael Paquier wrote:
> Actually, I've come up with an idea just after hitting the send
> button: let's use an extra ALTER TABLE SET ACCESS METHOD rather than
> rely on the GUC to set the AM of the partitioned table correctly.
> This extra command should be optional, depending on
> --no-table-access-method.  If a partitioned table has 0 as relam,
> let's not add this extra ALTER TABLE at all.

I have explored this idea, and while this is tempting this faces a
couple of challenges:
1) Binary upgrades would fail because the table rewrite created by
ALTER TABLE SET ACCESS METHOD for relkinds with physical storage
expects heap_create_with_catalog to have a fixed OID, but the rewrite
would require extra steps to be able to handle that, and I am not
convinced that more binary_upgrade_set_next_heap_relfilenode() is a
good idea.
2) We could limit these extra ALTER TABLE commands to be generated for
partitioned tables.  This is kind of confusing as resulting dumps
would mix SET commands for default_table_access_method that would
affect tables with physical storage, while partitioned tables would
have their own extra ALTER TABLE commands.  Another issue that needs
more consideration is that TocEntrys don't hold any relkind
information so pg_backup_archiver.c cannot make a difference with
tables and partitioned tables to select if SET or ALTER TABLE should
be generated.

Several designs are possible, like:
- Mix SET and ALTER TABLE commands in the dumps to set the AM, SET for
tables and matviews, ALTER TABLE for relations without storage.  This
would bypass the binary upgrade problem with the fixed relid.
- Use only SET, requiring a new "default" value for
default_table_access_method that would force a partitioned table's
relam to be 0.  Be stricter with the "current" table AM tracked in
pg_dump's backup archiver.
- Use only ALTER TABLE commands, with extra binary upgrade tweaks to
force relation OIDs for the second heap_create_with_catalog() done
with the rewrite to update a relation's AM.

With all that in mind, it may be better to revert 374c7a229042 and
e2395cdbe83a from HEAD and reconsider how to tackle the dump issues in
v18 or newer versions as all of the approaches I can think of lead to
more complications of their own.

Please see attached a non-polished POC that switches dumps to use
ALTER TABLE, that I've used to detect the upgrade problems.

Thoughts or comments are welcome.
--
Michael


Attachments:

  [text/x-diff] dump-partitions.patch (7.7K, ../../[email protected]/2-dump-partitions.patch)
  download | inline diff:
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index c7a6c918a6..93df260986 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -61,7 +61,7 @@ static void _becomeUser(ArchiveHandle *AH, const char *user);
 static void _becomeOwner(ArchiveHandle *AH, TocEntry *te);
 static void _selectOutputSchema(ArchiveHandle *AH, const char *schemaName);
 static void _selectTablespace(ArchiveHandle *AH, const char *tablespace);
-static void _selectTableAccessMethod(ArchiveHandle *AH, const char *tableam);
+static void _selectTableAccessMethod(ArchiveHandle *AH, TocEntry *te);
 static void processEncodingEntry(ArchiveHandle *AH, TocEntry *te);
 static void processStdStringsEntry(ArchiveHandle *AH, TocEntry *te);
 static void processSearchPathEntry(ArchiveHandle *AH, TocEntry *te);
@@ -2373,7 +2373,6 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 	AH->currUser = NULL;		/* unknown */
 	AH->currSchema = NULL;		/* ditto */
 	AH->currTablespace = NULL;	/* ditto */
-	AH->currTableAm = NULL;		/* ditto */
 
 	AH->toc = (TocEntry *) pg_malloc0(sizeof(TocEntry));
 
@@ -3356,9 +3355,6 @@ _reconnectToDB(ArchiveHandle *AH, const char *dbname)
 	free(AH->currSchema);
 	AH->currSchema = NULL;
 
-	free(AH->currTableAm);
-	AH->currTableAm = NULL;
-
 	free(AH->currTablespace);
 	AH->currTablespace = NULL;
 
@@ -3519,31 +3515,32 @@ _selectTablespace(ArchiveHandle *AH, const char *tablespace)
 }
 
 /*
- * Set the proper default_table_access_method value for the table.
+ * Set the proper default table access method for the table.
  */
 static void
-_selectTableAccessMethod(ArchiveHandle *AH, const char *tableam)
+_selectTableAccessMethod(ArchiveHandle *AH, TocEntry *te)
 {
 	RestoreOptions *ropt = AH->public.ropt;
+	const char *tableam = te->tableam;
 	PQExpBuffer cmd;
-	const char *want,
-			   *have;
 
 	/* do nothing in --no-table-access-method mode */
 	if (ropt->noTableAm)
 		return;
 
-	have = AH->currTableAm;
-	want = tableam;
-
-	if (!want)
-		return;
-
-	if (have && strcmp(want, have) == 0)
+	if (!tableam)
 		return;
 
 	cmd = createPQExpBuffer();
-	appendPQExpBuffer(cmd, "SET default_table_access_method = %s;", fmtId(want));
+
+	appendPQExpBufferStr(cmd, "ALTER ");
+	if (strcmp(te->desc, "MATERIALIZED VIEW") == 0)
+		appendPQExpBufferStr(cmd, "MATERIALIZED VIEW ");
+	else
+		appendPQExpBufferStr(cmd, "TABLE ");
+	appendPQExpBuffer(cmd, "%s ", fmtQualifiedId(te->namespace, te->tag));
+	appendPQExpBuffer(cmd, "SET ACCESS METHOD %s;",
+					  fmtId(tableam));
 
 	if (RestoringToDB(AH))
 	{
@@ -3553,7 +3550,7 @@ _selectTableAccessMethod(ArchiveHandle *AH, const char *tableam)
 
 		if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
 			warn_or_exit_horribly(AH,
-								  "could not set default_table_access_method: %s",
+								  "could not set table access method: %s",
 								  PQerrorMessage(AH->connection));
 
 		PQclear(res);
@@ -3562,9 +3559,6 @@ _selectTableAccessMethod(ArchiveHandle *AH, const char *tableam)
 		ahprintf(AH, "%s\n\n", cmd->data);
 
 	destroyPQExpBuffer(cmd);
-
-	free(AH->currTableAm);
-	AH->currTableAm = pg_strdup(want);
 }
 
 /*
@@ -3673,11 +3667,10 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
 {
 	RestoreOptions *ropt = AH->public.ropt;
 
-	/* Select owner, schema, tablespace and default AM as necessary */
+	/* Select owner, schema and tablespace as necessary */
 	_becomeOwner(AH, te);
 	_selectOutputSchema(AH, te->namespace);
 	_selectTablespace(AH, te->tablespace);
-	_selectTableAccessMethod(AH, te->tableam);
 
 	/* Emit header comment for item */
 	if (!AH->noTocComments)
@@ -3812,6 +3805,12 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
 		}
 	}
 
+	/*
+	 * Select the table's default AM, once the table definition has
+	 * been generated.
+	 */
+	_selectTableAccessMethod(AH, te);
+
 	/*
 	 * If it's an ACL entry, it might contain SET SESSION AUTHORIZATION
 	 * commands, so we can no longer assume we know the current auth setting.
@@ -4173,8 +4172,6 @@ restore_toc_entries_prefork(ArchiveHandle *AH, TocEntry *pending_list)
 	AH->currSchema = NULL;
 	free(AH->currTablespace);
 	AH->currTablespace = NULL;
-	free(AH->currTableAm);
-	AH->currTableAm = NULL;
 }
 
 /*
@@ -4910,7 +4907,6 @@ CloneArchive(ArchiveHandle *AH)
 	clone->connCancel = NULL;
 	clone->currUser = NULL;
 	clone->currSchema = NULL;
-	clone->currTableAm = NULL;
 	clone->currTablespace = NULL;
 
 	/* savedPassword must be local in case we change it while connecting */
@@ -4970,7 +4966,6 @@ DeCloneArchive(ArchiveHandle *AH)
 	free(AH->currUser);
 	free(AH->currSchema);
 	free(AH->currTablespace);
-	free(AH->currTableAm);
 	free(AH->savedPassword);
 
 	free(AH);
diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h
index d6104a7196..d7409916d7 100644
--- a/src/bin/pg_dump/pg_backup_archiver.h
+++ b/src/bin/pg_dump/pg_backup_archiver.h
@@ -322,7 +322,6 @@ struct _archiveHandle
 	char	   *currUser;		/* current username, or NULL if unknown */
 	char	   *currSchema;		/* current schema, or NULL */
 	char	   *currTablespace; /* current tablespace, or NULL */
-	char	   *currTableAm;	/* current table access method, or NULL */
 
 	/* in --transaction-size mode, this counts objects emitted in cur xact */
 	int			txnCount;
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 0c057fef94..c37054e7e7 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4539,11 +4539,8 @@ my %tests = (
 			CREATE TABLE dump_test.regress_pg_dump_table_am_1 (col1 int) USING regress_table_am;
 			CREATE TABLE dump_test.regress_pg_dump_table_am_2() USING heap;',
 		regexp => qr/^
-			\QSET default_table_access_method = regress_table_am;\E
-			(\n(?!SET[^;]+;)[^\n]*)*
-			\n\QCREATE TABLE dump_test.regress_pg_dump_table_am_1 (\E
-			\n\s+\Qcol1 integer\E
-			\n\);/xm,
+			\QALTER TABLE dump_test.regress_pg_dump_table_am_1 SET ACCESS METHOD regress_table_am;\E
+			\n/xm,
 		like => {
 			%full_runs, %dump_test_schema_runs, section_pre_data => 1,
 		},
@@ -4562,12 +4559,8 @@ my %tests = (
 				USING regress_table_am AS SELECT count(*) FROM pg_class;
 			CREATE MATERIALIZED VIEW dump_test.regress_pg_dump_matview_am_2 USING heap AS SELECT 1;',
 		regexp => qr/^
-			\QSET default_table_access_method = regress_table_am;\E
-			(\n(?!SET[^;]+;)[^\n]*)*
-			\QCREATE MATERIALIZED VIEW dump_test.regress_pg_dump_matview_am_1 AS\E
-			\n\s+\QSELECT count(*) AS count\E
-			\n\s+\QFROM pg_class\E
-			\n\s+\QWITH NO DATA;\E\n/xm,
+			\QALTER MATERIALIZED VIEW dump_test.regress_pg_dump_matview_am_1 SET ACCESS METHOD regress_table_am;\E
+			\n/xm,
 		like => {
 			%full_runs, %dump_test_schema_runs, section_pre_data => 1,
 		},
@@ -4591,18 +4584,12 @@ my %tests = (
 			CREATE TABLE dump_test.regress_pg_dump_table_am_child_2
 			  PARTITION OF dump_test.regress_pg_dump_table_am_parent FOR VALUES IN (2);',
 		regexp => qr/^
-			\QSET default_table_access_method = regress_table_am;\E
-			(\n(?!SET[^;]+;)[^\n]*)*
-			\n\QCREATE TABLE dump_test.regress_pg_dump_table_am_parent (\E
+			\QALTER TABLE dump_test.regress_pg_dump_table_am_parent SET ACCESS METHOD regress_table_am;\E
 			(.*\n)*
-			\QSET default_table_access_method = heap;\E
-			(\n(?!SET[^;]+;)[^\n]*)*
-			\n\QCREATE TABLE dump_test.regress_pg_dump_table_am_child_1 (\E
+			\n\QALTER TABLE dump_test.regress_pg_dump_table_am_child_1 SET ACCESS METHOD heap;\E
 			(.*\n)*
-			\QSET default_table_access_method = regress_table_am;\E
-			(\n(?!SET[^;]+;)[^\n]*)*
-			\n\QCREATE TABLE dump_test.regress_pg_dump_table_am_child_2 (\E
-			(.*\n)*/xm,
+			\n\QALTER TABLE dump_test.regress_pg_dump_table_am_child_2 SET ACCESS METHOD regress_table_am;\E
+			\n/xm,
 		like => {
 			%full_runs, %dump_test_schema_runs, section_pre_data => 1,
 		},


  [application/pgp-signature] signature.asc (833B, ../../[email protected]/3-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
@ 2024-04-17 07:40  Alvaro Herrera <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Alvaro Herrera @ 2024-04-17 07:40 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; Justin Pryzby <[email protected]>; Alexander Lakhin <[email protected]>; Peter Eisentraut <[email protected]>; Soumyadeep Chakraborty <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers; Ashwin Agrawal <[email protected]>; [email protected]

On 2024-Apr-17, Michael Paquier wrote:

> 2) We could limit these extra ALTER TABLE commands to be generated for
> partitioned tables.  This is kind of confusing as resulting dumps
> would mix SET commands for default_table_access_method that would
> affect tables with physical storage, while partitioned tables would
> have their own extra ALTER TABLE commands.

Hmm, cannot we simply add a USING clause to the CREATE TABLE command for
partitioned tables?  That would override the
default_table_access_method, so it should give the correct result, no?

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/






^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
@ 2024-04-17 07:50  Alvaro Herrera <[email protected]>
  parent: Alvaro Herrera <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Alvaro Herrera @ 2024-04-17 07:50 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; Justin Pryzby <[email protected]>; Alexander Lakhin <[email protected]>; Peter Eisentraut <[email protected]>; Soumyadeep Chakraborty <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers; Ashwin Agrawal <[email protected]>; [email protected]

On 2024-Apr-17, Alvaro Herrera wrote:

> Hmm, cannot we simply add a USING clause to the CREATE TABLE command for
> partitioned tables?  That would override the
> default_table_access_method, so it should give the correct result, no?

Ah, upthread you noted that pg_restore-time --no-table-access-method
needs to be able to elide it, so a dump-time USING clause doesn't work.

I think it's easy enough to add a "bool ispartitioned" to TableInfo and
use an ALTER TABLE or rely on the GUC depending on that -- seems easy
enough.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/






^ permalink  raw  reply  [nested|flat] 7+ messages in thread


end of thread, other threads:[~2024-04-17 07:50 UTC | newest]

Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-25 23:34 [PATCH v7 7/7] pg_dump: partitioned index depend on its partitions Justin Pryzby <[email protected]>
2024-04-15 01:46 Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Michael Paquier <[email protected]>
2024-04-16 05:14 ` Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Michael Paquier <[email protected]>
2024-04-16 05:19   ` Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Michael Paquier <[email protected]>
2024-04-17 05:31     ` Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Michael Paquier <[email protected]>
2024-04-17 07:40       ` Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Alvaro Herrera <[email protected]>
2024-04-17 07:50         ` Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Alvaro Herrera <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox