From 5f7f79c4c8e85528e88ffa0b749faad1da4ab4d5 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Tue, 25 Feb 2020 14:27:09 -0600 Subject: [PATCH v2 2/2] Allow reloptions on partitioned tables and indexes.. ..which provides default values used for future (direct) partitions. See also similar behavior from: dfa60814 - Fix tablespace handling for partitioned indexes ca410302 - Fix tablespace handling for partitioned tables c94e6942 - Don't allocate storage for partitioned tables. --- src/backend/access/common/reloptions.c | 19 +------------------ src/backend/commands/tablecmds.c | 32 +++++++++++++++++++++++--------- src/test/regress/expected/reloptions.out | 29 +++++++++++++++++++++++++++++ src/test/regress/sql/reloptions.sql | 11 +++++++++++ 4 files changed, 64 insertions(+), 27 deletions(-) diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c index 79430d2..a2678e7 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -1108,10 +1108,8 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc, case RELKIND_RELATION: case RELKIND_TOASTVALUE: case RELKIND_MATVIEW: - options = heap_reloptions(classForm->relkind, datum, false); - break; case RELKIND_PARTITIONED_TABLE: - options = partitioned_table_reloptions(datum, false); + options = heap_reloptions(classForm->relkind, datum, false); break; case RELKIND_VIEW: options = view_reloptions(datum, false); @@ -1581,21 +1579,6 @@ build_reloptions(Datum reloptions, bool validate, } /* - * Option parser for partitioned tables - */ -bytea * -partitioned_table_reloptions(Datum reloptions, bool validate) -{ - /* - * There are no options for partitioned tables yet, but this is able to do - * some validation. - */ - return (bytea *) build_reloptions(reloptions, validate, - RELOPT_KIND_PARTITIONED, - 0, NULL, 0); -} - -/* * Option parser for views */ bytea * diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1b271af..675b74d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -740,17 +740,33 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, /* * Parse and validate reloptions, if any. */ - reloptions = transformRelOptions((Datum) 0, stmt->options, NULL, validnsps, - true, false); + + if (stmt->partbound) + { + /* If partitioned, also use reloptions from the immediate parent */ + Datum oldoptions; + bool isnull; + Oid parentoid = linitial_oid(inheritOids); + HeapTuple tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(parentoid)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for relation %u", parentoid); + + oldoptions = SysCacheGetAttr(RELOID, tuple, Anum_pg_class_reloptions, + &isnull); + + reloptions = transformRelOptions(isnull ? 0 : oldoptions, + stmt->options, NULL, validnsps, true, false); + + ReleaseSysCache(tuple); + } else + reloptions = transformRelOptions((Datum) 0, stmt->options, NULL, + validnsps, true, false); switch (relkind) { case RELKIND_VIEW: (void) view_reloptions(reloptions, true); break; - case RELKIND_PARTITIONED_TABLE: - (void) partitioned_table_reloptions(reloptions, true); - break; default: (void) heap_reloptions(relkind, reloptions, true); } @@ -4155,7 +4171,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, case AT_SetRelOptions: /* SET (...) */ case AT_ResetRelOptions: /* RESET (...) */ case AT_ReplaceRelOptions: /* reset them all, then set just these */ - ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_MATVIEW | ATT_INDEX); + ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX); /* This command never recurses */ /* No command-specific prep needed */ pass = AT_PASS_MISC; @@ -12693,10 +12709,8 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation, case RELKIND_RELATION: case RELKIND_TOASTVALUE: case RELKIND_MATVIEW: - (void) heap_reloptions(rel->rd_rel->relkind, newOptions, true); - break; case RELKIND_PARTITIONED_TABLE: - (void) partitioned_table_reloptions(newOptions, true); + (void) heap_reloptions(rel->rd_rel->relkind, newOptions, true); break; case RELKIND_VIEW: (void) view_reloptions(newOptions, true); diff --git a/src/test/regress/expected/reloptions.out b/src/test/regress/expected/reloptions.out index 44c1304..8fc8b62 100644 --- a/src/test/regress/expected/reloptions.out +++ b/src/test/regress/expected/reloptions.out @@ -222,3 +222,32 @@ SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test_idx3'::regclass; {fillfactor=40} (1 row) +-- For partitioned tables/indexes, expect ALTER..SET fillfactor not to recurse, +-- but rather to set a value in the parent table which is used for new children +CREATE TABLE par(i int) PARTITION BY RANGE(i); +CREATE INDEX par_i_idx ON par(i); +CREATE TABLE par1 PARTITION OF par FOR VALUES FROM (1)TO(2); +ALTER TABLE par SET (fillfactor=13); +ALTER INDEX par_i_idx SET (fillfactor=13); +CREATE TABLE par2 PARTITION OF par FOR VALUES FROM (2)TO(3); +\d+ par1 + Table "public.par1" + Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+------------- + i | integer | | | | plain | | +Partition of: par FOR VALUES FROM (1) TO (2) +Partition constraint: ((i IS NOT NULL) AND (i >= 1) AND (i < 2)) +Indexes: + "par1_i_idx" btree (i) + +\d+ par2 + Table "public.par2" + Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+------------- + i | integer | | | | plain | | +Partition of: par FOR VALUES FROM (2) TO (3) +Partition constraint: ((i IS NOT NULL) AND (i >= 2) AND (i < 3)) +Indexes: + "par2_i_idx" btree (i) WITH (fillfactor='13') +Options: fillfactor=13 + diff --git a/src/test/regress/sql/reloptions.sql b/src/test/regress/sql/reloptions.sql index cac5b0b..804523c 100644 --- a/src/test/regress/sql/reloptions.sql +++ b/src/test/regress/sql/reloptions.sql @@ -128,3 +128,14 @@ SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test_idx'::regclass; CREATE INDEX reloptions_test_idx3 ON reloptions_test (s); ALTER INDEX reloptions_test_idx3 SET (fillfactor=40); SELECT reloptions FROM pg_class WHERE oid = 'reloptions_test_idx3'::regclass; + +-- For partitioned tables/indexes, expect ALTER..SET fillfactor not to recurse, +-- but rather to set a value in the parent table which is used for new children +CREATE TABLE par(i int) PARTITION BY RANGE(i); +CREATE INDEX par_i_idx ON par(i); +CREATE TABLE par1 PARTITION OF par FOR VALUES FROM (1)TO(2); +ALTER TABLE par SET (fillfactor=13); +ALTER INDEX par_i_idx SET (fillfactor=13); +CREATE TABLE par2 PARTITION OF par FOR VALUES FROM (2)TO(3); +\d par1 +\d par2 -- 2.7.4