On 30.08.21 04:13, Kyotaro Horiguchi wrote:
However, the patch adds:

+typedef struct Null
+{
+       NodeTag         type;
+       char       *val;
+} Null;

which doesn't seem to be used anywhere. Is that a leftoverf from an
intermediate development stage?

+1 Looks like so, it can be simply removed.

fixed

0002:
   there's an "integer Value node" in gram.y: 7776.

fixed

-                       n = makeFloatConst(v->val.str, location);
+                       n = (Node *) makeFloatConst(castNode(Float, v)->val, 
location);

makeFloatConst is Node* so the cast doesn't seem needed. The same can
be said for Int and String Consts.  This looks like a confustion with
makeInteger and friends.

fixed

+       else if (IsA(obj, Integer))
+               _outInteger(str, (Integer *) obj);
+       else if (IsA(obj, Float))
+               _outFloat(str, (Float *) obj);

I felt that the type enames are a bit confusing as they might be too
generic, or too close with the corresponding binary types.


-       Node       *arg;                        /* a (Value *) or a (TypeName 
*) */
+       Node       *arg;

Mmm. It's a bit pity that we lose the generic name for the value nodes.

Not sure what you mean here.
From 01773e4aeedea35fa2f23af08c30cfbda6fe8b27 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 25 Aug 2021 13:41:09 +0200
Subject: [PATCH v2 1/2] Remove useless casts

Casting the argument of strVal() to (Value *) is useless, since
strVal() already does that.

Most code didn't do that anyway; this was apparently just a style that
snuck into certain files.
---
 src/backend/catalog/objectaddress.c | 20 ++++++++++----------
 src/backend/commands/alter.c        | 16 ++++++++--------
 src/backend/commands/comment.c      |  2 +-
 src/backend/commands/dropcmds.c     | 16 ++++++++--------
 src/backend/commands/statscmds.c    |  2 +-
 5 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/src/backend/catalog/objectaddress.c 
b/src/backend/catalog/objectaddress.c
index deaabaeae9..bc2a4ccdde 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -2386,7 +2386,7 @@ check_object_ownership(Oid roleid, ObjectType objtype, 
ObjectAddress address,
                case OBJECT_DATABASE:
                        if (!pg_database_ownercheck(address.objectId, roleid))
                                aclcheck_error(ACLCHECK_NOT_OWNER, objtype,
-                                                          strVal((Value *) 
object));
+                                                          strVal(object));
                        break;
                case OBJECT_TYPE:
                case OBJECT_DOMAIN:
@@ -2433,7 +2433,7 @@ check_object_ownership(Oid roleid, ObjectType objtype, 
ObjectAddress address,
                case OBJECT_SCHEMA:
                        if (!pg_namespace_ownercheck(address.objectId, roleid))
                                aclcheck_error(ACLCHECK_NOT_OWNER, objtype,
-                                                          strVal((Value *) 
object));
+                                                          strVal(object));
                        break;
                case OBJECT_COLLATION:
                        if (!pg_collation_ownercheck(address.objectId, roleid))
@@ -2448,27 +2448,27 @@ check_object_ownership(Oid roleid, ObjectType objtype, 
ObjectAddress address,
                case OBJECT_EXTENSION:
                        if (!pg_extension_ownercheck(address.objectId, roleid))
                                aclcheck_error(ACLCHECK_NOT_OWNER, objtype,
-                                                          strVal((Value *) 
object));
+                                                          strVal(object));
                        break;
                case OBJECT_FDW:
                        if 
(!pg_foreign_data_wrapper_ownercheck(address.objectId, roleid))
                                aclcheck_error(ACLCHECK_NOT_OWNER, objtype,
-                                                          strVal((Value *) 
object));
+                                                          strVal(object));
                        break;
                case OBJECT_FOREIGN_SERVER:
                        if (!pg_foreign_server_ownercheck(address.objectId, 
roleid))
                                aclcheck_error(ACLCHECK_NOT_OWNER, objtype,
-                                                          strVal((Value *) 
object));
+                                                          strVal(object));
                        break;
                case OBJECT_EVENT_TRIGGER:
                        if (!pg_event_trigger_ownercheck(address.objectId, 
roleid))
                                aclcheck_error(ACLCHECK_NOT_OWNER, objtype,
-                                                          strVal((Value *) 
object));
+                                                          strVal(object));
                        break;
                case OBJECT_LANGUAGE:
                        if (!pg_language_ownercheck(address.objectId, roleid))
                                aclcheck_error(ACLCHECK_NOT_OWNER, objtype,
-                                                          strVal((Value *) 
object));
+                                                          strVal(object));
                        break;
                case OBJECT_OPCLASS:
                        if (!pg_opclass_ownercheck(address.objectId, roleid))
@@ -2508,12 +2508,12 @@ check_object_ownership(Oid roleid, ObjectType objtype, 
ObjectAddress address,
                case OBJECT_PUBLICATION:
                        if (!pg_publication_ownercheck(address.objectId, 
roleid))
                                aclcheck_error(ACLCHECK_NOT_OWNER, objtype,
-                                                          strVal((Value *) 
object));
+                                                          strVal(object));
                        break;
                case OBJECT_SUBSCRIPTION:
                        if (!pg_subscription_ownercheck(address.objectId, 
roleid))
                                aclcheck_error(ACLCHECK_NOT_OWNER, objtype,
-                                                          strVal((Value *) 
object));
+                                                          strVal(object));
                        break;
                case OBJECT_TRANSFORM:
                        {
@@ -2527,7 +2527,7 @@ check_object_ownership(Oid roleid, ObjectType objtype, 
ObjectAddress address,
                case OBJECT_TABLESPACE:
                        if (!pg_tablespace_ownercheck(address.objectId, roleid))
                                aclcheck_error(ACLCHECK_NOT_OWNER, objtype,
-                                                          strVal((Value *) 
object));
+                                                          strVal(object));
                        break;
                case OBJECT_TSDICTIONARY:
                        if (!pg_ts_dict_ownercheck(address.objectId, roleid))
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 29249498a9..c47d54e96b 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -501,7 +501,7 @@ ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt,
        switch (stmt->objectType)
        {
                case OBJECT_EXTENSION:
-                       address = AlterExtensionNamespace(strVal((Value *) 
stmt->object), stmt->newschema,
+                       address = AlterExtensionNamespace(strVal(stmt->object), 
stmt->newschema,
                                                                                
          oldSchemaAddr ? &oldNspOid : NULL);
                        break;
 
@@ -837,10 +837,10 @@ ExecAlterOwnerStmt(AlterOwnerStmt *stmt)
        switch (stmt->objectType)
        {
                case OBJECT_DATABASE:
-                       return AlterDatabaseOwner(strVal((Value *) 
stmt->object), newowner);
+                       return AlterDatabaseOwner(strVal(stmt->object), 
newowner);
 
                case OBJECT_SCHEMA:
-                       return AlterSchemaOwner(strVal((Value *) stmt->object), 
newowner);
+                       return AlterSchemaOwner(strVal(stmt->object), newowner);
 
                case OBJECT_TYPE:
                case OBJECT_DOMAIN:             /* same as TYPE */
@@ -848,23 +848,23 @@ ExecAlterOwnerStmt(AlterOwnerStmt *stmt)
                        break;
 
                case OBJECT_FDW:
-                       return AlterForeignDataWrapperOwner(strVal((Value *) 
stmt->object),
+                       return 
AlterForeignDataWrapperOwner(strVal(stmt->object),
                                                                                
                newowner);
 
                case OBJECT_FOREIGN_SERVER:
-                       return AlterForeignServerOwner(strVal((Value *) 
stmt->object),
+                       return AlterForeignServerOwner(strVal(stmt->object),
                                                                                
   newowner);
 
                case OBJECT_EVENT_TRIGGER:
-                       return AlterEventTriggerOwner(strVal((Value *) 
stmt->object),
+                       return AlterEventTriggerOwner(strVal(stmt->object),
                                                                                
  newowner);
 
                case OBJECT_PUBLICATION:
-                       return AlterPublicationOwner(strVal((Value *) 
stmt->object),
+                       return AlterPublicationOwner(strVal(stmt->object),
                                                                                
 newowner);
 
                case OBJECT_SUBSCRIPTION:
-                       return AlterSubscriptionOwner(strVal((Value *) 
stmt->object),
+                       return AlterSubscriptionOwner(strVal(stmt->object),
                                                                                
  newowner);
 
                        /* Generic cases */
diff --git a/src/backend/commands/comment.c b/src/backend/commands/comment.c
index 834f1a2a3f..d4943e374a 100644
--- a/src/backend/commands/comment.c
+++ b/src/backend/commands/comment.c
@@ -52,7 +52,7 @@ CommentObject(CommentStmt *stmt)
         */
        if (stmt->objtype == OBJECT_DATABASE)
        {
-               char       *database = strVal((Value *) stmt->object);
+               char       *database = strVal(stmt->object);
 
                if (!OidIsValid(get_database_oid(database, true)))
                {
diff --git a/src/backend/commands/dropcmds.c b/src/backend/commands/dropcmds.c
index 97e5e9a765..4e545adf95 100644
--- a/src/backend/commands/dropcmds.c
+++ b/src/backend/commands/dropcmds.c
@@ -255,7 +255,7 @@ does_not_exist_skipping(ObjectType objtype, Node *object)
        {
                case OBJECT_ACCESS_METHOD:
                        msg = gettext_noop("access method \"%s\" does not 
exist, skipping");
-                       name = strVal((Value *) object);
+                       name = strVal(object);
                        break;
                case OBJECT_TYPE:
                case OBJECT_DOMAIN:
@@ -285,7 +285,7 @@ does_not_exist_skipping(ObjectType objtype, Node *object)
                        break;
                case OBJECT_SCHEMA:
                        msg = gettext_noop("schema \"%s\" does not exist, 
skipping");
-                       name = strVal((Value *) object);
+                       name = strVal(object);
                        break;
                case OBJECT_STATISTIC_EXT:
                        if (!schema_does_not_exist_skipping(castNode(List, 
object), &msg, &name))
@@ -324,7 +324,7 @@ does_not_exist_skipping(ObjectType objtype, Node *object)
                        break;
                case OBJECT_EXTENSION:
                        msg = gettext_noop("extension \"%s\" does not exist, 
skipping");
-                       name = strVal((Value *) object);
+                       name = strVal(object);
                        break;
                case OBJECT_FUNCTION:
                        {
@@ -392,7 +392,7 @@ does_not_exist_skipping(ObjectType objtype, Node *object)
                        }
                case OBJECT_LANGUAGE:
                        msg = gettext_noop("language \"%s\" does not exist, 
skipping");
-                       name = strVal((Value *) object);
+                       name = strVal(object);
                        break;
                case OBJECT_CAST:
                        {
@@ -434,7 +434,7 @@ does_not_exist_skipping(ObjectType objtype, Node *object)
                        break;
                case OBJECT_EVENT_TRIGGER:
                        msg = gettext_noop("event trigger \"%s\" does not 
exist, skipping");
-                       name = strVal((Value *) object);
+                       name = strVal(object);
                        break;
                case OBJECT_RULE:
                        if (!owningrel_does_not_exist_skipping(castNode(List, 
object), &msg, &name))
@@ -447,11 +447,11 @@ does_not_exist_skipping(ObjectType objtype, Node *object)
                        break;
                case OBJECT_FDW:
                        msg = gettext_noop("foreign-data wrapper \"%s\" does 
not exist, skipping");
-                       name = strVal((Value *) object);
+                       name = strVal(object);
                        break;
                case OBJECT_FOREIGN_SERVER:
                        msg = gettext_noop("server \"%s\" does not exist, 
skipping");
-                       name = strVal((Value *) object);
+                       name = strVal(object);
                        break;
                case OBJECT_OPCLASS:
                        {
@@ -479,7 +479,7 @@ does_not_exist_skipping(ObjectType objtype, Node *object)
                        break;
                case OBJECT_PUBLICATION:
                        msg = gettext_noop("publication \"%s\" does not exist, 
skipping");
-                       name = strVal((Value *) object);
+                       name = strVal(object);
                        break;
                default:
                        elog(ERROR, "unrecognized object type: %d", (int) 
objtype);
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index 59369f8736..78917844de 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -335,7 +335,7 @@ CreateStatistics(CreateStatsStmt *stmt)
        build_mcv = false;
        foreach(cell, stmt->stat_types)
        {
-               char       *type = strVal((Value *) lfirst(cell));
+               char       *type = strVal(lfirst(cell));
 
                if (strcmp(type, "ndistinct") == 0)
                {
-- 
2.33.0

From 6eb5ddb6b46446731df38ac5288df2feb17efd7c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 7 Sep 2021 11:11:28 +0200
Subject: [PATCH v2 2/2] Remove Value node struct

The Value node struct is a weird construct.  It is its own node type,
but most of the time, it actually has a node type of Integer, Float,
String, or BitString.  As a consequence, the struct name and the node
type don't match most of the time, and so it has to be treated
specially a lot.  There doesn't seem to be any value in the special
construct.  There is very little code that wants to accept all Value
variants but nothing else (and even if it did, this doesn't provide
any convenient way to check it), and most code wants either just one
particular node type (usually String), or it accepts a broader set of
node types besides just Value.

This change removes the Value struct and node type and replaces them
by separate Integer, Float, String, and BitString node types that are
proper node types and structs of their own and behave mostly like
normal node types.

Also, this removes the T_Null node tag, which was previously also a
possible variant of Value but wasn't actually used outside of the
Value contained in A_Const.  Replace that by an isnull field in
A_Const.
---
 contrib/postgres_fdw/postgres_fdw.c      |  12 +--
 src/backend/catalog/namespace.c          |   4 +-
 src/backend/catalog/objectaddress.c      |   6 +-
 src/backend/catalog/pg_enum.c            |   2 +-
 src/backend/commands/copy.c              |   5 +-
 src/backend/commands/define.c            |   9 +-
 src/backend/commands/tsearchcmds.c       |   4 +-
 src/backend/executor/nodeTableFuncscan.c |   2 +-
 src/backend/nodes/README                 |   2 +-
 src/backend/nodes/copyfuncs.c            | 107 ++++++++++++++---------
 src/backend/nodes/equalfuncs.c           |  55 +++++++-----
 src/backend/nodes/nodeFuncs.c            |   1 -
 src/backend/nodes/outfuncs.c             |  93 ++++++++++----------
 src/backend/nodes/value.c                |  32 +++----
 src/backend/parser/gram.y                |  97 ++++++++++----------
 src/backend/parser/parse_clause.c        |  17 ++--
 src/backend/parser/parse_cte.c           |   4 +-
 src/backend/parser/parse_expr.c          |  13 +--
 src/backend/parser/parse_node.c          |  50 ++++++-----
 src/backend/parser/parse_relation.c      |   6 +-
 src/backend/parser/parse_type.c          |  14 +--
 src/backend/parser/parse_utilcmd.c       |   4 +-
 src/backend/utils/adt/oid.c              |   2 +-
 src/backend/utils/adt/ruleutils.c        |   2 +-
 src/backend/utils/misc/guc.c             |   3 +-
 src/include/nodes/nodes.h                |   2 -
 src/include/nodes/parsenodes.h           |  66 ++++++++------
 src/include/nodes/primnodes.h            |   6 +-
 src/include/nodes/value.h                |  87 ++++++++++--------
 src/include/parser/parse_node.h          |   3 +-
 30 files changed, 373 insertions(+), 337 deletions(-)

diff --git a/contrib/postgres_fdw/postgres_fdw.c 
b/contrib/postgres_fdw/postgres_fdw.c
index 4bdab30a73..76d4fea21c 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -101,9 +101,9 @@ enum FdwModifyPrivateIndex
        FdwModifyPrivateUpdateSql,
        /* Integer list of target attribute numbers for INSERT/UPDATE */
        FdwModifyPrivateTargetAttnums,
-       /* Length till the end of VALUES clause (as an integer Value node) */
+       /* Length till the end of VALUES clause (as an Integer node) */
        FdwModifyPrivateLen,
-       /* has-returning flag (as an integer Value node) */
+       /* has-returning flag (as an Integer node) */
        FdwModifyPrivateHasReturning,
        /* Integer list of attribute numbers retrieved by RETURNING */
        FdwModifyPrivateRetrievedAttrs
@@ -122,11 +122,11 @@ enum FdwDirectModifyPrivateIndex
 {
        /* SQL statement to execute remotely (as a String node) */
        FdwDirectModifyPrivateUpdateSql,
-       /* has-returning flag (as an integer Value node) */
+       /* has-returning flag (as an Integer node) */
        FdwDirectModifyPrivateHasReturning,
        /* Integer list of attribute numbers retrieved by RETURNING */
        FdwDirectModifyPrivateRetrievedAttrs,
-       /* set-processed flag (as an integer Value node) */
+       /* set-processed flag (as an Integer node) */
        FdwDirectModifyPrivateSetProcessed
 };
 
@@ -280,9 +280,9 @@ typedef struct PgFdwAnalyzeState
  */
 enum FdwPathPrivateIndex
 {
-       /* has-final-sort flag (as an integer Value node) */
+       /* has-final-sort flag (as an Integer node) */
        FdwPathPrivateHasFinalSort,
-       /* has-limit flag (as an integer Value node) */
+       /* has-limit flag (as an Integer node) */
        FdwPathPrivateHasLimit
 };
 
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index fd767fc5cf..4de8400fd0 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3026,7 +3026,7 @@ CheckSetNamespace(Oid oldNspOid, Oid nspOid)
 
 /*
  * QualifiedNameGetCreationNamespace
- *             Given a possibly-qualified name for an object (in List-of-Values
+ *             Given a possibly-qualified name for an object (in 
List-of-Strings
  *             format), determine what namespace the object should be created 
in.
  *             Also extract and return the object name (last component of 
list).
  *
@@ -3140,7 +3140,7 @@ makeRangeVarFromNameList(List *names)
  * This is used primarily to form error messages, and so we do not quote
  * the list elements, for the sake of legibility.
  *
- * In most scenarios the list elements should always be Value strings,
+ * In most scenarios the list elements should always be String values,
  * but we also allow A_Star for the convenience of ColumnRef processing.
  */
 char *
diff --git a/src/backend/catalog/objectaddress.c 
b/src/backend/catalog/objectaddress.c
index bc2a4ccdde..8c94939baa 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -851,7 +851,7 @@ const ObjectAddress InvalidObjectAddress =
 };
 
 static ObjectAddress get_object_address_unqualified(ObjectType objtype,
-                                                                               
                        Value *strval, bool missing_ok);
+                                                                               
                        String *strval, bool missing_ok);
 static ObjectAddress get_relation_by_qualified_name(ObjectType objtype,
                                                                                
                        List *object, Relation *relp,
                                                                                
                        LOCKMODE lockmode, bool missing_ok);
@@ -1011,7 +1011,7 @@ get_object_address(ObjectType objtype, Node *object,
                        case OBJECT_PUBLICATION:
                        case OBJECT_SUBSCRIPTION:
                                address = 
get_object_address_unqualified(objtype,
-                                                                               
                                 (Value *) object, missing_ok);
+                                                                               
                                 castNode(String, object), missing_ok);
                                break;
                        case OBJECT_TYPE:
                        case OBJECT_DOMAIN:
@@ -1244,7 +1244,7 @@ get_object_address_rv(ObjectType objtype, RangeVar *rel, 
List *object,
  */
 static ObjectAddress
 get_object_address_unqualified(ObjectType objtype,
-                                                          Value *strval, bool 
missing_ok)
+                                                          String *strval, bool 
missing_ok)
 {
        const char *name;
        ObjectAddress address;
diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.c
index f958f1541d..be1c5a5b0d 100644
--- a/src/backend/catalog/pg_enum.c
+++ b/src/backend/catalog/pg_enum.c
@@ -55,7 +55,7 @@ static int    sort_order_cmp(const void *p1, const void *p2);
  * EnumValuesCreate
  *             Create an entry in pg_enum for each of the supplied enum values.
  *
- * vals is a list of Value strings.
+ * vals is a list of String values.
  */
 void
 EnumValuesCreate(Oid enumTypeOid, List *vals)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 6b33951e0c..53f4853141 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -222,9 +222,8 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
                                {
                                        /*
                                         * Build the ColumnRef for each column. 
 The ColumnRef
-                                        * 'fields' property is a String 
'Value' node (see
-                                        * nodes/value.h) that corresponds to 
the column name
-                                        * respectively.
+                                        * 'fields' property is a String node 
that corresponds to
+                                        * the column name respectively.
                                         */
                                        cr = makeNode(ColumnRef);
                                        cr->fields = list_make1(lfirst(lc));
diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c
index aafd7554e4..19c317a472 100644
--- a/src/backend/commands/define.c
+++ b/src/backend/commands/define.c
@@ -58,12 +58,7 @@ defGetString(DefElem *def)
                case T_Integer:
                        return psprintf("%ld", (long) intVal(def->arg));
                case T_Float:
-
-                       /*
-                        * T_Float values are kept in string form, so this type 
cheat
-                        * works (and doesn't risk losing precision)
-                        */
-                       return strVal(def->arg);
+                       return castNode(Float, def->arg)->val;
                case T_String:
                        return strVal(def->arg);
                case T_TypeName:
@@ -206,7 +201,7 @@ defGetInt64(DefElem *def)
                         * strings.
                         */
                        return DatumGetInt64(DirectFunctionCall1(int8in,
-                                                                               
                         CStringGetDatum(strVal(def->arg))));
+                                                                               
                         CStringGetDatum(castNode(Float, def->arg)->val)));
                default:
                        ereport(ERROR,
                                        (errcode(ERRCODE_SYNTAX_ERROR),
diff --git a/src/backend/commands/tsearchcmds.c 
b/src/backend/commands/tsearchcmds.c
index e06fb32b3d..c47a05d10d 100644
--- a/src/backend/commands/tsearchcmds.c
+++ b/src/backend/commands/tsearchcmds.c
@@ -1179,7 +1179,7 @@ getTokenTypes(Oid prsId, List *tokennames)
        i = 0;
        foreach(tn, tokennames)
        {
-               Value      *val = (Value *) lfirst(tn);
+               String     *val = lfirst_node(String, tn);
                bool            found = false;
                int                     j;
 
@@ -1395,7 +1395,7 @@ DropConfigurationMapping(AlterTSConfigurationStmt *stmt,
        i = 0;
        foreach(c, stmt->tokentype)
        {
-               Value      *val = (Value *) lfirst(c);
+               String     *val = lfirst_node(String, c);
                bool            found = false;
 
                ScanKeyInit(&skey[0],
diff --git a/src/backend/executor/nodeTableFuncscan.c 
b/src/backend/executor/nodeTableFuncscan.c
index 4d7eca4ace..27dfa1b956 100644
--- a/src/backend/executor/nodeTableFuncscan.c
+++ b/src/backend/executor/nodeTableFuncscan.c
@@ -364,7 +364,7 @@ tfuncInitialize(TableFuncScanState *tstate, ExprContext 
*econtext, Datum doc)
        forboth(lc1, tstate->ns_uris, lc2, tstate->ns_names)
        {
                ExprState  *expr = (ExprState *) lfirst(lc1);
-               Value      *ns_node = (Value *) lfirst(lc2);
+               String     *ns_node = lfirst_node(String, lc2);
                char       *ns_uri;
                char       *ns_name;
 
diff --git a/src/backend/nodes/README b/src/backend/nodes/README
index dcd66d7243..d066ac5c61 100644
--- a/src/backend/nodes/README
+++ b/src/backend/nodes/README
@@ -28,7 +28,7 @@ FILES IN THIS DIRECTORY (src/backend/nodes/)
        list.c          - generic list support
        params.c        - Param support
        tidbitmap.c     - TIDBitmap support
-       value.c         - support for Value nodes
+       value.c         - support for value nodes
 
 FILES IN src/include/nodes/
 
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index e308de170e..83ec2a369e 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2729,25 +2729,30 @@ _copyA_Const(const A_Const *from)
 {
        A_Const    *newnode = makeNode(A_Const);
 
-       /* This part must duplicate _copyValue */
-       COPY_SCALAR_FIELD(val.type);
-       switch (from->val.type)
+       COPY_SCALAR_FIELD(isnull);
+       if (!from->isnull)
        {
-               case T_Integer:
-                       COPY_SCALAR_FIELD(val.val.ival);
-                       break;
-               case T_Float:
-               case T_String:
-               case T_BitString:
-                       COPY_STRING_FIELD(val.val.str);
-                       break;
-               case T_Null:
-                       /* nothing to do */
-                       break;
-               default:
-                       elog(ERROR, "unrecognized node type: %d",
-                                (int) from->val.type);
-                       break;
+               /* This part must duplicate other _copy*() functions. */
+               COPY_SCALAR_FIELD(val.node.type);
+               switch (nodeTag(&from->val))
+               {
+                       case T_Integer:
+                               COPY_SCALAR_FIELD(val.ival.val);
+                               break;
+                       case T_Float:
+                               COPY_STRING_FIELD(val.fval.val);
+                               break;
+                       case T_String:
+                               COPY_STRING_FIELD(val.sval.val);
+                               break;
+                       case T_BitString:
+                               COPY_STRING_FIELD(val.bsval.val);
+                               break;
+                       default:
+                               elog(ERROR, "unrecognized node type: %d",
+                                        (int) nodeTag(&from->val));
+                               break;
+               }
        }
 
        COPY_LOCATION_FIELD(location);
@@ -4892,32 +4897,43 @@ _copyExtensibleNode(const ExtensibleNode *from)
  *                                     value.h copy functions
  * ****************************************************************
  */
-static Value *
-_copyValue(const Value *from)
+static Integer *
+_copyInteger(const Integer *from)
 {
-       Value      *newnode = makeNode(Value);
+       Integer    *newnode = makeNode(Integer);
 
-       /* See also _copyAConst when changing this code! */
+       COPY_SCALAR_FIELD(val);
+
+       return newnode;
+}
+
+static Float *
+_copyFloat(const Float *from)
+{
+       Float      *newnode = makeNode(Float);
+
+       COPY_STRING_FIELD(val);
+
+       return newnode;
+}
+
+static String *
+_copyString(const String *from)
+{
+       String     *newnode = makeNode(String);
+
+       COPY_STRING_FIELD(val);
+
+       return newnode;
+}
+
+static BitString *
+_copyBitString(const BitString *from)
+{
+       BitString   *newnode = makeNode(BitString);
+
+       COPY_STRING_FIELD(val);
 
-       COPY_SCALAR_FIELD(type);
-       switch (from->type)
-       {
-               case T_Integer:
-                       COPY_SCALAR_FIELD(val.ival);
-                       break;
-               case T_Float:
-               case T_String:
-               case T_BitString:
-                       COPY_STRING_FIELD(val.str);
-                       break;
-               case T_Null:
-                       /* nothing to do */
-                       break;
-               default:
-                       elog(ERROR, "unrecognized node type: %d",
-                                (int) from->type);
-                       break;
-       }
        return newnode;
 }
 
@@ -5314,11 +5330,16 @@ copyObjectImpl(const void *from)
                         * VALUE NODES
                         */
                case T_Integer:
+                       retval = _copyInteger(from);
+                       break;
                case T_Float:
+                       retval = _copyFloat(from);
+                       break;
                case T_String:
+                       retval = _copyString(from);
+                       break;
                case T_BitString:
-               case T_Null:
-                       retval = _copyValue(from);
+                       retval = _copyBitString(from);
                        break;
 
                        /*
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 99440b40be..231380ccf7 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2409,7 +2409,7 @@ _equalParamRef(const ParamRef *a, const ParamRef *b)
 static bool
 _equalA_Const(const A_Const *a, const A_Const *b)
 {
-       if (!equal(&a->val, &b->val))   /* hack for in-line Value field */
+       if (!equal(&a->val, &b->val))   /* hack for in-line val field */
                return false;
        COMPARE_LOCATION_FIELD(location);
 
@@ -3089,27 +3089,33 @@ _equalList(const List *a, const List *b)
  */
 
 static bool
-_equalValue(const Value *a, const Value *b)
+_equalInteger(const Integer *a, const Integer *b)
 {
-       COMPARE_SCALAR_FIELD(type);
+       COMPARE_SCALAR_FIELD(val);
 
-       switch (a->type)
-       {
-               case T_Integer:
-                       COMPARE_SCALAR_FIELD(val.ival);
-                       break;
-               case T_Float:
-               case T_String:
-               case T_BitString:
-                       COMPARE_STRING_FIELD(val.str);
-                       break;
-               case T_Null:
-                       /* nothing to do */
-                       break;
-               default:
-                       elog(ERROR, "unrecognized node type: %d", (int) 
a->type);
-                       break;
-       }
+       return true;
+}
+
+static bool
+_equalFloat(const Float *a, const Float *b)
+{
+       COMPARE_STRING_FIELD(val);
+
+       return true;
+}
+
+static bool
+_equalString(const String *a, const String *b)
+{
+       COMPARE_STRING_FIELD(val);
+
+       return true;
+}
+
+static bool
+_equalBitString(const BitString *a, const BitString *b)
+{
+       COMPARE_STRING_FIELD(val);
 
        return true;
 }
@@ -3337,11 +3343,16 @@ equal(const void *a, const void *b)
                        break;
 
                case T_Integer:
+                       retval = _equalInteger(a, b);
+                       break;
                case T_Float:
+                       retval = _equalFloat(a, b);
+                       break;
                case T_String:
+                       retval = _equalString(a, b);
+                       break;
                case T_BitString:
-               case T_Null:
-                       retval = _equalValue(a, b);
+                       retval = _equalBitString(a, b);
                        break;
 
                        /*
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index ff3dcc7b18..e276264882 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -3537,7 +3537,6 @@ raw_expression_tree_walker(Node *node,
                case T_Float:
                case T_String:
                case T_BitString:
-               case T_Null:
                case T_ParamRef:
                case T_A_Const:
                case T_A_Star:
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 87561cbb6f..36e618611f 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -3414,44 +3414,39 @@ _outA_Expr(StringInfo str, const A_Expr *node)
 }
 
 static void
-_outValue(StringInfo str, const Value *node)
+_outInteger(StringInfo str, const Integer *node)
 {
-       switch (node->type)
-       {
-               case T_Integer:
-                       appendStringInfo(str, "%d", node->val.ival);
-                       break;
-               case T_Float:
+       appendStringInfo(str, "%d", node->val);
+}
 
-                       /*
-                        * We assume the value is a valid numeric literal and 
so does not
-                        * need quoting.
-                        */
-                       appendStringInfoString(str, node->val.str);
-                       break;
-               case T_String:
-
-                       /*
-                        * We use outToken to provide escaping of the string's 
content,
-                        * but we don't want it to do anything with an empty 
string.
-                        */
-                       appendStringInfoChar(str, '"');
-                       if (node->val.str[0] != '\0')
-                               outToken(str, node->val.str);
-                       appendStringInfoChar(str, '"');
-                       break;
-               case T_BitString:
-                       /* internal representation already has leading 'b' */
-                       appendStringInfoString(str, node->val.str);
-                       break;
-               case T_Null:
-                       /* this is seen only within A_Const, not in transformed 
trees */
-                       appendStringInfoString(str, "NULL");
-                       break;
-               default:
-                       elog(ERROR, "unrecognized node type: %d", (int) 
node->type);
-                       break;
-       }
+static void
+_outFloat(StringInfo str, const Float *node)
+{
+       /*
+        * We assume the value is a valid numeric literal and so does not
+        * need quoting.
+        */
+       appendStringInfoString(str, node->val);
+}
+
+static void
+_outString(StringInfo str, const String *node)
+{
+       /*
+        * We use outToken to provide escaping of the string's content,
+        * but we don't want it to do anything with an empty string.
+        */
+       appendStringInfoChar(str, '"');
+       if (node->val[0] != '\0')
+               outToken(str, node->val);
+       appendStringInfoChar(str, '"');
+}
+
+static void
+_outBitString(StringInfo str, const BitString *node)
+{
+       /* internal representation already has leading 'b' */
+       appendStringInfoString(str, node->val);
 }
 
 static void
@@ -3491,8 +3486,13 @@ _outA_Const(StringInfo str, const A_Const *node)
 {
        WRITE_NODE_TYPE("A_CONST");
 
-       appendStringInfoString(str, " :val ");
-       _outValue(str, &(node->val));
+       if (node->isnull)
+               appendStringInfoString(str, "NULL");
+       else
+       {
+               appendStringInfoString(str, " :val ");
+               outNode(str, &node->val);
+       }
        WRITE_LOCATION_FIELD(location);
 }
 
@@ -3835,14 +3835,15 @@ outNode(StringInfo str, const void *obj)
                appendStringInfoString(str, "<>");
        else if (IsA(obj, List) || IsA(obj, IntList) || IsA(obj, OidList))
                _outList(str, obj);
-       else if (IsA(obj, Integer) ||
-                        IsA(obj, Float) ||
-                        IsA(obj, String) ||
-                        IsA(obj, BitString))
-       {
-               /* nodeRead does not want to see { } around these! */
-               _outValue(str, obj);
-       }
+       /* nodeRead does not want to see { } around these! */
+       else if (IsA(obj, Integer))
+               _outInteger(str, (Integer *) obj);
+       else if (IsA(obj, Float))
+               _outFloat(str, (Float *) obj);
+       else if (IsA(obj, String))
+               _outString(str, (String *) obj);
+       else if (IsA(obj, BitString))
+               _outBitString(str, (BitString *) obj);
        else
        {
                appendStringInfoChar(str, '{');
diff --git a/src/backend/nodes/value.c b/src/backend/nodes/value.c
index 15e6d26752..515f93c223 100644
--- a/src/backend/nodes/value.c
+++ b/src/backend/nodes/value.c
@@ -1,7 +1,7 @@
 /*-------------------------------------------------------------------------
  *
  * value.c
- *       implementation of Value nodes
+ *       implementation of value nodes
  *
  *
  * Copyright (c) 2003-2021, PostgreSQL Global Development Group
@@ -14,18 +14,17 @@
  */
 #include "postgres.h"
 
-#include "nodes/parsenodes.h"
+#include "nodes/value.h"
 
 /*
  *     makeInteger
  */
-Value *
+Integer *
 makeInteger(int i)
 {
-       Value      *v = makeNode(Value);
+       Integer    *v = makeNode(Integer);
 
-       v->type = T_Integer;
-       v->val.ival = i;
+       v->val = i;
        return v;
 }
 
@@ -34,13 +33,12 @@ makeInteger(int i)
  *
  * Caller is responsible for passing a palloc'd string.
  */
-Value *
+Float *
 makeFloat(char *numericStr)
 {
-       Value      *v = makeNode(Value);
+       Float      *v = makeNode(Float);
 
-       v->type = T_Float;
-       v->val.str = numericStr;
+       v->val = numericStr;
        return v;
 }
 
@@ -49,13 +47,12 @@ makeFloat(char *numericStr)
  *
  * Caller is responsible for passing a palloc'd string.
  */
-Value *
+String *
 makeString(char *str)
 {
-       Value      *v = makeNode(Value);
+       String     *v = makeNode(String);
 
-       v->type = T_String;
-       v->val.str = str;
+       v->val = str;
        return v;
 }
 
@@ -64,12 +61,11 @@ makeString(char *str)
  *
  * Caller is responsible for passing a palloc'd string.
  */
-Value *
+BitString *
 makeBitString(char *str)
 {
-       Value      *v = makeNode(Value);
+       BitString  *v = makeNode(BitString);
 
-       v->type = T_BitString;
-       v->val.str = str;
+       v->val = str;
        return v;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 6a0f46505c..e3068a374e 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -166,7 +166,7 @@ static Node *makeIntConst(int val, int location);
 static Node *makeFloatConst(char *str, int location);
 static Node *makeBitStringConst(char *str, int location);
 static Node *makeNullAConst(int location);
-static Node *makeAConst(Value *v, int location);
+static Node *makeAConst(Node *v, int location);
 static Node *makeBoolAConst(bool state, int location);
 static RoleSpec *makeRoleSpec(RoleSpecType type, int location);
 static void check_qualified_name(List *names, core_yyscan_t yyscanner);
@@ -183,7 +183,7 @@ static void insertSelectOptions(SelectStmt *stmt,
                                                                core_yyscan_t 
yyscanner);
 static Node *makeSetOp(SetOperation op, bool all, Node *larg, Node *rarg);
 static Node *doNegate(Node *n, int location);
-static void doNegateFloat(Value *v);
+static void doNegateFloat(Float *v);
 static Node *makeAndExpr(Node *lexpr, Node *rexpr, int location);
 static Node *makeOrExpr(Node *lexpr, Node *rexpr, int location);
 static Node *makeNotExpr(Node *expr, int location);
@@ -228,7 +228,6 @@ static Node *makeRecursiveViewSelect(char *relname, List 
*aliases, Node *query);
        OnCommitAction          oncommit;
        List                            *list;
        Node                            *node;
-       Value                           *value;
        ObjectType                      objtype;
        TypeName                        *typnam;
        FunctionParameter   *fun_param;
@@ -351,7 +350,7 @@ static Node *makeRecursiveViewSelect(char *relname, List 
*aliases, Node *query);
 %type <boolean> TriggerForSpec TriggerForType
 %type <ival>   TriggerActionTime
 %type <list>   TriggerEvents TriggerOneEvent
-%type <value>  TriggerFuncArg
+%type <node>   TriggerFuncArg
 %type <node>   TriggerWhen
 %type <str>            TransitionRelName
 %type <boolean>        TransitionRowOrTable TransitionOldOrNew
@@ -508,7 +507,7 @@ static Node *makeRecursiveViewSelect(char *relname, List 
*aliases, Node *query);
 %type <list>   when_clause_list
 %type <node>   opt_search_clause opt_cycle_clause
 %type <ival>   sub_type opt_materialized
-%type <value>  NumericOnly
+%type <node>   NumericOnly
 %type <list>   NumericOnly_list
 %type <alias>  alias_clause opt_alias_clause opt_alias_clause_for_join_using
 %type <list>   func_alias_clause
@@ -1696,7 +1695,7 @@ zone_value:
                                        if ($3 != NIL)
                                        {
                                                A_Const *n = (A_Const *) 
linitial($3);
-                                               if ((n->val.val.ival & 
~(INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE))) != 0)
+                                               if ((n->val.ival.val & 
~(INTERVAL_MASK(HOUR) | INTERVAL_MASK(MINUTE))) != 0)
                                                        ereport(ERROR,
                                                                        
(errcode(ERRCODE_SYNTAX_ERROR),
                                                                         
errmsg("time zone interval must be HOUR or HOUR TO MINUTE"),
@@ -4459,14 +4458,15 @@ opt_by:         BY
          ;
 
 NumericOnly:
-                       FCONST                                                  
        { $$ = makeFloat($1); }
-                       | '+' FCONST                                            
{ $$ = makeFloat($2); }
+                       FCONST                                                  
        { $$ = (Node *) makeFloat($1); }
+                       | '+' FCONST                                            
{ $$ = (Node *) makeFloat($2); }
                        | '-' FCONST
                                {
-                                       $$ = makeFloat($2);
-                                       doNegateFloat($$);
+                                       Float *f = makeFloat($2);
+                                       doNegateFloat(f);
+                                       $$ = (Node *) f;
                                }
-                       | SignedIconst                                          
{ $$ = makeInteger($1); }
+                       | SignedIconst                                          
{ $$ = (Node *) makeInteger($1); }
                ;
 
 NumericOnly_list:      NumericOnly                                             
{ $$ = list_make1($1); }
@@ -5535,11 +5535,11 @@ TriggerFuncArgs:
 TriggerFuncArg:
                        Iconst
                                {
-                                       $$ = makeString(psprintf("%d", $1));
+                                       $$ = (Node *) makeString(psprintf("%d", 
$1));
                                }
-                       | FCONST                                                
                { $$ = makeString($1); }
-                       | Sconst                                                
                { $$ = makeString($1); }
-                       | ColLabel                                              
                { $$ = makeString($1); }
+                       | FCONST                                                
                { $$ = (Node *) makeString($1); }
+                       | Sconst                                                
                { $$ = (Node *) makeString($1); }
+                       | ColLabel                                              
                { $$ = (Node *) makeString($1); }
                ;
 
 OptConstrFromTable:
@@ -7773,8 +7773,8 @@ aggr_arg: func_arg
  *
  * The return value of this production is a two-element list, in which the
  * first item is a sublist of FunctionParameter nodes (with any duplicate
- * VARIADIC item already dropped, as per above) and the second is an integer
- * Value node, containing -1 if there was no ORDER BY and otherwise the number
+ * VARIADIC item already dropped, as per above) and the second is an Integer
+ * node, containing -1 if there was no ORDER BY and otherwise the number
  * of argument declarations before the ORDER BY.  (If this number is equal
  * to the first sublist's length, then we dropped a duplicate VARIADIC item.)
  * This representation is passed as-is to CREATE AGGREGATE; for operations
@@ -16520,11 +16520,11 @@ makeStringConst(char *str, int location)
 {
        A_Const *n = makeNode(A_Const);
 
-       n->val.type = T_String;
-       n->val.val.str = str;
+       n->val.sval.type = T_String;
+       n->val.sval.val = str;
        n->location = location;
 
-       return (Node *)n;
+   return (Node *)n;
 }
 
 static Node *
@@ -16540,11 +16540,11 @@ makeIntConst(int val, int location)
 {
        A_Const *n = makeNode(A_Const);
 
-       n->val.type = T_Integer;
-       n->val.val.ival = val;
+       n->val.ival.type = T_Integer;
+       n->val.ival.val = val;
        n->location = location;
 
-       return (Node *)n;
+   return (Node *)n;
 }
 
 static Node *
@@ -16552,11 +16552,11 @@ makeFloatConst(char *str, int location)
 {
        A_Const *n = makeNode(A_Const);
 
-       n->val.type = T_Float;
-       n->val.val.str = str;
+       n->val.fval.type = T_Float;
+       n->val.fval.val = str;
        n->location = location;
 
-       return (Node *)n;
+   return (Node *)n;
 }
 
 static Node *
@@ -16564,11 +16564,11 @@ makeBitStringConst(char *str, int location)
 {
        A_Const *n = makeNode(A_Const);
 
-       n->val.type = T_BitString;
-       n->val.val.str = str;
+       n->val.bsval.type = T_BitString;
+       n->val.bsval.val = str;
        n->location = location;
 
-       return (Node *)n;
+   return (Node *)n;
 }
 
 static Node *
@@ -16576,30 +16576,30 @@ makeNullAConst(int location)
 {
        A_Const *n = makeNode(A_Const);
 
-       n->val.type = T_Null;
+       n->isnull = true;
        n->location = location;
 
        return (Node *)n;
 }
 
 static Node *
-makeAConst(Value *v, int location)
+makeAConst(Node *v, int location)
 {
        Node *n;
 
        switch (v->type)
        {
                case T_Float:
-                       n = makeFloatConst(v->val.str, location);
+                       n = makeFloatConst(castNode(Float, v)->val, location);
                        break;
 
                case T_Integer:
-                       n = makeIntConst(v->val.ival, location);
+                       n = makeIntConst(castNode(Integer, v)->val, location);
                        break;
 
                case T_String:
                default:
-                       n = makeStringConst(v->val.str, location);
+                       n = makeStringConst(castNode(String, v)->val, location);
                        break;
        }
 
@@ -16612,13 +16612,9 @@ makeAConst(Value *v, int location)
 static Node *
 makeBoolAConst(bool state, int location)
 {
-       A_Const *n = makeNode(A_Const);
-
-       n->val.type = T_String;
-       n->val.val.str = (state ? "t" : "f");
-       n->location = location;
-
-       return makeTypeCast((Node *)n, SystemTypeName("bool"), -1);
+       return makeStringConstCast((state ? "t" : "f"),
+                                                          location,
+                                                          
SystemTypeName("bool"));
 }
 
 /* makeRoleSpec
@@ -16733,7 +16729,7 @@ makeOrderedSetArgs(List *directargs, List *orderedargs,
                                   core_yyscan_t yyscanner)
 {
        FunctionParameter *lastd = (FunctionParameter *) llast(directargs);
-       Value      *ndirectargs;
+       Integer    *ndirectargs;
 
        /* No restriction unless last direct arg is VARIADIC */
        if (lastd->mode == FUNC_PARAM_VARIADIC)
@@ -16890,14 +16886,14 @@ doNegate(Node *n, int location)
                /* report the constant's location as that of the '-' sign */
                con->location = location;
 
-               if (con->val.type == T_Integer)
+               if (IsA(&con->val, Integer))
                {
-                       con->val.val.ival = -con->val.val.ival;
+                       con->val.ival.val = -con->val.ival.val;
                        return n;
                }
-               if (con->val.type == T_Float)
+               if (IsA(&con->val, Float))
                {
-                       doNegateFloat(&con->val);
+                       doNegateFloat(&con->val.fval);
                        return n;
                }
        }
@@ -16906,17 +16902,16 @@ doNegate(Node *n, int location)
 }
 
 static void
-doNegateFloat(Value *v)
+doNegateFloat(Float *v)
 {
-       char   *oldval = v->val.str;
+       char   *oldval = v->val;
 
-       Assert(IsA(v, Float));
        if (*oldval == '+')
                oldval++;
        if (*oldval == '-')
-               v->val.str = oldval+1;  /* just strip the '-' */
+               v->val = oldval+1;      /* just strip the '-' */
        else
-               v->val.str = psprintf("-%s", oldval);
+               v->val = psprintf("-%s", oldval);
 }
 
 static Node *
diff --git a/src/backend/parser/parse_clause.c 
b/src/backend/parser/parse_clause.c
index b3f151d33b..078029ba1f 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -852,7 +852,7 @@ transformRangeTableFunc(ParseState *pstate, RangeTableFunc 
*rtf)
                        {
                                foreach(lc2, ns_names)
                                {
-                                       Value      *ns_node = (Value *) 
lfirst(lc2);
+                                       String     *ns_node = 
lfirst_node(String, lc2);
 
                                        if (ns_node == NULL)
                                                continue;
@@ -1240,7 +1240,7 @@ transformFromClauseItem(ParseState *pstate, Node *n,
                        foreach(lx, l_colnames)
                        {
                                char       *l_colname = strVal(lfirst(lx));
-                               Value      *m_name = NULL;
+                               String     *m_name = NULL;
 
                                if (l_colname[0] == '\0')
                                        continue;       /* ignore dropped 
columns */
@@ -1785,7 +1785,7 @@ transformLimitClause(ParseState *pstate, Node *clause,
         * unadorned NULL that's not accepted back by the grammar.
         */
        if (exprKind == EXPR_KIND_LIMIT && limitOption == 
LIMIT_OPTION_WITH_TIES &&
-               IsA(clause, A_Const) && ((A_Const *) clause)->val.type == 
T_Null)
+               IsA(clause, A_Const) && castNode(A_Const, clause)->isnull)
                ereport(ERROR,
                                
(errcode(ERRCODE_INVALID_ROW_COUNT_IN_LIMIT_CLAUSE),
                                 errmsg("row count cannot be null in FETCH 
FIRST ... WITH TIES clause")));
@@ -1998,20 +1998,19 @@ findTargetlistEntrySQL92(ParseState *pstate, Node 
*node, List **tlist,
        }
        if (IsA(node, A_Const))
        {
-               Value      *val = &((A_Const *) node)->val;
-               int                     location = ((A_Const *) node)->location;
+               A_Const    *aconst = castNode(A_Const, node);
                int                     targetlist_pos = 0;
                int                     target_pos;
 
-               if (!IsA(val, Integer))
+               if (!IsA(&aconst->val, Integer))
                        ereport(ERROR,
                                        (errcode(ERRCODE_SYNTAX_ERROR),
                        /* translator: %s is name of a SQL construct, eg ORDER 
BY */
                                         errmsg("non-integer constant in %s",
                                                        
ParseExprKindName(exprKind)),
-                                        parser_errposition(pstate, location)));
+                                        parser_errposition(pstate, 
aconst->location)));
 
-               target_pos = intVal(val);
+               target_pos = intVal(&aconst->val);
                foreach(tl, *tlist)
                {
                        TargetEntry *tle = (TargetEntry *) lfirst(tl);
@@ -2031,7 +2030,7 @@ findTargetlistEntrySQL92(ParseState *pstate, Node *node, 
List **tlist,
                /* translator: %s is name of a SQL construct, eg ORDER BY */
                                 errmsg("%s position %d is not in select list",
                                                ParseExprKindName(exprKind), 
target_pos),
-                                parser_errposition(pstate, location)));
+                                parser_errposition(pstate, aconst->location)));
        }
 
        /*
diff --git a/src/backend/parser/parse_cte.c b/src/backend/parser/parse_cte.c
index f6ae96333a..2f51caf76c 100644
--- a/src/backend/parser/parse_cte.c
+++ b/src/backend/parser/parse_cte.c
@@ -393,7 +393,7 @@ analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
 
                foreach(lc, cte->search_clause->search_col_list)
                {
-                       Value      *colname = lfirst(lc);
+                       String     *colname = lfirst_node(String, lc);
 
                        if (!list_member(cte->ctecolnames, colname))
                                ereport(ERROR,
@@ -428,7 +428,7 @@ analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
 
                foreach(lc, cte->cycle_clause->cycle_col_list)
                {
-                       Value      *colname = lfirst(lc);
+                       String     *colname = lfirst_node(String, lc);
 
                        if (!list_member(cte->ctecolnames, colname))
                                ereport(ERROR,
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index f928c32311..2d1a477154 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -130,13 +130,8 @@ transformExprRecurse(ParseState *pstate, Node *expr)
                        break;
 
                case T_A_Const:
-                       {
-                               A_Const    *con = (A_Const *) expr;
-                               Value      *val = &con->val;
-
-                               result = (Node *) make_const(pstate, val, 
con->location);
-                               break;
-                       }
+                       result = (Node *) make_const(pstate, (A_Const *) expr);
+                       break;
 
                case T_A_Indirection:
                        result = transformIndirection(pstate, (A_Indirection *) 
expr);
@@ -855,7 +850,7 @@ exprIsNullConstant(Node *arg)
        {
                A_Const    *con = (A_Const *) arg;
 
-               if (con->val.type == T_Null)
+               if (con->isnull)
                        return true;
        }
        return false;
@@ -1626,7 +1621,7 @@ transformCaseExpr(ParseState *pstate, CaseExpr *c)
        {
                A_Const    *n = makeNode(A_Const);
 
-               n->val.type = T_Null;
+               n->isnull = true;
                n->location = -1;
                defresult = (Node *) n;
        }
diff --git a/src/backend/parser/parse_node.c b/src/backend/parser/parse_node.c
index 17c900da31..8cfe6f67c0 100644
--- a/src/backend/parser/parse_node.c
+++ b/src/backend/parser/parse_node.c
@@ -333,7 +333,7 @@ transformContainerSubscripts(ParseState *pstate,
 /*
  * make_const
  *
- *     Convert a Value node (as returned by the grammar) to a Const node
+ *     Convert an A_Const node (as returned by the grammar) to a Const node
  *     of the "natural" type for the constant.  Note that this routine is
  *     only used when there is no explicit cast for the constant, so we
  *     have to guess what type is wanted.
@@ -349,7 +349,7 @@ transformContainerSubscripts(ParseState *pstate,
  *     too many examples that fail if we try.
  */
 Const *
-make_const(ParseState *pstate, Value *value, int location)
+make_const(ParseState *pstate, A_Const *aconst)
 {
        Const      *con;
        Datum           val;
@@ -359,10 +359,24 @@ make_const(ParseState *pstate, Value *value, int location)
        bool            typebyval;
        ParseCallbackState pcbstate;
 
-       switch (nodeTag(value))
+       if (aconst->isnull)
+       {
+               /* return a null const */
+               con = makeConst(UNKNOWNOID,
+                                               -1,
+                                               InvalidOid,
+                                               -2,
+                                               (Datum) 0,
+                                               true,
+                                               false);
+               con->location = aconst->location;
+               return con;
+       }
+
+       switch (nodeTag(&aconst->val))
        {
                case T_Integer:
-                       val = Int32GetDatum(intVal(value));
+                       val = Int32GetDatum(aconst->val.ival.val);
 
                        typeid = INT4OID;
                        typelen = sizeof(int32);
@@ -371,7 +385,7 @@ make_const(ParseState *pstate, Value *value, int location)
 
                case T_Float:
                        /* could be an oversize integer as well as a float ... 
*/
-                       if (scanint8(strVal(value), true, &val64))
+                       if (scanint8(aconst->val.fval.val, true, &val64))
                        {
                                /*
                                 * It might actually fit in int32. Probably 
only INT_MIN can
@@ -399,9 +413,9 @@ make_const(ParseState *pstate, Value *value, int location)
                        else
                        {
                                /* arrange to report location if numeric_in() 
fails */
-                               setup_parser_errposition_callback(&pcbstate, 
pstate, location);
+                               setup_parser_errposition_callback(&pcbstate, 
pstate, aconst->location);
                                val = DirectFunctionCall3(numeric_in,
-                                                                               
  CStringGetDatum(strVal(value)),
+                                                                               
  CStringGetDatum(aconst->val.fval.val),
                                                                                
  ObjectIdGetDatum(InvalidOid),
                                                                                
  Int32GetDatum(-1));
                                cancel_parser_errposition_callback(&pcbstate);
@@ -418,7 +432,7 @@ make_const(ParseState *pstate, Value *value, int location)
                         * We assume here that UNKNOWN's internal 
representation is the
                         * same as CSTRING
                         */
-                       val = CStringGetDatum(strVal(value));
+                       val = CStringGetDatum(aconst->val.sval.val);
 
                        typeid = UNKNOWNOID;    /* will be coerced later */
                        typelen = -2;           /* cstring-style varwidth type 
*/
@@ -427,9 +441,9 @@ make_const(ParseState *pstate, Value *value, int location)
 
                case T_BitString:
                        /* arrange to report location if bit_in() fails */
-                       setup_parser_errposition_callback(&pcbstate, pstate, 
location);
+                       setup_parser_errposition_callback(&pcbstate, pstate, 
aconst->location);
                        val = DirectFunctionCall3(bit_in,
-                                                                         
CStringGetDatum(strVal(value)),
+                                                                         
CStringGetDatum(aconst->val.bsval.val),
                                                                          
ObjectIdGetDatum(InvalidOid),
                                                                          
Int32GetDatum(-1));
                        cancel_parser_errposition_callback(&pcbstate);
@@ -438,20 +452,8 @@ make_const(ParseState *pstate, Value *value, int location)
                        typebyval = false;
                        break;
 
-               case T_Null:
-                       /* return a null const */
-                       con = makeConst(UNKNOWNOID,
-                                                       -1,
-                                                       InvalidOid,
-                                                       -2,
-                                                       (Datum) 0,
-                                                       true,
-                                                       false);
-                       con->location = location;
-                       return con;
-
                default:
-                       elog(ERROR, "unrecognized node type: %d", (int) 
nodeTag(value));
+                       elog(ERROR, "unrecognized node type: %d", (int) 
nodeTag(&aconst->val));
                        return NULL;            /* keep compiler quiet */
        }
 
@@ -462,7 +464,7 @@ make_const(ParseState *pstate, Value *value, int location)
                                        val,
                                        false,
                                        typebyval);
-       con->location = location;
+       con->location = aconst->location;
 
        return con;
 }
diff --git a/src/backend/parser/parse_relation.c 
b/src/backend/parser/parse_relation.c
index 7465919044..c5c3f26ecf 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -1140,7 +1140,7 @@ buildRelationAliases(TupleDesc tupdesc, Alias *alias, 
Alias *eref)
        for (varattno = 0; varattno < maxattrs; varattno++)
        {
                Form_pg_attribute attr = TupleDescAttr(tupdesc, varattno);
-               Value      *attrname;
+               String     *attrname;
 
                if (attr->attisdropped)
                {
@@ -1153,7 +1153,7 @@ buildRelationAliases(TupleDesc tupdesc, Alias *alias, 
Alias *eref)
                else if (aliaslc)
                {
                        /* Use the next user-supplied alias */
-                       attrname = (Value *) lfirst(aliaslc);
+                       attrname = lfirst_node(String, aliaslc);
                        aliaslc = lnext(aliaslist, aliaslc);
                        alias->colnames = lappend(alias->colnames, attrname);
                }
@@ -3052,7 +3052,7 @@ expandNSItemVars(ParseNamespaceItem *nsitem,
        colindex = 0;
        foreach(lc, nsitem->p_names->colnames)
        {
-               Value      *colnameval = (Value *) lfirst(lc);
+               String     *colnameval = lfirst(lc);
                const char *colname = strVal(colnameval);
                ParseNamespaceColumn *nscol = nsitem->p_nscolumns + colindex;
 
diff --git a/src/backend/parser/parse_type.c b/src/backend/parser/parse_type.c
index abe131ebeb..31b07ad5ae 100644
--- a/src/backend/parser/parse_type.c
+++ b/src/backend/parser/parse_type.c
@@ -382,13 +382,17 @@ typenameTypeMod(ParseState *pstate, const TypeName 
*typeName, Type typ)
 
                        if (IsA(&ac->val, Integer))
                        {
-                               cstr = psprintf("%ld", (long) ac->val.val.ival);
+                               cstr = psprintf("%ld", (long) ac->val.ival.val);
                        }
-                       else if (IsA(&ac->val, Float) ||
-                                        IsA(&ac->val, String))
+                       else if (IsA(&ac->val, Float))
                        {
-                               /* we can just use the str field directly. */
-                               cstr = ac->val.val.str;
+                               /* we can just use the string representation 
directly. */
+                               cstr = ac->val.fval.val;
+                       }
+                       else if (IsA(&ac->val, String))
+                       {
+                               /* we can just use the string representation 
directly. */
+                               cstr = ac->val.sval.val;
                        }
                }
                else if (IsA(tm, ColumnRef))
diff --git a/src/backend/parser/parse_utilcmd.c 
b/src/backend/parser/parse_utilcmd.c
index e5eefdbd43..1d3ee53244 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -602,8 +602,8 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef 
*column)
                 */
                qstring = quote_qualified_identifier(snamespace, sname);
                snamenode = makeNode(A_Const);
-               snamenode->val.type = T_String;
-               snamenode->val.val.str = qstring;
+               snamenode->val.node.type = T_String;
+               snamenode->val.sval.val = qstring;
                snamenode->location = -1;
                castnode = makeNode(TypeCast);
                castnode->typeName = SystemTypeName("regclass");
diff --git a/src/backend/utils/adt/oid.c b/src/backend/utils/adt/oid.c
index fd94e0c881..7be260663e 100644
--- a/src/backend/utils/adt/oid.c
+++ b/src/backend/utils/adt/oid.c
@@ -324,7 +324,7 @@ oidparse(Node *node)
                         * constants by the lexer.  Accept these if they are 
valid OID
                         * strings.
                         */
-                       return oidin_subr(strVal(node), NULL);
+                       return oidin_subr(castNode(Float, node)->val, NULL);
                default:
                        elog(ERROR, "unrecognized node type: %d", (int) 
nodeTag(node));
        }
diff --git a/src/backend/utils/adt/ruleutils.c 
b/src/backend/utils/adt/ruleutils.c
index 8ff4e5dc07..45fd339da9 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -10514,7 +10514,7 @@ get_tablefunc(TableFunc *tf, deparse_context *context, 
bool showimplicit)
                forboth(lc1, tf->ns_uris, lc2, tf->ns_names)
                {
                        Node       *expr = (Node *) lfirst(lc1);
-                       Value      *ns_node = (Value *) lfirst(lc2);
+                       String     *ns_node = lfirst_node(String, lc2);
 
                        if (!first)
                                appendStringInfoString(buf, ", ");
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 467b0fd6fe..27e54ccf4b 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -8289,7 +8289,7 @@ flatten_set_variable_args(const char *name, List *args)
                                break;
                        case T_Float:
                                /* represented as a string, so just copy it */
-                               appendStringInfoString(&buf, strVal(&con->val));
+                               appendStringInfoString(&buf, castNode(Float, 
&con->val)->val);
                                break;
                        case T_String:
                                val = strVal(&con->val);
@@ -8785,7 +8785,6 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool 
isTopLevel)
                                                         errmsg("SET LOCAL 
TRANSACTION SNAPSHOT is not implemented")));
 
                                WarnNoTransactionBlock(isTopLevel, "SET 
TRANSACTION");
-                               Assert(nodeTag(&con->val) == T_String);
                                ImportSnapshot(strVal(&con->val));
                        }
                        else
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 56d13ff022..a692eb7b09 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -291,12 +291,10 @@ typedef enum NodeTag
        /*
         * TAGS FOR VALUE NODES (value.h)
         */
-       T_Value,
        T_Integer,
        T_Float,
        T_String,
        T_BitString,
-       T_Null,
 
        /*
         * TAGS FOR LIST NODES (pg_list.h)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 743e5aa4f3..7b397e683d 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -218,7 +218,7 @@ typedef struct Query
 typedef struct TypeName
 {
        NodeTag         type;
-       List       *names;                      /* qualified name (list of 
Value strings) */
+       List       *names;                      /* qualified name (list of 
String nodes) */
        Oid                     typeOid;                /* type identified by 
OID */
        bool            setof;                  /* is a set? */
        bool            pct_type;               /* %TYPE specified? */
@@ -231,7 +231,7 @@ typedef struct TypeName
 /*
  * ColumnRef - specifies a reference to a column, or possibly a whole tuple
  *
- * The "fields" list must be nonempty.  It can contain string Value nodes
+ * The "fields" list must be nonempty.  It can contain String nodes
  * (representing names) and A_Star nodes (representing occurrence of a '*').
  * Currently, A_Star must appear only as the last list element --- the grammar
  * is responsible for enforcing this!
@@ -244,7 +244,7 @@ typedef struct TypeName
 typedef struct ColumnRef
 {
        NodeTag         type;
-       List       *fields;                     /* field names (Value strings) 
or A_Star */
+       List       *fields;                     /* field names (String nodes) 
or A_Star */
        int                     location;               /* token location, or 
-1 if unknown */
 } ColumnRef;
 
@@ -295,7 +295,19 @@ typedef struct A_Expr
 typedef struct A_Const
 {
        NodeTag         type;
-       Value           val;                    /* value (includes type info, 
see value.h) */
+       /*
+        * Value nodes are inline for performance.  You can treat 'val' as a 
node,
+        * as in IsA(&val, Integer).  'val' is not valid if isnull is true.
+        */
+       union ValUnion
+       {
+               Node            node;
+               Integer         ival;
+               Float           fval;
+               String          sval;
+               BitString       bsval;
+       }                       val;
+       bool            isnull;                 /* SQL NULL constant */
        int                     location;               /* token location, or 
-1 if unknown */
 } A_Const;
 
@@ -400,7 +412,7 @@ typedef struct A_Indices
  * A_Indirection - select a field and/or array element from an expression
  *
  * The indirection list can contain A_Indices nodes (representing
- * subscripting), string Value nodes (representing field selection --- the
+ * subscripting), String nodes (representing field selection --- the
  * string value is the name of the field to select), and A_Star nodes
  * (representing selection of all fields of a composite type).
  * For example, a complex selection operation like
@@ -744,7 +756,7 @@ typedef struct DefElem
        NodeTag         type;
        char       *defnamespace;       /* NULL if unqualified name */
        char       *defname;
-       Node       *arg;                        /* a (Value *) or a (TypeName 
*) */
+       Node       *arg;
        DefElemAction defaction;        /* unspecified action, or SET/ADD/DROP 
*/
        int                     location;               /* token location, or 
-1 if unknown */
 } DefElem;
@@ -2015,7 +2027,7 @@ typedef struct GrantStmt
        GrantTargetType targtype;       /* type of the grant target */
        ObjectType      objtype;                /* kind of object being 
operated on */
        List       *objects;            /* list of RangeVar nodes, 
ObjectWithArgs
-                                                                * nodes, or 
plain names (as Value strings) */
+                                                                * nodes, or 
plain names (as String values) */
        List       *privileges;         /* list of AccessPriv nodes */
        /* privileges == NIL denotes ALL PRIVILEGES */
        List       *grantees;           /* list of RoleSpec nodes */
@@ -2061,7 +2073,7 @@ typedef struct AccessPriv
 {
        NodeTag         type;
        char       *priv_name;          /* string name of privilege */
-       List       *cols;                       /* list of Value strings */
+       List       *cols;                       /* list of String */
 } AccessPriv;
 
 /* ----------------------
@@ -2070,7 +2082,7 @@ typedef struct AccessPriv
  * Note: because of the parsing ambiguity with the GRANT <privileges>
  * statement, granted_roles is a list of AccessPriv; the execution code
  * should complain if any column lists appear.  grantee_roles is a list
- * of role names, as Value strings.
+ * of role names, as String values.
  * ----------------------
  */
 typedef struct GrantRoleStmt
@@ -2531,7 +2543,7 @@ typedef struct CreateTrigStmt
        char       *trigname;           /* TRIGGER's name */
        RangeVar   *relation;           /* relation trigger is on */
        List       *funcname;           /* qual. name of function to call */
-       List       *args;                       /* list of (T_String) Values or 
NIL */
+       List       *args;                       /* list of String or NIL */
        bool            row;                    /* ROW/STATEMENT */
        /* timing uses the TRIGGER_TYPE bits defined in catalog/pg_trigger.h */
        int16           timing;                 /* BEFORE, AFTER, or INSTEAD */
@@ -2667,7 +2679,7 @@ typedef struct DefineStmt
        NodeTag         type;
        ObjectType      kind;                   /* aggregate, operator, type */
        bool            oldstyle;               /* hack to signal old CREATE 
AGG syntax */
-       List       *defnames;           /* qualified name (list of Value 
strings) */
+       List       *defnames;           /* qualified name (list of String) */
        List       *args;                       /* a list of TypeName (if 
needed) */
        List       *definition;         /* a list of DefElem */
        bool            if_not_exists;  /* just do nothing if it already 
exists? */
@@ -2681,7 +2693,7 @@ typedef struct DefineStmt
 typedef struct CreateDomainStmt
 {
        NodeTag         type;
-       List       *domainname;         /* qualified name (list of Value 
strings) */
+       List       *domainname;         /* qualified name (list of String) */
        TypeName   *typeName;           /* the base type */
        CollateClause *collClause;      /* untransformed COLLATE spec, if any */
        List       *constraints;        /* constraints (list of Constraint 
nodes) */
@@ -2694,7 +2706,7 @@ typedef struct CreateDomainStmt
 typedef struct CreateOpClassStmt
 {
        NodeTag         type;
-       List       *opclassname;        /* qualified name (list of Value 
strings) */
+       List       *opclassname;        /* qualified name (list of String) */
        List       *opfamilyname;       /* qualified name (ditto); NIL if 
omitted */
        char       *amname;                     /* name of index AM opclass is 
for */
        TypeName   *datatype;           /* datatype of indexed column */
@@ -2726,7 +2738,7 @@ typedef struct CreateOpClassItem
 typedef struct CreateOpFamilyStmt
 {
        NodeTag         type;
-       List       *opfamilyname;       /* qualified name (list of Value 
strings) */
+       List       *opfamilyname;       /* qualified name (list of String) */
        char       *amname;                     /* name of index AM opfamily is 
for */
 } CreateOpFamilyStmt;
 
@@ -2737,7 +2749,7 @@ typedef struct CreateOpFamilyStmt
 typedef struct AlterOpFamilyStmt
 {
        NodeTag         type;
-       List       *opfamilyname;       /* qualified name (list of Value 
strings) */
+       List       *opfamilyname;       /* qualified name (list of String) */
        char       *amname;                     /* name of index AM opfamily is 
for */
        bool            isDrop;                 /* ADD or DROP the items? */
        List       *items;                      /* List of CreateOpClassItem 
nodes */
@@ -2908,8 +2920,8 @@ typedef struct IndexStmt
 typedef struct CreateStatsStmt
 {
        NodeTag         type;
-       List       *defnames;           /* qualified name (list of Value 
strings) */
-       List       *stat_types;         /* stat types (list of Value strings) */
+       List       *defnames;           /* qualified name (list of String) */
+       List       *stat_types;         /* stat types (list of String) */
        List       *exprs;                      /* expressions to build 
statistics on */
        List       *relations;          /* rels to build stats on (list of 
RangeVar) */
        char       *stxcomment;         /* comment to apply to stats, or NULL */
@@ -2939,7 +2951,7 @@ typedef struct StatsElem
 typedef struct AlterStatsStmt
 {
        NodeTag         type;
-       List       *defnames;           /* qualified name (list of Value 
strings) */
+       List       *defnames;           /* qualified name (list of String) */
        int                     stxstattarget;  /* statistics target */
        bool            missing_ok;             /* skip error if statistics 
object is missing */
 } AlterStatsStmt;
@@ -3061,7 +3073,7 @@ typedef struct AlterObjectDependsStmt
        ObjectType      objectType;             /* OBJECT_FUNCTION, 
OBJECT_TRIGGER, etc */
        RangeVar   *relation;           /* in case a table is involved */
        Node       *object;                     /* name of the object */
-       Value      *extname;            /* extension name */
+       String     *extname;            /* extension name */
        bool            remove;                 /* set true to remove dep 
rather than add */
 } AlterObjectDependsStmt;
 
@@ -3207,8 +3219,8 @@ typedef struct CompositeTypeStmt
 typedef struct CreateEnumStmt
 {
        NodeTag         type;
-       List       *typeName;           /* qualified name (list of Value 
strings) */
-       List       *vals;                       /* enum values (list of Value 
strings) */
+       List       *typeName;           /* qualified name (list of String) */
+       List       *vals;                       /* enum values (list of String) 
*/
 } CreateEnumStmt;
 
 /* ----------------------
@@ -3218,7 +3230,7 @@ typedef struct CreateEnumStmt
 typedef struct CreateRangeStmt
 {
        NodeTag         type;
-       List       *typeName;           /* qualified name (list of Value 
strings) */
+       List       *typeName;           /* qualified name (list of String) */
        List       *params;                     /* range parameters (list of 
DefElem) */
 } CreateRangeStmt;
 
@@ -3229,7 +3241,7 @@ typedef struct CreateRangeStmt
 typedef struct AlterEnumStmt
 {
        NodeTag         type;
-       List       *typeName;           /* qualified name (list of Value 
strings) */
+       List       *typeName;           /* qualified name (list of String) */
        char       *oldVal;                     /* old enum value's name, if 
renaming */
        char       *newVal;                     /* new enum value's name */
        char       *newValNeighbor; /* neighboring enum value, if specified */
@@ -3591,7 +3603,7 @@ typedef struct ReassignOwnedStmt
 typedef struct AlterTSDictionaryStmt
 {
        NodeTag         type;
-       List       *dictname;           /* qualified name (list of Value 
strings) */
+       List       *dictname;           /* qualified name (list of String) */
        List       *options;            /* List of DefElem nodes */
 } AlterTSDictionaryStmt;
 
@@ -3611,14 +3623,14 @@ typedef struct AlterTSConfigurationStmt
 {
        NodeTag         type;
        AlterTSConfigType kind;         /* ALTER_TSCONFIG_ADD_MAPPING, etc */
-       List       *cfgname;            /* qualified name (list of Value 
strings) */
+       List       *cfgname;            /* qualified name (list of String) */
 
        /*
         * dicts will be non-NIL if ADD/ALTER MAPPING was specified. If dicts is
         * NIL, but tokentype isn't, DROP MAPPING was specified.
         */
-       List       *tokentype;          /* list of Value strings */
-       List       *dicts;                      /* list of list of Value 
strings */
+       List       *tokentype;          /* list of String */
+       List       *dicts;                      /* list of list of String */
        bool            override;               /* if true - remove old variant 
*/
        bool            replace;                /* if true - replace dictionary 
by another */
        bool            missing_ok;             /* for DROP - skip error if 
missing? */
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index c04282f91f..7b125904b4 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -32,7 +32,7 @@
  *       specifies an alias for a range variable; the alias might also
  *       specify renaming of columns within the table.
  *
- * Note: colnames is a list of Value nodes (always strings).  In Alias structs
+ * Note: colnames is a list of String nodes.  In Alias structs
  * associated with RTEs, there may be entries corresponding to dropped
  * columns; these are normally empty strings ("").  See parsenodes.h for info.
  */
@@ -76,7 +76,7 @@ typedef struct RangeVar
 /*
  * TableFunc - node for a table function, such as XMLTABLE.
  *
- * Entries in the ns_names list are either string Value nodes containing
+ * Entries in the ns_names list are either String nodes containing
  * literal namespace names, or NULL pointers to represent DEFAULT.
  */
 typedef struct TableFunc
@@ -1227,7 +1227,7 @@ typedef struct XmlExpr
        XmlExprOp       op;                             /* xml function ID */
        char       *name;                       /* name in xml(NAME foo ...) 
syntaxes */
        List       *named_args;         /* non-XML expressions for 
xml_attributes */
-       List       *arg_names;          /* parallel list of Value strings */
+       List       *arg_names;          /* parallel list of String values */
        List       *args;                       /* list of expressions */
        XmlOptionType xmloption;        /* DOCUMENT or CONTENT */
        Oid                     type;                   /* target type/typmod 
for XMLSERIALIZE */
diff --git a/src/include/nodes/value.h b/src/include/nodes/value.h
index b28928de54..8b71b510eb 100644
--- a/src/include/nodes/value.h
+++ b/src/include/nodes/value.h
@@ -1,7 +1,7 @@
 /*-------------------------------------------------------------------------
  *
  * value.h
- *       interface for Value nodes
+ *       interface for value nodes
  *
  *
  * Copyright (c) 2003-2021, PostgreSQL Global Development Group
@@ -16,46 +16,57 @@
 
 #include "nodes/nodes.h"
 
-/*----------------------
- *             Value node
+/*
+ * The node types Integer, Float, String, and BitString are used to represent
+ * literals in the lexer and are also used to pass constants around in the
+ * parser.  One difference between these node types and, say, a plain int or
+ * char * is that the nodes can be put into a List.
  *
- * The same Value struct is used for five node types: T_Integer,
- * T_Float, T_String, T_BitString, T_Null.
- *
- * Integral values are actually represented by a machine integer,
- * but both floats and strings are represented as strings.
- * Using T_Float as the node type simply indicates that
- * the contents of the string look like a valid numeric literal.
- *
- * (Before Postgres 7.0, we used a double to represent T_Float,
- * but that creates loss-of-precision problems when the value is
- * ultimately destined to be converted to NUMERIC.  Since Value nodes
- * are only used in the parsing process, not for runtime data, it's
- * better to use the more general representation.)
- *
- * Note that an integer-looking string will get lexed as T_Float if
- * the value is too large to fit in an 'int'.
+ * (There used to be a Value node, which encompassed all these different node 
types.  Hence the name of this file.)
+ */
+
+typedef struct Integer
+{
+       NodeTag         type;
+       int                     val;
+} Integer;
+
+/*
+ * Float is internally represented as string.  Using T_Float as the node type
+ * simply indicates that the contents of the string look like a valid numeric
+ * literal.  The value might end up being converted to NUMERIC, so we can't
+ * store it internally as a C double, since that could lose precision.  Since
+ * these nodes are generally only used in the parsing process, not for runtime
+ * data, it's better to use the more general representation.
  *
- * Nulls, of course, don't need the value part at all.
- *----------------------
+ * Note that an integer-looking string will get lexed as T_Float if the value
+ * is too large to fit in an 'int'.
  */
-typedef struct Value
+typedef struct Float
 {
-       NodeTag         type;                   /* tag appropriately (eg. 
T_String) */
-       union ValUnion
-       {
-               int                     ival;           /* machine integer */
-               char       *str;                /* string */
-       }                       val;
-} Value;
-
-#define intVal(v)              (((Value *)(v))->val.ival)
-#define floatVal(v)            atof(((Value *)(v))->val.str)
-#define strVal(v)              (((Value *)(v))->val.str)
-
-extern Value *makeInteger(int i);
-extern Value *makeFloat(char *numericStr);
-extern Value *makeString(char *str);
-extern Value *makeBitString(char *str);
+       NodeTag         type;
+       char       *val;
+} Float;
+
+typedef struct String
+{
+       NodeTag         type;
+       char       *val;
+} String;
+
+typedef struct BitString
+{
+       NodeTag         type;
+       char       *val;
+} BitString;
+
+#define intVal(v)              (castNode(Integer, v)->val)
+#define floatVal(v)            atof(castNode(Float, v)->val)
+#define strVal(v)              (castNode(String, v)->val)
+
+extern Integer *makeInteger(int i);
+extern Float *makeFloat(char *numericStr);
+extern String *makeString(char *str);
+extern BitString *makeBitString(char *str);
 
 #endif                                                 /* VALUE_H */
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 1500de2dd0..ee179082ce 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -333,7 +333,6 @@ extern SubscriptingRef 
*transformContainerSubscripts(ParseState *pstate,
                                                                                
                         int32 containerTypMod,
                                                                                
                         List *indirection,
                                                                                
                         bool isAssignment);
-
-extern Const *make_const(ParseState *pstate, Value *value, int location);
+extern Const *make_const(ParseState *pstate, A_Const *aconst);
 
 #endif                                                 /* PARSE_NODE_H */
-- 
2.33.0

Reply via email to