From c5d88bf6ec53ea562736c62b71ab598159f6384a Mon Sep 17 00:00:00 2001 From: Koval Dmitry Date: Thu, 29 Aug 2024 20:27:03 +0300 Subject: [PATCH v34 3/3] Refactor createPartitionTable to remove ProcessUtility call --- src/backend/catalog/heap.c | 8 +- src/backend/commands/tablecmds.c | 290 ++++++++++++++---- src/include/catalog/heap.h | 6 + src/test/regress/expected/partition_split.out | 59 ++++ src/test/regress/sql/partition_split.sql | 21 ++ 5 files changed, 326 insertions(+), 58 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index d7b88b61dc..5e8078baf2 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -101,16 +101,12 @@ static ObjectAddress AddNewRelationType(const char *typeName, Oid new_row_type, Oid new_array_type); static void RelationRemoveInheritance(Oid relid); -static Oid StoreRelCheck(Relation rel, const char *ccname, Node *expr, - bool is_validated, bool is_local, int16 inhcount, - bool is_no_inherit, bool is_internal); static void StoreConstraints(Relation rel, List *cooked_constraints, bool is_internal); static bool MergeWithExistingConstraint(Relation rel, const char *ccname, Node *expr, bool allow_merge, bool is_local, bool is_initially_valid, bool is_no_inherit); -static void SetRelationNumChecks(Relation rel, int numchecks); static Node *cookConstraint(ParseState *pstate, Node *raw_constraint, char *relname); @@ -2072,7 +2068,7 @@ SetAttrMissing(Oid relid, char *attname, char *value) * * The OID of the new constraint is returned. */ -static Oid +Oid StoreRelCheck(Relation rel, const char *ccname, Node *expr, bool is_validated, bool is_local, int16 inhcount, bool is_no_inherit, bool is_internal) @@ -3045,7 +3041,7 @@ AddRelationNotNullConstraints(Relation rel, List *constraints, * relcache entries for the rel. Also, this backend will rebuild its * own relcache entry at the next CommandCounterIncrement. */ -static void +void SetRelationNumChecks(Relation rel, int numchecks) { Relation relrel; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 93d46d85b8..2545320426 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -21118,6 +21118,191 @@ moveSplitTableRows(Relation rel, Relation splitRel, List *partlist, List *newPar } } + +/* + * getAttributesList: return list of columns (ColumnDef) like model table + * (modelRel) + */ +static List * +getAttributesList(Relation modelRel) +{ + AttrNumber parent_attno; + TupleDesc modelDesc; + List *colList = NIL; + + modelDesc = RelationGetDescr(modelRel); + + for (parent_attno = 1; parent_attno <= modelDesc->natts; + parent_attno++) + { + Form_pg_attribute attribute = TupleDescAttr(modelDesc, + parent_attno - 1); + ColumnDef *def; + + /* Ignore dropped columns in the parent. */ + if (attribute->attisdropped) + continue; + + def = makeColumnDef(NameStr(attribute->attname), attribute->atttypid, + attribute->atttypmod, attribute->attcollation); + + def->is_not_null = attribute->attnotnull; + + /* Add to column list */ + colList = lappend(colList, def); + + /* + * Although we don't transfer the column's default/generation + * expression now, we need to mark it GENERATED if appropriate. + */ + if (attribute->atthasdef && attribute->attgenerated) + def->generated = attribute->attgenerated; + + def->storage = attribute->attstorage; + + /* Likewise, copy compression if requested */ + if (CompressionMethodIsValid(attribute->attcompression)) + def->compression = + pstrdup(GetCompressionMethodName(attribute->attcompression)); + else + def->compression = NULL; + } + + return colList; +} + + +/* + * createTableConstraints: create constraints, default values and generated + * values (prototype is function expandTableLikeClause). + */ +static void +createTableConstraints(Relation modelRel, Relation newRel) +{ + TupleDesc tupleDesc; + TupleConstr *constr; + AttrMap *attmap; + AttrNumber parent_attno; + int ccnum; + + tupleDesc = RelationGetDescr(modelRel); + constr = tupleDesc->constr; + + if (!constr) + return; + + /* + * Construct a map from the LIKE relation's attnos to the child rel's. + * This re-checks type match etc, although it shouldn't be possible to + * have a failure since both tables are locked. + */ + attmap = build_attrmap_by_name(RelationGetDescr(newRel), + tupleDesc, + false); + + /* Cycle for default values. */ + for (parent_attno = 1; parent_attno <= tupleDesc->natts; parent_attno++) + { + Form_pg_attribute attribute = TupleDescAttr(tupleDesc, + parent_attno - 1); + + /* Ignore dropped columns in the parent. */ + if (attribute->attisdropped) + continue; + + /* Copy default, if present and it should be copied. */ + if (attribute->atthasdef) + { + Node *this_default = NULL; + AttrDefault *attrdef = constr->defval; + bool found_whole_row; + int16 num; + Node *def; + + /* Find default in constraint structure */ + for (int i = 0; i < constr->num_defval; i++) + { + if (attrdef[i].adnum == parent_attno) + { + this_default = stringToNode(attrdef[i].adbin); + break; + } + } + if (this_default == NULL) + elog(ERROR, "default expression not found for attribute %d of relation \"%s\"", + parent_attno, RelationGetRelationName(modelRel)); + + num = attmap->attnums[parent_attno - 1]; + def = map_variable_attnos(this_default, 1, 0, attmap, InvalidOid, &found_whole_row); + + /* + * Prevent this for the same reason as for constraints below. Note + * that defaults cannot contain any vars, so it's OK that the + * error message refers to generated columns. + */ + if (found_whole_row) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot convert whole-row table reference"), + errdetail("Generation expression for column \"%s\" contains a whole-row reference to table \"%s\".", + NameStr(attribute->attname), + RelationGetRelationName(modelRel)))); + + /* Add a pre-cooked default expression. */ + (void) StoreAttrDefault(newRel, num, def, true, false); + } + } + + /* Cycle for CHECK constraints. */ + for (ccnum = 0; ccnum < constr->num_check; ccnum++) + { + char *ccname = constr->check[ccnum].ccname; + char *ccbin = constr->check[ccnum].ccbin; + bool ccnoinherit = constr->check[ccnum].ccnoinherit; + Node *ccbin_node; + bool found_whole_row; + + ccbin_node = map_variable_attnos(stringToNode(ccbin), + 1, 0, + attmap, + InvalidOid, &found_whole_row); + + /* + * We reject whole-row variables because the whole point of LIKE is + * that the new table's rowtype might later diverge from the parent's. + * So, while translation might be possible right now, it wouldn't be + * possible to guarantee it would work in future. + */ + if (found_whole_row) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot convert whole-row table reference"), + errdetail("Constraint \"%s\" contains a whole-row reference to table \"%s\".", + ccname, + RelationGetRelationName(modelRel)))); + + /* We can skip validation, since the new table should be empty. */ + (void) StoreRelCheck(newRel, ccname, ccbin_node, true, true, + 0, ccnoinherit, false); + } + + /* Update the count of constraints in the relation's pg_class tuple. */ + SetRelationNumChecks(newRel, constr->num_check); + + /* Reproduce not-null constraints. */ + if (constr->has_not_null) + { + List *nnconstraints; + + nnconstraints = RelationGetNotNullConstraints(RelationGetRelid(modelRel), + false, true); + + Assert(list_length(nnconstraints) > 0); + AddRelationNotNullConstraints(newRel, nnconstraints, NULL); + } +} + + /* * createPartitionTable: create table for a new partition with given name * (newPartName) like table (modelRel) @@ -21129,13 +21314,14 @@ moveSplitTableRows(Relation rel, Relation splitRel, List *partlist, List *newPar * Function returns the created relation (locked in AccessExclusiveLock mode). */ static Relation -createPartitionTable(RangeVar *newPartName, Relation modelRel, - AlterTableUtilityContext *context) +createPartitionTable(RangeVar *newPartName, Relation modelRel) { - CreateStmt *createStmt; - TableLikeClause *tlc; - PlannedStmt *wrapper; Relation newRel; + Oid newRelId; + TupleDesc descriptor; + List *colList = NIL; + Oid relamId; + Oid namespaceId; /* If existing rel is temp, it must belong to this session */ if (modelRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP && @@ -21144,56 +21330,53 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("cannot create as partition of temporary relation of another session"))); - /* New partition should have the same persistence as modelRel */ - newPartName->relpersistence = modelRel->rd_rel->relpersistence; - - createStmt = makeNode(CreateStmt); - createStmt->relation = newPartName; - createStmt->tableElts = NIL; - createStmt->inhRelations = NIL; - createStmt->constraints = NIL; - createStmt->options = NIL; - createStmt->oncommit = ONCOMMIT_NOOP; - createStmt->tablespacename = get_tablespace_name(modelRel->rd_rel->reltablespace); - createStmt->if_not_exists = false; - createStmt->accessMethod = get_am_name(modelRel->rd_rel->relam); - - tlc = makeNode(TableLikeClause); - tlc->relation = makeRangeVar(get_namespace_name(RelationGetNamespace(modelRel)), - RelationGetRelationName(modelRel), -1); - - /* - * Indexes will be inherited on "attach new partitions" stage, after data - * moving. We also don't copy the extended statistics for consistency - * with CREATE TABLE PARTITION OF. - */ - tlc->options = CREATE_TABLE_LIKE_ALL & - ~(CREATE_TABLE_LIKE_INDEXES | CREATE_TABLE_LIKE_IDENTITY | CREATE_TABLE_LIKE_STATISTICS); - tlc->relationOid = InvalidOid; - createStmt->tableElts = lappend(createStmt->tableElts, tlc); - - /* Need to make a wrapper PlannedStmt. */ - wrapper = makeNode(PlannedStmt); - wrapper->commandType = CMD_UTILITY; - wrapper->canSetTag = false; - wrapper->utilityStmt = (Node *) createStmt; - wrapper->stmt_location = context->pstmt->stmt_location; - wrapper->stmt_len = context->pstmt->stmt_len; - - ProcessUtility(wrapper, - context->queryString, - false, - PROCESS_UTILITY_SUBCOMMAND, - NULL, - NULL, - None_Receiver, - NULL); + /* Look up inheritance ancestors and generate relation schema. */ + colList = getAttributesList(modelRel); + + /* Create a tuple descriptor from the relation schema. */ + descriptor = BuildDescForRelation(colList); + + /* Look up the access method for new relation. */ + relamId = (modelRel->rd_rel->relam != InvalidOid) ? modelRel->rd_rel->relam : HEAP_TABLE_AM_OID; + + /* Look up the namespace in which we are supposed to create the relation. */ + namespaceId = + RangeVarGetAndCheckCreationNamespace(newPartName, NoLock, NULL); + + /* Create the relation. */ + newRelId = heap_create_with_catalog(newPartName->relname, + namespaceId, + modelRel->rd_rel->reltablespace, + InvalidOid, + InvalidOid, + InvalidOid, + GetUserId(), + relamId, + descriptor, + NIL, + RELKIND_RELATION, + newPartName->relpersistence, + false, + false, + ONCOMMIT_NOOP, + (Datum) 0, + true, + allowSystemTableMods, + false, + InvalidOid, + NULL); + + /* + * We must bump the command counter to make the newly-created relation + * tuple visible for opening. + */ + CommandCounterIncrement(); /* * Open the new partition with no lock, because we already have * AccessExclusiveLock placed there after creation. */ - newRel = table_openrv(newPartName, NoLock); + newRel = table_open(newRelId, NoLock); /* * We intended to create the partition with the same persistence as the @@ -21216,6 +21399,9 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel, errmsg("cannot create a permanent relation as partition of temporary relation \"%s\"", RelationGetRelationName(modelRel)))); + /* Create constraints, default values and generated values */ + createTableConstraints(modelRel, newRel); + return newRel; } @@ -21313,7 +21499,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, SinglePartitionSpec *sps = (SinglePartitionSpec *) lfirst(listptr); Relation newPartRel; - newPartRel = createPartitionTable(sps->name, rel, context); + newPartRel = createPartitionTable(sps->name, rel); newPartRels = lappend(newPartRels, newPartRel); } @@ -21557,7 +21743,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel, } /* Create table for new partition, use partitioned table as model. */ - newPartRel = createPartitionTable(cmd->name, rel, context); + newPartRel = createPartitionTable(cmd->name, rel); /* Copy data from merged partitions to new partition. */ moveMergedTablesRows(rel, mergingPartitionsList, newPartRel); diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h index 8c278f202b..afa5e8628a 100644 --- a/src/include/catalog/heap.h +++ b/src/include/catalog/heap.h @@ -118,6 +118,12 @@ extern List *AddRelationNotNullConstraints(Relation rel, List *constraints, List *old_notnulls); +extern Oid StoreRelCheck(Relation rel, const char *ccname, Node *expr, + bool is_validated, bool is_local, int16 inhcount, + bool is_no_inherit, bool is_internal); + +extern void SetRelationNumChecks(Relation rel, int numchecks); + extern void RelationClearMissing(Relation rel); extern void SetAttrMissing(Oid relid, char *attname, char *value); diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out index 0128f53d93..a47a635e50 100644 --- a/src/test/regress/expected/partition_split.out +++ b/src/test/regress/expected/partition_split.out @@ -1584,6 +1584,65 @@ ALTER TABLE t SPLIT PARTITION tp_0 INTO (PARTITION tp_0 FOR VALUES FROM (0) TO (1), PARTITION tp_1 FOR VALUES FROM (1) TO (2)); DROP TABLE t; +-- Check defaults and constraints of new partitions +CREATE TABLE t_bigint ( + b bigint, + i int DEFAULT (3+10), + j int DEFAULT 101, + k int GENERATED ALWAYS AS (b+10) STORED +) +PARTITION BY RANGE (b); +CREATE TABLE t_bigint_default PARTITION OF t_bigint DEFAULT; +-- Show defaults/constraints before SPLIT PARTITION +\d+ t_bigint + Partitioned table "partition_split_schema.t_bigint" + Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------------------------------------+---------+--------------+------------- + b | bigint | | | | plain | | + i | integer | | | 3 + 10 | plain | | + j | integer | | | 101 | plain | | + k | integer | | | generated always as ((b + 10)) stored | plain | | +Partition key: RANGE (b) +Partitions: t_bigint_default DEFAULT + +\d+ t_bigint_default + Table "partition_split_schema.t_bigint_default" + Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------------------------------------+---------+--------------+------------- + b | bigint | | | | plain | | + i | integer | | | 3 + 10 | plain | | + j | integer | | | 101 | plain | | + k | integer | | | generated always as ((b + 10)) stored | plain | | +Partition of: t_bigint DEFAULT +No partition constraint + +ALTER TABLE t_bigint SPLIT PARTITION t_bigint_default INTO + (PARTITION t_bigint_01_10 FOR VALUES FROM (0) TO (10), + PARTITION t_bigint_default DEFAULT); +-- Show defaults/constraints after SPLIT PARTITION +\d+ t_bigint_default + Table "partition_split_schema.t_bigint_default" + Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------------------------------------+---------+--------------+------------- + b | bigint | | | | plain | | + i | integer | | | 3 + 10 | plain | | + j | integer | | | 101 | plain | | + k | integer | | | generated always as ((b + 10)) stored | plain | | +Partition of: t_bigint DEFAULT +Partition constraint: (NOT ((b IS NOT NULL) AND ((b >= '0'::bigint) AND (b < '10'::bigint)))) + +\d+ t_bigint_01_10 + Table "partition_split_schema.t_bigint_01_10" + Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------------------------------------+---------+--------------+------------- + b | bigint | | | | plain | | + i | integer | | | 3 + 10 | plain | | + j | integer | | | 101 | plain | | + k | integer | | | generated always as ((b + 10)) stored | plain | | +Partition of: t_bigint FOR VALUES FROM ('0') TO ('10') +Partition constraint: ((b IS NOT NULL) AND (b >= '0'::bigint) AND (b < '10'::bigint)) + +DROP TABLE t_bigint; RESET search_path; -- DROP SCHEMA partition_split_schema; diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql index ef5ea07f74..e185458e4e 100644 --- a/src/test/regress/sql/partition_split.sql +++ b/src/test/regress/sql/partition_split.sql @@ -955,6 +955,27 @@ ALTER TABLE t SPLIT PARTITION tp_0 INTO PARTITION tp_1 FOR VALUES FROM (1) TO (2)); DROP TABLE t; +-- Check defaults and constraints of new partitions +CREATE TABLE t_bigint ( + b bigint, + i int DEFAULT (3+10), + j int DEFAULT 101, + k int GENERATED ALWAYS AS (b+10) STORED +) +PARTITION BY RANGE (b); +CREATE TABLE t_bigint_default PARTITION OF t_bigint DEFAULT; +-- Show defaults/constraints before SPLIT PARTITION +\d+ t_bigint +\d+ t_bigint_default +ALTER TABLE t_bigint SPLIT PARTITION t_bigint_default INTO + (PARTITION t_bigint_01_10 FOR VALUES FROM (0) TO (10), + PARTITION t_bigint_default DEFAULT); +-- Show defaults/constraints after SPLIT PARTITION +\d+ t_bigint_default +\d+ t_bigint_01_10 +DROP TABLE t_bigint; + + RESET search_path; -- -- 2.40.1.windows.1