Skip to content

Commit 639a86e

Browse files
peterepetere
committed
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. Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/5ba6bc5b-3f95-04f2-2419-f8ddb4c046fb@enterprisedb.com
1 parent cbdf75b commit 639a86e

30 files changed

+373
-337
lines changed

‎contrib/postgres_fdw/postgres_fdw.c

+6-6
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,9 @@ enum FdwModifyPrivateIndex
101101
FdwModifyPrivateUpdateSql,
102102
/* Integer list of target attribute numbers for INSERT/UPDATE */
103103
FdwModifyPrivateTargetAttnums,
104-
/* Length till the end of VALUES clause (as an integer Value node) */
104+
/* Length till the end of VALUES clause (as an Integer node) */
105105
FdwModifyPrivateLen,
106-
/* has-returning flag (as an integer Value node) */
106+
/* has-returning flag (as an Integer node) */
107107
FdwModifyPrivateHasReturning,
108108
/* Integer list of attribute numbers retrieved by RETURNING */
109109
FdwModifyPrivateRetrievedAttrs
@@ -122,11 +122,11 @@ enum FdwDirectModifyPrivateIndex
122122
{
123123
/* SQL statement to execute remotely (as a String node) */
124124
FdwDirectModifyPrivateUpdateSql,
125-
/* has-returning flag (as an integer Value node) */
125+
/* has-returning flag (as an Integer node) */
126126
FdwDirectModifyPrivateHasReturning,
127127
/* Integer list of attribute numbers retrieved by RETURNING */
128128
FdwDirectModifyPrivateRetrievedAttrs,
129-
/* set-processed flag (as an integer Value node) */
129+
/* set-processed flag (as an Integer node) */
130130
FdwDirectModifyPrivateSetProcessed
131131
};
132132

@@ -280,9 +280,9 @@ typedef struct PgFdwAnalyzeState
280280
*/
281281
enum FdwPathPrivateIndex
282282
{
283-
/* has-final-sort flag (as an integer Value node) */
283+
/* has-final-sort flag (as an Integer node) */
284284
FdwPathPrivateHasFinalSort,
285-
/* has-limit flag (as an integer Value node) */
285+
/* has-limit flag (as an Integer node) */
286286
FdwPathPrivateHasLimit
287287
};
288288

‎src/backend/catalog/namespace.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -3026,7 +3026,7 @@ CheckSetNamespace(Oid oldNspOid, Oid nspOid)
30263026

30273027
/*
30283028
* QualifiedNameGetCreationNamespace
3029-
* Given a possibly-qualified name for an object (in List-of-Values
3029+
* Given a possibly-qualified name for an object (in List-of-Strings
30303030
* format), determine what namespace the object should be created in.
30313031
* Also extract and return the object name (last component of list).
30323032
*
@@ -3140,7 +3140,7 @@ makeRangeVarFromNameList(List *names)
31403140
* This is used primarily to form error messages, and so we do not quote
31413141
* the list elements, for the sake of legibility.
31423142
*
3143-
* In most scenarios the list elements should always be Value strings,
3143+
* In most scenarios the list elements should always be String values,
31443144
* but we also allow A_Star for the convenience of ColumnRef processing.
31453145
*/
31463146
char *

‎src/backend/catalog/objectaddress.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -851,7 +851,7 @@ const ObjectAddress InvalidObjectAddress =
851851
};
852852

853853
static ObjectAddress get_object_address_unqualified(ObjectType objtype,
854-
Value *strval, bool missing_ok);
854+
String *strval, bool missing_ok);
855855
static ObjectAddress get_relation_by_qualified_name(ObjectType objtype,
856856
List *object, Relation *relp,
857857
LOCKMODE lockmode, bool missing_ok);
@@ -1011,7 +1011,7 @@ get_object_address(ObjectType objtype, Node *object,
10111011
case OBJECT_PUBLICATION:
10121012
case OBJECT_SUBSCRIPTION:
10131013
address = get_object_address_unqualified(objtype,
1014-
(Value *) object, missing_ok);
1014+
castNode(String, object), missing_ok);
10151015
break;
10161016
case OBJECT_TYPE:
10171017
case OBJECT_DOMAIN:
@@ -1244,7 +1244,7 @@ get_object_address_rv(ObjectType objtype, RangeVar *rel, List *object,
12441244
*/
12451245
static ObjectAddress
12461246
get_object_address_unqualified(ObjectType objtype,
1247-
Value *strval, bool missing_ok)
1247+
String *strval, bool missing_ok)
12481248
{
12491249
const char *name;
12501250
ObjectAddress address;

‎src/backend/catalog/pg_enum.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ static int sort_order_cmp(const void *p1, const void *p2);
5555
* EnumValuesCreate
5656
* Create an entry in pg_enum for each of the supplied enum values.
5757
*
58-
* vals is a list of Value strings.
58+
* vals is a list of String values.
5959
*/
6060
void
6161
EnumValuesCreate(Oid enumTypeOid, List *vals)

‎src/backend/commands/copy.c

+2-3
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,8 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
222222
{
223223
/*
224224
* Build the ColumnRef for each column. The ColumnRef
225-
* 'fields' property is a String 'Value' node (see
226-
* nodes/value.h) that corresponds to the column name
227-
* respectively.
225+
* 'fields' property is a String node that corresponds to
226+
* the column name respectively.
228227
*/
229228
cr = makeNode(ColumnRef);
230229
cr->fields = list_make1(lfirst(lc));

‎src/backend/commands/define.c

+2-7
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,7 @@ defGetString(DefElem *def)
5858
case T_Integer:
5959
return psprintf("%ld", (long) intVal(def->arg));
6060
case T_Float:
61-
62-
/*
63-
* T_Float values are kept in string form, so this type cheat
64-
* works (and doesn't risk losing precision)
65-
*/
66-
return strVal(def->arg);
61+
return castNode(Float, def->arg)->val;
6762
case T_String:
6863
return strVal(def->arg);
6964
case T_TypeName:
@@ -206,7 +201,7 @@ defGetInt64(DefElem *def)
206201
* strings.
207202
*/
208203
return DatumGetInt64(DirectFunctionCall1(int8in,
209-
CStringGetDatum(strVal(def->arg))));
204+
CStringGetDatum(castNode(Float, def->arg)->val)));
210205
default:
211206
ereport(ERROR,
212207
(errcode(ERRCODE_SYNTAX_ERROR),

‎src/backend/commands/tsearchcmds.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -1179,7 +1179,7 @@ getTokenTypes(Oid prsId, List *tokennames)
11791179
i = 0;
11801180
foreach(tn, tokennames)
11811181
{
1182-
Value *val = (Value *) lfirst(tn);
1182+
String *val = lfirst_node(String, tn);
11831183
bool found = false;
11841184
int j;
11851185

@@ -1395,7 +1395,7 @@ DropConfigurationMapping(AlterTSConfigurationStmt *stmt,
13951395
i = 0;
13961396
foreach(c, stmt->tokentype)
13971397
{
1398-
Value *val = (Value *) lfirst(c);
1398+
String *val = lfirst_node(String, c);
13991399
bool found = false;
14001400

14011401
ScanKeyInit(&skey[0],

‎src/backend/executor/nodeTableFuncscan.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ tfuncInitialize(TableFuncScanState *tstate, ExprContext *econtext, Datum doc)
364364
forboth(lc1, tstate->ns_uris, lc2, tstate->ns_names)
365365
{
366366
ExprState *expr = (ExprState *) lfirst(lc1);
367-
Value *ns_node = (Value *) lfirst(lc2);
367+
String *ns_node = lfirst_node(String, lc2);
368368
char *ns_uri;
369369
char *ns_name;
370370

‎src/backend/nodes/README

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ FILES IN THIS DIRECTORY (src/backend/nodes/)
2828
list.c - generic list support
2929
params.c - Param support
3030
tidbitmap.c - TIDBitmap support
31-
value.c - support for Value nodes
31+
value.c - support for value nodes
3232

3333
FILES IN src/include/nodes/
3434

‎src/backend/nodes/copyfuncs.c

+64-43
Original file line numberDiff line numberDiff line change
@@ -2729,25 +2729,30 @@ _copyA_Const(const A_Const *from)
27292729
{
27302730
A_Const *newnode = makeNode(A_Const);
27312731

2732-
/* This part must duplicate _copyValue */
2733-
COPY_SCALAR_FIELD(val.type);
2734-
switch (from->val.type)
2732+
COPY_SCALAR_FIELD(isnull);
2733+
if (!from->isnull)
27352734
{
2736-
case T_Integer:
2737-
COPY_SCALAR_FIELD(val.val.ival);
2738-
break;
2739-
case T_Float:
2740-
case T_String:
2741-
case T_BitString:
2742-
COPY_STRING_FIELD(val.val.str);
2743-
break;
2744-
case T_Null:
2745-
/* nothing to do */
2746-
break;
2747-
default:
2748-
elog(ERROR, "unrecognized node type: %d",
2749-
(int) from->val.type);
2750-
break;
2735+
/* This part must duplicate other _copy*() functions. */
2736+
COPY_SCALAR_FIELD(val.node.type);
2737+
switch (nodeTag(&from->val))
2738+
{
2739+
case T_Integer:
2740+
COPY_SCALAR_FIELD(val.ival.val);
2741+
break;
2742+
case T_Float:
2743+
COPY_STRING_FIELD(val.fval.val);
2744+
break;
2745+
case T_String:
2746+
COPY_STRING_FIELD(val.sval.val);
2747+
break;
2748+
case T_BitString:
2749+
COPY_STRING_FIELD(val.bsval.val);
2750+
break;
2751+
default:
2752+
elog(ERROR, "unrecognized node type: %d",
2753+
(int) nodeTag(&from->val));
2754+
break;
2755+
}
27512756
}
27522757

27532758
COPY_LOCATION_FIELD(location);
@@ -4892,32 +4897,43 @@ _copyExtensibleNode(const ExtensibleNode *from)
48924897
* value.h copy functions
48934898
* ****************************************************************
48944899
*/
4895-
static Value *
4896-
_copyValue(const Value *from)
4900+
static Integer *
4901+
_copyInteger(const Integer *from)
48974902
{
4898-
Value *newnode = makeNode(Value);
4903+
Integer *newnode = makeNode(Integer);
48994904

4900-
/* See also _copyAConst when changing this code! */
4905+
COPY_SCALAR_FIELD(val);
4906+
4907+
return newnode;
4908+
}
4909+
4910+
static Float *
4911+
_copyFloat(const Float *from)
4912+
{
4913+
Float *newnode = makeNode(Float);
4914+
4915+
COPY_STRING_FIELD(val);
4916+
4917+
return newnode;
4918+
}
4919+
4920+
static String *
4921+
_copyString(const String *from)
4922+
{
4923+
String *newnode = makeNode(String);
4924+
4925+
COPY_STRING_FIELD(val);
4926+
4927+
return newnode;
4928+
}
4929+
4930+
static BitString *
4931+
_copyBitString(const BitString *from)
4932+
{
4933+
BitString *newnode = makeNode(BitString);
4934+
4935+
COPY_STRING_FIELD(val);
49014936

4902-
COPY_SCALAR_FIELD(type);
4903-
switch (from->type)
4904-
{
4905-
case T_Integer:
4906-
COPY_SCALAR_FIELD(val.ival);
4907-
break;
4908-
case T_Float:
4909-
case T_String:
4910-
case T_BitString:
4911-
COPY_STRING_FIELD(val.str);
4912-
break;
4913-
case T_Null:
4914-
/* nothing to do */
4915-
break;
4916-
default:
4917-
elog(ERROR, "unrecognized node type: %d",
4918-
(int) from->type);
4919-
break;
4920-
}
49214937
return newnode;
49224938
}
49234939

@@ -5314,11 +5330,16 @@ copyObjectImpl(const void *from)
53145330
* VALUE NODES
53155331
*/
53165332
case T_Integer:
5333+
retval = _copyInteger(from);
5334+
break;
53175335
case T_Float:
5336+
retval = _copyFloat(from);
5337+
break;
53185338
case T_String:
5339+
retval = _copyString(from);
5340+
break;
53195341
case T_BitString:
5320-
case T_Null:
5321-
retval = _copyValue(from);
5342+
retval = _copyBitString(from);
53225343
break;
53235344

53245345
/*

‎src/backend/nodes/equalfuncs.c

+33-22
Original file line numberDiff line numberDiff line change
@@ -2409,7 +2409,7 @@ _equalParamRef(const ParamRef *a, const ParamRef *b)
24092409
static bool
24102410
_equalA_Const(const A_Const *a, const A_Const *b)
24112411
{
2412-
if (!equal(&a->val, &b->val)) /* hack for in-line Value field */
2412+
if (!equal(&a->val, &b->val)) /* hack for in-line val field */
24132413
return false;
24142414
COMPARE_LOCATION_FIELD(location);
24152415

@@ -3089,27 +3089,33 @@ _equalList(const List *a, const List *b)
30893089
*/
30903090

30913091
static bool
3092-
_equalValue(const Value *a, const Value *b)
3092+
_equalInteger(const Integer *a, const Integer *b)
30933093
{
3094-
COMPARE_SCALAR_FIELD(type);
3094+
COMPARE_SCALAR_FIELD(val);
30953095

3096-
switch (a->type)
3097-
{
3098-
case T_Integer:
3099-
COMPARE_SCALAR_FIELD(val.ival);
3100-
break;
3101-
case T_Float:
3102-
case T_String:
3103-
case T_BitString:
3104-
COMPARE_STRING_FIELD(val.str);
3105-
break;
3106-
case T_Null:
3107-
/* nothing to do */
3108-
break;
3109-
default:
3110-
elog(ERROR, "unrecognized node type: %d", (int) a->type);
3111-
break;
3112-
}
3096+
return true;
3097+
}
3098+
3099+
static bool
3100+
_equalFloat(const Float *a, const Float *b)
3101+
{
3102+
COMPARE_STRING_FIELD(val);
3103+
3104+
return true;
3105+
}
3106+
3107+
static bool
3108+
_equalString(const String *a, const String *b)
3109+
{
3110+
COMPARE_STRING_FIELD(val);
3111+
3112+
return true;
3113+
}
3114+
3115+
static bool
3116+
_equalBitString(const BitString *a, const BitString *b)
3117+
{
3118+
COMPARE_STRING_FIELD(val);
31133119

31143120
return true;
31153121
}
@@ -3337,11 +3343,16 @@ equal(const void *a, const void *b)
33373343
break;
33383344

33393345
case T_Integer:
3346+
retval = _equalInteger(a, b);
3347+
break;
33403348
case T_Float:
3349+
retval = _equalFloat(a, b);
3350+
break;
33413351
case T_String:
3352+
retval = _equalString(a, b);
3353+
break;
33423354
case T_BitString:
3343-
case T_Null:
3344-
retval = _equalValue(a, b);
3355+
retval = _equalBitString(a, b);
33453356
break;
33463357

33473358
/*

‎src/backend/nodes/nodeFuncs.c

-1
Original file line numberDiff line numberDiff line change
@@ -3537,7 +3537,6 @@ raw_expression_tree_walker(Node *node,
35373537
case T_Float:
35383538
case T_String:
35393539
case T_BitString:
3540-
case T_Null:
35413540
case T_ParamRef:
35423541
case T_A_Const:
35433542
case T_A_Star:

0 commit comments

Comments
 (0)