On 2022-Oct-21, Japin Li wrote: > Does there an error about forget the LIST partition?
Of course. https://cirrus-ci.com/build/4721735111540736 This is what you get for moving cases around at the last minute ... Fixed, thanks. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
>From dc345f3cb70c335421f29a6867438ed4bb95bd91 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Thu, 20 Oct 2022 12:29:18 +0200 Subject: [PATCH v2] have gram.y resolve partition strategy names --- src/backend/commands/tablecmds.c | 35 +++++++++------------------ src/backend/parser/gram.y | 22 ++++++++++++++++- src/backend/partitioning/partbounds.c | 26 ++++---------------- src/backend/utils/cache/partcache.c | 6 +++++ src/include/nodes/parsenodes.h | 15 ++++++------ src/include/partitioning/partbounds.h | 2 +- src/include/utils/partcache.h | 2 +- 7 files changed, 53 insertions(+), 55 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 20135ef1b0..e07fd747f7 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -605,9 +605,10 @@ static void RangeVarCallbackForDropRelation(const RangeVar *rel, Oid relOid, Oid oldRelOid, void *arg); static void RangeVarCallbackForAlterRelation(const RangeVar *rv, Oid relid, Oid oldrelid, void *arg); -static PartitionSpec *transformPartitionSpec(Relation rel, PartitionSpec *partspec, char *strategy); +static PartitionSpec *transformPartitionSpec(Relation rel, PartitionSpec *partspec); static void ComputePartitionAttrs(ParseState *pstate, Relation rel, List *partParams, AttrNumber *partattrs, - List **partexprs, Oid *partopclass, Oid *partcollation, char strategy); + List **partexprs, Oid *partopclass, Oid *partcollation, + PartitionStrategy strategy); static void CreateInheritance(Relation child_rel, Relation parent_rel); static void RemoveInheritance(Relation child_rel, Relation parent_rel, bool expect_detached); @@ -1122,7 +1123,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, if (partitioned) { ParseState *pstate; - char strategy; int partnatts; AttrNumber partattrs[PARTITION_MAX_KEYS]; Oid partopclass[PARTITION_MAX_KEYS]; @@ -1147,14 +1147,14 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, * and CHECK constraints, we could not have done the transformation * earlier. */ - stmt->partspec = transformPartitionSpec(rel, stmt->partspec, - &strategy); + stmt->partspec = transformPartitionSpec(rel, stmt->partspec); ComputePartitionAttrs(pstate, rel, stmt->partspec->partParams, partattrs, &partexprs, partopclass, - partcollation, strategy); + partcollation, stmt->partspec->strategy); - StorePartitionKey(rel, strategy, partnatts, partattrs, partexprs, + StorePartitionKey(rel, stmt->partspec->strategy, partnatts, partattrs, + partexprs, partopclass, partcollation); /* make it all visible */ @@ -17132,10 +17132,10 @@ RangeVarCallbackForAlterRelation(const RangeVar *rv, Oid relid, Oid oldrelid, /* * Transform any expressions present in the partition key * - * Returns a transformed PartitionSpec, as well as the strategy code + * Returns a transformed PartitionSpec. */ static PartitionSpec * -transformPartitionSpec(Relation rel, PartitionSpec *partspec, char *strategy) +transformPartitionSpec(Relation rel, PartitionSpec *partspec) { PartitionSpec *newspec; ParseState *pstate; @@ -17148,21 +17148,8 @@ transformPartitionSpec(Relation rel, PartitionSpec *partspec, char *strategy) newspec->partParams = NIL; newspec->location = partspec->location; - /* Parse partitioning strategy name */ - if (pg_strcasecmp(partspec->strategy, "hash") == 0) - *strategy = PARTITION_STRATEGY_HASH; - else if (pg_strcasecmp(partspec->strategy, "list") == 0) - *strategy = PARTITION_STRATEGY_LIST; - else if (pg_strcasecmp(partspec->strategy, "range") == 0) - *strategy = PARTITION_STRATEGY_RANGE; - else - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("unrecognized partitioning strategy \"%s\"", - partspec->strategy))); - /* Check valid number of columns for strategy */ - if (*strategy == PARTITION_STRATEGY_LIST && + if (partspec->strategy == PARTITION_STRATEGY_LIST && list_length(partspec->partParams) != 1) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), @@ -17208,7 +17195,7 @@ transformPartitionSpec(Relation rel, PartitionSpec *partspec, char *strategy) static void ComputePartitionAttrs(ParseState *pstate, Relation rel, List *partParams, AttrNumber *partattrs, List **partexprs, Oid *partopclass, Oid *partcollation, - char strategy) + PartitionStrategy strategy) { int attn; ListCell *lc; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 737bd2d06d..6ca23f88c4 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -213,6 +213,7 @@ static void SplitColQualList(List *qualList, static void processCASbits(int cas_bits, int location, const char *constrType, bool *deferrable, bool *initdeferred, bool *not_valid, bool *no_inherit, core_yyscan_t yyscanner); +static PartitionStrategy parsePartitionStrategy(char *strategy); static void preprocess_pubobj_list(List *pubobjspec_list, core_yyscan_t yyscanner); static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); @@ -4357,7 +4358,7 @@ PartitionSpec: PARTITION BY ColId '(' part_params ')' { PartitionSpec *n = makeNode(PartitionSpec); - n->strategy = $3; + n->strategy = parsePartitionStrategy($3); n->partParams = $5; n->location = @1; @@ -18414,6 +18415,25 @@ processCASbits(int cas_bits, int location, const char *constrType, } } +/* + * Parse a user-supplied partition strategy string into parse node + * PartitionStrategy representation, or die trying. + */ +static PartitionStrategy +parsePartitionStrategy(char *strategy) +{ + if (pg_strcasecmp(strategy, "list") == 0) + return PARTITION_STRATEGY_LIST; + else if (pg_strcasecmp(strategy, "range") == 0) + return PARTITION_STRATEGY_RANGE; + else if (pg_strcasecmp(strategy, "hash") == 0) + return PARTITION_STRATEGY_HASH; + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("unrecognized partitioning strategy \"%s\"", + strategy))); +} + /* * Process pubobjspec_list to check for errors in any of the objects and * convert PUBLICATIONOBJ_CONTINUATION into appropriate PublicationObjSpecType. diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c index 0823fa7b1d..7b5cd55e80 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -270,10 +270,6 @@ get_qual_from_partbound(Relation parent, PartitionBoundSpec *spec) Assert(spec->strategy == PARTITION_STRATEGY_RANGE); my_qual = get_qual_for_range(parent, spec, false); break; - - default: - elog(ERROR, "unexpected partition strategy: %d", - (int) key->strategy); } return my_qual; @@ -338,11 +334,6 @@ partition_bounds_create(PartitionBoundSpec **boundspecs, int nparts, case PARTITION_STRATEGY_RANGE: return create_range_bounds(boundspecs, nparts, key, mapping); - - default: - elog(ERROR, "unexpected partition strategy: %d", - (int) key->strategy); - break; } Assert(false); @@ -1181,11 +1172,9 @@ partition_bounds_merge(int partnatts, jointype, outer_parts, inner_parts); - default: - elog(ERROR, "unexpected partition strategy: %d", - (int) outer_rel->boundinfo->strategy); - return NULL; /* keep compiler quiet */ + Assert(false); + return NULL; } } @@ -2892,8 +2881,7 @@ partitions_are_ordered(PartitionBoundInfo boundinfo, Bitmapset *live_parts) return true; break; - default: - /* HASH, or some other strategy */ + case PARTITION_STRATEGY_HASH: break; } @@ -3241,10 +3229,6 @@ check_new_partition_bound(char *relname, Relation parent, break; } - - default: - elog(ERROR, "unexpected partition strategy: %d", - (int) key->strategy); } if (overlap) @@ -3980,8 +3964,8 @@ make_partition_op_expr(PartitionKey key, int keynum, key->partcollation[keynum]); break; - default: - elog(ERROR, "invalid partitioning strategy"); + case PARTITION_STRATEGY_HASH: + Assert(false); break; } diff --git a/src/backend/utils/cache/partcache.c b/src/backend/utils/cache/partcache.c index afa99c5d03..8f3e411476 100644 --- a/src/backend/utils/cache/partcache.c +++ b/src/backend/utils/cache/partcache.c @@ -115,6 +115,12 @@ RelationBuildPartitionKey(Relation relation) key->strategy = form->partstrat; key->partnatts = form->partnatts; + /* Validate partition strategy code */ + if (key->strategy != PARTITION_STRATEGY_HASH && + key->strategy != PARTITION_STRATEGY_LIST && + key->strategy != PARTITION_STRATEGY_RANGE) + elog(ERROR, "invalid partition strategy \"%c\"", key->strategy); + /* * We can rely on the first variable-length attribute being mapped to the * relevant field of the catalog's C struct, because all previous diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 633e7671b3..99cba300f3 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -827,6 +827,13 @@ typedef struct PartitionElem int location; /* token location, or -1 if unknown */ } PartitionElem; +typedef enum PartitionStrategy +{ + PARTITION_STRATEGY_LIST = 'l', + PARTITION_STRATEGY_RANGE = 'r', + PARTITION_STRATEGY_HASH = 'h' +} PartitionStrategy; + /* * PartitionSpec - parse-time representation of a partition key specification * @@ -835,17 +842,11 @@ typedef struct PartitionElem typedef struct PartitionSpec { NodeTag type; - char *strategy; /* partitioning strategy ('hash', 'list' or - * 'range') */ + PartitionStrategy strategy; List *partParams; /* List of PartitionElems */ int location; /* token location, or -1 if unknown */ } PartitionSpec; -/* Internal codes for partitioning strategies */ -#define PARTITION_STRATEGY_HASH 'h' -#define PARTITION_STRATEGY_LIST 'l' -#define PARTITION_STRATEGY_RANGE 'r' - /* * PartitionBoundSpec - a partition bound specification * diff --git a/src/include/partitioning/partbounds.h b/src/include/partitioning/partbounds.h index 1f5b706d83..9af4b1b5c7 100644 --- a/src/include/partitioning/partbounds.h +++ b/src/include/partitioning/partbounds.h @@ -78,7 +78,7 @@ struct RelOptInfo; /* avoid including pathnodes.h here */ */ typedef struct PartitionBoundInfoData { - char strategy; /* hash, list or range? */ + PartitionStrategy strategy; /* hash, list or range? */ int ndatums; /* Length of the datums[] array */ Datum **datums; PartitionRangeDatumKind **kind; /* The kind of each range bound datum; diff --git a/src/include/utils/partcache.h b/src/include/utils/partcache.h index 3394e1fcdb..c01ce48241 100644 --- a/src/include/utils/partcache.h +++ b/src/include/utils/partcache.h @@ -23,7 +23,7 @@ */ typedef struct PartitionKeyData { - char strategy; /* partitioning strategy */ + PartitionStrategy strategy; /* partitioning strategy */ int16 partnatts; /* number of columns in the partition key */ AttrNumber *partattrs; /* attribute numbers of columns in the * partition key or 0 if it's an expr */ -- 2.30.2