Get rid of ParseState.p_value_substitute; use a columnref hook instead.

I noticed that p_value_substitute, which is a single-purpose kluge I added
in 2002 (commit b0422b215), could be replaced by having domainAddConstraint
install a parser hook that looks for the name "value".  The parser hook
code only dates back to 2009, so it's not surprising that we had to kluge
this in 2002, but we can do it more cleanly now.

diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index c07babbe9726991e980a064b1f3f37199965cdc0..3ff6cbca56f43caecf0c93cc33950484c1779003 100644 (file)
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -104,6 +104,8 @@ static char *domainAddConstraint(Oid domainOid, Oid domainNamespace,
                                        Oid baseTypeOid,
                                        int typMod, Constraint *constr,
                                        char *domainName, ObjectAddress *constrAddr);
+static Node *replace_domain_constraint_value(ParseState *pstate,
+                                                               ColumnRef *cref);
 
 
 /*
@@ -3022,7 +3024,8 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid,
        domVal->collation = get_typcollation(baseTypeOid);
        domVal->location = -1;          /* will be set when/if used */
 
-       pstate->p_value_substitute = (Node *) domVal;
+       pstate->p_pre_columnref_hook = replace_domain_constraint_value;
+       pstate->p_ref_hook_state = (void *) domVal;
 
        expr = transformExpr(pstate, constr->raw_expr, EXPR_KIND_DOMAIN_CHECK);
 
@@ -3099,6 +3102,35 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid,
        return ccbin;
 }
 
+/* Parser pre_columnref_hook for domain CHECK constraint parsing */
+static Node *
+replace_domain_constraint_value(ParseState *pstate, ColumnRef *cref)
+{
+       /*
+        * Check for a reference to "value", and if that's what it is, replace
+        * with a CoerceToDomainValue as prepared for us by domainAddConstraint.
+        * (We handle VALUE as a name, not a keyword, to avoid breaking a lot of
+        * applications that have used VALUE as a column name in the past.)
+        */
+       if (list_length(cref->fields) == 1)
+       {
+               Node       *field1 = (Node *) linitial(cref->fields);
+               char       *colname;
+
+               Assert(IsA(field1, String));
+               colname = strVal(field1);
+               if (strcmp(colname, "value") == 0)
+               {
+                       CoerceToDomainValue *domVal = copyObject(pstate->p_ref_hook_state);
+
+                       /* Propagate location knowledge, if any */
+                       domVal->location = cref->location;
+                       return (Node *) domVal;
+               }
+       }
+       return NULL;
+}
+
 
 /*
  * Execute ALTER TYPE RENAME

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 38c2535b65f27393e509e14cbfb407c92e81555e..f62e45f8ac8fbdc24c1379ae421336fd10c2bed5 100644 (file)
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -577,27 +577,6 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
                                        /*
                                         * Not known as a column of any range-table entry.
                                         *
-                                        * Consider the possibility that it's VALUE in a domain
-                                        * check expression.  (We handle VALUE as a name, not a
-                                        * keyword, to avoid breaking a lot of applications that
-                                        * have used VALUE as a column name in the past.)
-                                        */
-                                       if (pstate->p_value_substitute != NULL &&
-                                               strcmp(colname, "value") == 0)
-                                       {
-                                               node = (Node *) copyObject(pstate->p_value_substitute);
-
-                                               /*
-                                                * Try to propagate location knowledge.  This should
-                                                * be extended if p_value_substitute can ever take on
-                                                * other node types.
-                                                */
-                                               if (IsA(node, CoerceToDomainValue))
-                                                       ((CoerceToDomainValue *) node)->location = cref->location;
-                                               break;
-                                       }
-
-                                       /*
                                         * Try to find the name as a relation.  Note that only
                                         * relations already entered into the rangetable will be
                                         * recognized.

diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 92b2cb35ce7c970ae3613f8188f576b1cd24c986..7cdf142df630e4a9700afcc1f26fdf424cd94502 100644 (file)
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -149,9 +149,6 @@ typedef Node *(*CoerceParamHook) (ParseState *pstate, Param *param,
  * p_locked_from_parent: true if parent query level applies FOR UPDATE/SHARE
  * to this subquery as a whole.
  *
- * p_value_substitute: replacement for VALUE references, if we're parsing
- * a domain CHECK constraint.
- *
  * p_hasAggs, p_hasWindowFuncs, etc: true if we've found any of the indicated
  * constructs in the query.
  *
@@ -184,7 +181,6 @@ struct ParseState
        List       *p_locking_clause;           /* raw FOR UPDATE/FOR SHARE info */
        bool            p_locked_from_parent;   /* parent has marked this subquery
                                                                                 * with FOR UPDATE/FOR SHARE */
-       Node       *p_value_substitute;         /* what to replace VALUE with, if any */
 
        /* Flags telling about things found in the query: */
        bool            p_hasAggs;