Skip to content

Commit 090173a

Browse files
tglsfdctglsfdc
committed
Remove the recently added node types ReloptElem and OptionDefElem in favor
of adding optional namespace and action fields to DefElem. Having three node types that do essentially the same thing bloats the code and leads to errors of confusion, such as in yesterday's bug report from Khee Chin.
1 parent c973051 commit 090173a

File tree

17 files changed

+208
-333
lines changed

17 files changed

+208
-333
lines changed

‎src/backend/access/common/reloptions.c

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/common/reloptions.c,v 1.25 2009/04/04 00:45:02 alvherre Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/common/reloptions.c,v 1.26 2009/04/04 21:12:30 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -483,7 +483,7 @@ add_string_reloption(bits32 kinds, char *name, char *desc, char *default_val,
483483
}
484484

485485
/*
486-
* Transform a relation options list (list of ReloptElem) into the text array
486+
* Transform a relation options list (list of DefElem) into the text array
487487
* format that is kept in pg_class.reloptions, including only those options
488488
* that are in the passed namespace. The output values do not include the
489489
* namespace.
@@ -542,23 +542,23 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
542542
/* Search for a match in defList */
543543
foreach(cell, defList)
544544
{
545-
ReloptElem *def = lfirst(cell);
545+
DefElem *def = (DefElem *) lfirst(cell);
546546
int kw_len;
547547

548548
/* ignore if not in the same namespace */
549549
if (namspace == NULL)
550550
{
551-
if (def->nmspc != NULL)
551+
if (def->defnamespace != NULL)
552552
continue;
553553
}
554-
else if (def->nmspc == NULL)
554+
else if (def->defnamespace == NULL)
555555
continue;
556-
else if (pg_strcasecmp(def->nmspc, namspace) != 0)
556+
else if (pg_strcasecmp(def->defnamespace, namspace) != 0)
557557
continue;
558558

559-
kw_len = strlen(def->optname);
559+
kw_len = strlen(def->defname);
560560
if (text_len > kw_len && text_str[kw_len] == '=' &&
561-
pg_strncasecmp(text_str, def->optname, kw_len) == 0)
561+
pg_strncasecmp(text_str, def->defname, kw_len) == 0)
562562
break;
563563
}
564564
if (!cell)
@@ -578,8 +578,7 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
578578
*/
579579
foreach(cell, defList)
580580
{
581-
ReloptElem *def = lfirst(cell);
582-
581+
DefElem *def = (DefElem *) lfirst(cell);
583582

584583
if (isReset)
585584
{
@@ -598,7 +597,7 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
598597
* Error out if the namespace is not valid. A NULL namespace
599598
* is always valid.
600599
*/
601-
if (def->nmspc != NULL)
600+
if (def->defnamespace != NULL)
602601
{
603602
bool valid = false;
604603
int i;
@@ -607,7 +606,8 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
607606
{
608607
for (i = 0; validnsps[i]; i++)
609608
{
610-
if (pg_strcasecmp(def->nmspc, validnsps[i]) == 0)
609+
if (pg_strcasecmp(def->defnamespace,
610+
validnsps[i]) == 0)
611611
{
612612
valid = true;
613613
break;
@@ -619,37 +619,37 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
619619
ereport(ERROR,
620620
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
621621
errmsg("unrecognized parameter namespace \"%s\"",
622-
def->nmspc)));
622+
def->defnamespace)));
623623
}
624624

625-
if (ignoreOids && pg_strcasecmp(def->optname, "oids") == 0)
625+
if (ignoreOids && pg_strcasecmp(def->defname, "oids") == 0)
626626
continue;
627627

628628
/* ignore if not in the same namespace */
629629
if (namspace == NULL)
630630
{
631-
if (def->nmspc != NULL)
631+
if (def->defnamespace != NULL)
632632
continue;
633633
}
634-
else if (def->nmspc == NULL)
634+
else if (def->defnamespace == NULL)
635635
continue;
636-
else if (pg_strcasecmp(def->nmspc, namspace) != 0)
636+
else if (pg_strcasecmp(def->defnamespace, namspace) != 0)
637637
continue;
638638

639639
/*
640-
* Flatten the ReloptElem into a text string like "name=arg". If we
640+
* Flatten the DefElem into a text string like "name=arg". If we
641641
* have just "name", assume "name=true" is meant. Note: the
642642
* namespace is not output.
643643
*/
644644
if (def->arg != NULL)
645-
value = reloptGetString(def);
645+
value = defGetString(def);
646646
else
647647
value = "true";
648-
len = VARHDRSZ + strlen(def->optname) + 1 + strlen(value);
648+
len = VARHDRSZ + strlen(def->defname) + 1 + strlen(value);
649649
/* +1 leaves room for sprintf's trailing null */
650650
t = (text *) palloc(len + 1);
651651
SET_VARSIZE(t, len);
652-
sprintf(VARDATA(t), "%s=%s", def->optname, value);
652+
sprintf(VARDATA(t), "%s=%s", def->defname, value);
653653

654654
astate = accumArrayResult(astate, PointerGetDatum(t),
655655
false, TEXTOID,

‎src/backend/commands/define.c

Lines changed: 31 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/backend/commands/define.c,v 1.103 2009/02/02 19:31:38 alvherre Exp $
12+
* $PostgreSQL: pgsql/src/backend/commands/define.c,v 1.104 2009/04/04 21:12:31 tgl Exp $
1313
*
1414
* DESCRIPTION
1515
* The "DefineFoo" routines take the parse tree and pick out the
@@ -55,20 +55,24 @@ case_translate_language_name(const char *input)
5555
}
5656

5757

58-
static char *
59-
nodeGetString(Node *value, char *name)
58+
/*
59+
* Extract a string value (otherwise uninterpreted) from a DefElem.
60+
*/
61+
char *
62+
defGetString(DefElem *def)
6063
{
61-
if (value == NULL)
64+
if (def->arg == NULL)
6265
ereport(ERROR,
6366
(errcode(ERRCODE_SYNTAX_ERROR),
64-
errmsg("%s requires a parameter", name)));
65-
switch (nodeTag(value))
67+
errmsg("%s requires a parameter",
68+
def->defname)));
69+
switch (nodeTag(def->arg))
6670
{
6771
case T_Integer:
6872
{
6973
char *str = palloc(32);
7074

71-
snprintf(str, 32, "%ld", (long) intVal(value));
75+
snprintf(str, 32, "%ld", (long) intVal(def->arg));
7276
return str;
7377
}
7478
case T_Float:
@@ -77,28 +81,19 @@ nodeGetString(Node *value, char *name)
7781
* T_Float values are kept in string form, so this type cheat
7882
* works (and doesn't risk losing precision)
7983
*/
80-
return strVal(value);
84+
return strVal(def->arg);
8185
case T_String:
82-
return strVal(value);
86+
return strVal(def->arg);
8387
case T_TypeName:
84-
return TypeNameToString((TypeName *) value);
88+
return TypeNameToString((TypeName *) def->arg);
8589
case T_List:
86-
return NameListToString((List *) value);
90+
return NameListToString((List *) def->arg);
8791
default:
88-
elog(ERROR, "unrecognized node type: %d", (int) nodeTag(value));
92+
elog(ERROR, "unrecognized node type: %d", (int) nodeTag(def->arg));
8993
}
9094
return NULL; /* keep compiler quiet */
9195
}
9296

93-
/*
94-
* Extract a string value (otherwise uninterpreted) from a DefElem.
95-
*/
96-
char *
97-
defGetString(DefElem *def)
98-
{
99-
return nodeGetString(def->arg, def->defname);
100-
}
101-
10297
/*
10398
* Extract a numeric value (actually double) from a DefElem.
10499
*/
@@ -125,22 +120,25 @@ defGetNumeric(DefElem *def)
125120
return 0; /* keep compiler quiet */
126121
}
127122

128-
static bool
129-
nodeGetBoolean(Node *value, char *name)
123+
/*
124+
* Extract a boolean value from a DefElem.
125+
*/
126+
bool
127+
defGetBoolean(DefElem *def)
130128
{
131129
/*
132130
* If no parameter given, assume "true" is meant.
133131
*/
134-
if (value == NULL)
132+
if (def->arg == NULL)
135133
return true;
136134

137135
/*
138136
* Allow 0, 1, "true", "false"
139137
*/
140-
switch (nodeTag(value))
138+
switch (nodeTag(def->arg))
141139
{
142140
case T_Integer:
143-
switch (intVal(value))
141+
switch (intVal(def->arg))
144142
{
145143
case 0:
146144
return false;
@@ -153,7 +151,7 @@ nodeGetBoolean(Node *value, char *name)
153151
break;
154152
default:
155153
{
156-
char *sval = nodeGetString(value, name);
154+
char *sval = defGetString(def);
157155

158156
if (pg_strcasecmp(sval, "true") == 0)
159157
return true;
@@ -165,19 +163,11 @@ nodeGetBoolean(Node *value, char *name)
165163
}
166164
ereport(ERROR,
167165
(errcode(ERRCODE_SYNTAX_ERROR),
168-
errmsg("%s requires a Boolean value", name)));
166+
errmsg("%s requires a Boolean value",
167+
def->defname)));
169168
return false; /* keep compiler quiet */
170169
}
171170

172-
/*
173-
* Extract a boolean value from a DefElem.
174-
*/
175-
bool
176-
defGetBoolean(DefElem *def)
177-
{
178-
return nodeGetBoolean(def->arg, def->defname);
179-
}
180-
181171
/*
182172
* Extract an int64 value from a DefElem.
183173
*/
@@ -315,35 +305,11 @@ defGetTypeLength(DefElem *def)
315305
return 0; /* keep compiler quiet */
316306
}
317307

318-
319308
/*
320-
* Extract a string value (otherwise uninterpreted) from a ReloptElem.
309+
* Create a DefElem setting "oids" to the specified value.
321310
*/
322-
char *
323-
reloptGetString(ReloptElem *relopt)
311+
DefElem *
312+
defWithOids(bool value)
324313
{
325-
return nodeGetString(relopt->arg, relopt->optname);
326-
}
327-
328-
/*
329-
* Extract a boolean value from a ReloptElem.
330-
*/
331-
bool
332-
reloptGetBoolean(ReloptElem *relopt)
333-
{
334-
return nodeGetBoolean(relopt->arg, relopt->optname);
335-
}
336-
337-
/*
338-
* Create a ReloptElem setting "oids" to the specified value.
339-
*/
340-
ReloptElem *
341-
reloptWithOids(bool value)
342-
{
343-
ReloptElem *f = makeNode(ReloptElem);
344-
345-
f->optname = "oids";
346-
f->nmspc = NULL;
347-
f->arg = (Node *) makeInteger(value);
348-
return f;
314+
return makeDefElem("oids", (Node *) makeInteger(value));
349315
}

0 commit comments

Comments
 (0)