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 */