Resolve partition strategy during early parsing

This has little practical value, but there's no reason to let the
partition strategy names travel through DDL as strings.

Reviewed-by: Japin Li <japinli@hotmail.com>
Discussion: https://postgr.es/m/20221021093216.ffupd7epy2mytkux@alvherre.pgsql

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index bc90185da6bd159e9eb60b827ccd3a15f36f3935..140ecb0cbfad344a2b3f94c978e52dbad56dcabe 100644 (file)
--- 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 737bd2d06d54dbbc4d9a3e3696ece68f53d68124..6ca23f88c4f5b8ea5ec6e17f7cfea9ad269fdb76 100644 (file)
--- 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 0823fa7b1dbab756ac17fd90d7d26699dd1494f6..29643fb4ab575ba0dc1d9fad60f95a5aec374eb7 100644 (file)
--- 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,12 +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 */
        }
+
+       return NULL;
 }
 
 /*
@@ -2892,8 +2880,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 +3228,6 @@ check_new_partition_bound(char *relname, Relation parent,
 
                                break;
                        }
-
-               default:
-                       elog(ERROR, "unexpected partition strategy: %d",
-                                (int) key->strategy);
        }
 
        if (overlap)
@@ -3980,8 +3963,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 afa99c5d0364504e87c29dde34863ab81c5d2a53..e32f4b961199c3784c6c7de32e072925d6063069 100644 (file)
--- 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_LIST &&
+               key->strategy != PARTITION_STRATEGY_RANGE &&
+               key->strategy != PARTITION_STRATEGY_HASH)
+               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 7e7ad3f7e47fc6981d9b26195fd1964943edc1dd..7caff62af7f348f790ea2a6050393b64e2306608 100644 (file)
--- 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 1f5b706d8318f569ee39e726455b30a22d1bb0ce..a32fc6c149da534faf8f6a9427b85299c49ef9ee 100644 (file)
--- 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 3394e1fcdbaa1b49b439ab5d938144cdf13621f6..c45fc3a3ac11805d1111c4738b3ada39841aad6f 100644 (file)
--- a/src/include/utils/partcache.h
+++ b/src/include/utils/partcache.h
@@ -13,6 +13,7 @@
 
 #include "access/attnum.h"
 #include "fmgr.h"
+#include "nodes/parsenodes.h"
 #include "nodes/pg_list.h"
 #include "nodes/primnodes.h"
 #include "partitioning/partdefs.h"
@@ -23,7 +24,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 */