Skip to content

Commit 1788828

Browse files
tglsfdctglsfdc
committed
Remove PLPGSQL_DTYPE_ARRAYELEM datum type within pl/pgsql.
In the wake of the previous commit, we don't really need this anymore, since array assignment is primarily handled by the core code. The only way that that code could still be reached is that a GET DIAGNOSTICS target variable could be an array element. But that doesn't seem like a particularly essential feature. I'd added it in commit 55caaae, but just because it was easy not because anyone had actually asked for it. Hence, revert that patch and then remove the now-unreachable stuff. (If we really had to, we could probably reimplement GET DIAGNOSTICS using the new assignment machinery; but the cost/benefit ratio looks very poor, and it'd likely be a bit slower.) Note that PLPGSQL_DTYPE_RECFIELD remains. It's possible that we could get rid of that too, but maintaining the existing behaviors for RECORD-type variables seems like it might be difficult. Since there's not any functional limitation in those code paths as there was in the ARRAYELEM code, I've not pursued the idea. Discussion: https://postgr.es/m/4165684.1607707277@sss.pgh.pa.us
1 parent c9d5298 commit 1788828

File tree

6 files changed

+26
-286
lines changed

6 files changed

+26
-286
lines changed

‎src/pl/plpgsql/src/pl_exec.c

Lines changed: 4 additions & 200 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,12 +1311,11 @@ copy_plpgsql_datums(PLpgSQL_execstate *estate,
13111311

13121312
case PLPGSQL_DTYPE_ROW:
13131313
case PLPGSQL_DTYPE_RECFIELD:
1314-
case PLPGSQL_DTYPE_ARRAYELEM:
13151314

13161315
/*
13171316
* These datum records are read-only at runtime, so no need to
1318-
* copy them (well, RECFIELD and ARRAYELEM contain cached
1319-
* data, but we'd just as soon centralize the caching anyway).
1317+
* copy them (well, RECFIELD contains cached data, but we'd
1318+
* just as soon centralize the caching anyway).
13201319
*/
13211320
outdatum = indatum;
13221321
break;
@@ -4136,9 +4135,6 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
41364135
*
41374136
* NB: the result of the evaluation is no longer valid after this is done,
41384137
* unless it is a pass-by-value datatype.
4139-
*
4140-
* NB: if you change this code, see also the hacks in exec_assign_value's
4141-
* PLPGSQL_DTYPE_ARRAYELEM case for partial cleanup after subscript evals.
41424138
* ----------
41434139
*/
41444140
static void
@@ -5288,198 +5284,6 @@ exec_assign_value(PLpgSQL_execstate *estate,
52885284
break;
52895285
}
52905286

5291-
case PLPGSQL_DTYPE_ARRAYELEM:
5292-
{
5293-
/*
5294-
* Target is an element of an array
5295-
*/
5296-
PLpgSQL_arrayelem *arrayelem;
5297-
int nsubscripts;
5298-
int i;
5299-
PLpgSQL_expr *subscripts[MAXDIM];
5300-
int subscriptvals[MAXDIM];
5301-
Datum oldarraydatum,
5302-
newarraydatum,
5303-
coerced_value;
5304-
bool oldarrayisnull;
5305-
Oid parenttypoid;
5306-
int32 parenttypmod;
5307-
SPITupleTable *save_eval_tuptable;
5308-
MemoryContext oldcontext;
5309-
5310-
/*
5311-
* We need to do subscript evaluation, which might require
5312-
* evaluating general expressions; and the caller might have
5313-
* done that too in order to prepare the input Datum. We have
5314-
* to save and restore the caller's SPI_execute result, if
5315-
* any.
5316-
*/
5317-
save_eval_tuptable = estate->eval_tuptable;
5318-
estate->eval_tuptable = NULL;
5319-
5320-
/*
5321-
* To handle constructs like x[1][2] := something, we have to
5322-
* be prepared to deal with a chain of arrayelem datums. Chase
5323-
* back to find the base array datum, and save the subscript
5324-
* expressions as we go. (We are scanning right to left here,
5325-
* but want to evaluate the subscripts left-to-right to
5326-
* minimize surprises.) Note that arrayelem is left pointing
5327-
* to the leftmost arrayelem datum, where we will cache the
5328-
* array element type data.
5329-
*/
5330-
nsubscripts = 0;
5331-
do
5332-
{
5333-
arrayelem = (PLpgSQL_arrayelem *) target;
5334-
if (nsubscripts >= MAXDIM)
5335-
ereport(ERROR,
5336-
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
5337-
errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)",
5338-
nsubscripts + 1, MAXDIM)));
5339-
subscripts[nsubscripts++] = arrayelem->subscript;
5340-
target = estate->datums[arrayelem->arrayparentno];
5341-
} while (target->dtype == PLPGSQL_DTYPE_ARRAYELEM);
5342-
5343-
/* Fetch current value of array datum */
5344-
exec_eval_datum(estate, target,
5345-
&parenttypoid, &parenttypmod,
5346-
&oldarraydatum, &oldarrayisnull);
5347-
5348-
/* Update cached type data if necessary */
5349-
if (arrayelem->parenttypoid != parenttypoid ||
5350-
arrayelem->parenttypmod != parenttypmod)
5351-
{
5352-
Oid arraytypoid;
5353-
int32 arraytypmod = parenttypmod;
5354-
int16 arraytyplen;
5355-
Oid elemtypoid;
5356-
int16 elemtyplen;
5357-
bool elemtypbyval;
5358-
char elemtypalign;
5359-
5360-
/* If target is domain over array, reduce to base type */
5361-
arraytypoid = getBaseTypeAndTypmod(parenttypoid,
5362-
&arraytypmod);
5363-
5364-
/* ... and identify the element type */
5365-
elemtypoid = get_element_type(arraytypoid);
5366-
if (!OidIsValid(elemtypoid))
5367-
ereport(ERROR,
5368-
(errcode(ERRCODE_DATATYPE_MISMATCH),
5369-
errmsg("subscripted object is not an array")));
5370-
5371-
/* Collect needed data about the types */
5372-
arraytyplen = get_typlen(arraytypoid);
5373-
5374-
get_typlenbyvalalign(elemtypoid,
5375-
&elemtyplen,
5376-
&elemtypbyval,
5377-
&elemtypalign);
5378-
5379-
/* Now safe to update the cached data */
5380-
arrayelem->parenttypoid = parenttypoid;
5381-
arrayelem->parenttypmod = parenttypmod;
5382-
arrayelem->arraytypoid = arraytypoid;
5383-
arrayelem->arraytypmod = arraytypmod;
5384-
arrayelem->arraytyplen = arraytyplen;
5385-
arrayelem->elemtypoid = elemtypoid;
5386-
arrayelem->elemtyplen = elemtyplen;
5387-
arrayelem->elemtypbyval = elemtypbyval;
5388-
arrayelem->elemtypalign = elemtypalign;
5389-
}
5390-
5391-
/*
5392-
* Evaluate the subscripts, switch into left-to-right order.
5393-
* Like the expression built by ExecInitSubscriptingRef(),
5394-
* complain if any subscript is null.
5395-
*/
5396-
for (i = 0; i < nsubscripts; i++)
5397-
{
5398-
bool subisnull;
5399-
5400-
subscriptvals[i] =
5401-
exec_eval_integer(estate,
5402-
subscripts[nsubscripts - 1 - i],
5403-
&subisnull);
5404-
if (subisnull)
5405-
ereport(ERROR,
5406-
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
5407-
errmsg("array subscript in assignment must not be null")));
5408-
5409-
/*
5410-
* Clean up in case the subscript expression wasn't
5411-
* simple. We can't do exec_eval_cleanup, but we can do
5412-
* this much (which is safe because the integer subscript
5413-
* value is surely pass-by-value), and we must do it in
5414-
* case the next subscript expression isn't simple either.
5415-
*/
5416-
if (estate->eval_tuptable != NULL)
5417-
SPI_freetuptable(estate->eval_tuptable);
5418-
estate->eval_tuptable = NULL;
5419-
}
5420-
5421-
/* Now we can restore caller's SPI_execute result if any. */
5422-
Assert(estate->eval_tuptable == NULL);
5423-
estate->eval_tuptable = save_eval_tuptable;
5424-
5425-
/* Coerce source value to match array element type. */
5426-
coerced_value = exec_cast_value(estate,
5427-
value,
5428-
&isNull,
5429-
valtype,
5430-
valtypmod,
5431-
arrayelem->elemtypoid,
5432-
arrayelem->arraytypmod);
5433-
5434-
/*
5435-
* If the original array is null, cons up an empty array so
5436-
* that the assignment can proceed; we'll end with a
5437-
* one-element array containing just the assigned-to
5438-
* subscript. This only works for varlena arrays, though; for
5439-
* fixed-length array types we skip the assignment. We can't
5440-
* support assignment of a null entry into a fixed-length
5441-
* array, either, so that's a no-op too. This is all ugly but
5442-
* corresponds to the current behavior of execExpr*.c.
5443-
*/
5444-
if (arrayelem->arraytyplen > 0 && /* fixed-length array? */
5445-
(oldarrayisnull || isNull))
5446-
return;
5447-
5448-
/* empty array, if any, and newarraydatum are short-lived */
5449-
oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate));
5450-
5451-
if (oldarrayisnull)
5452-
oldarraydatum = PointerGetDatum(construct_empty_array(arrayelem->elemtypoid));
5453-
5454-
/*
5455-
* Build the modified array value.
5456-
*/
5457-
newarraydatum = array_set_element(oldarraydatum,
5458-
nsubscripts,
5459-
subscriptvals,
5460-
coerced_value,
5461-
isNull,
5462-
arrayelem->arraytyplen,
5463-
arrayelem->elemtyplen,
5464-
arrayelem->elemtypbyval,
5465-
arrayelem->elemtypalign);
5466-
5467-
MemoryContextSwitchTo(oldcontext);
5468-
5469-
/*
5470-
* Assign the new array to the base variable. It's never NULL
5471-
* at this point. Note that if the target is a domain,
5472-
* coercing the base array type back up to the domain will
5473-
* happen within exec_assign_value.
5474-
*/
5475-
exec_assign_value(estate, target,
5476-
newarraydatum,
5477-
false,
5478-
arrayelem->arraytypoid,
5479-
arrayelem->arraytypmod);
5480-
break;
5481-
}
5482-
54835287
default:
54845288
elog(ERROR, "unrecognized dtype: %d", target->dtype);
54855289
}
@@ -5490,8 +5294,8 @@ exec_assign_value(PLpgSQL_execstate *estate,
54905294
*
54915295
* The type oid, typmod, value in Datum format, and null flag are returned.
54925296
*
5493-
* At present this doesn't handle PLpgSQL_expr or PLpgSQL_arrayelem datums;
5494-
* that's not needed because we never pass references to such datums to SPI.
5297+
* At present this doesn't handle PLpgSQL_expr datums; that's not needed
5298+
* because we never pass references to such datums to SPI.
54955299
*
54965300
* NOTE: the returned Datum points right at the stored value in the case of
54975301
* pass-by-reference datatypes. Generally callers should take care not to

‎src/pl/plpgsql/src/pl_funcs.c

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -768,9 +768,6 @@ plpgsql_free_function_memory(PLpgSQL_function *func)
768768
break;
769769
case PLPGSQL_DTYPE_RECFIELD:
770770
break;
771-
case PLPGSQL_DTYPE_ARRAYELEM:
772-
free_expr(((PLpgSQL_arrayelem *) d)->subscript);
773-
break;
774771
default:
775772
elog(ERROR, "unrecognized data type: %d", d->dtype);
776773
}
@@ -1704,12 +1701,6 @@ plpgsql_dumptree(PLpgSQL_function *func)
17041701
((PLpgSQL_recfield *) d)->fieldname,
17051702
((PLpgSQL_recfield *) d)->recparentno);
17061703
break;
1707-
case PLPGSQL_DTYPE_ARRAYELEM:
1708-
printf("ARRAYELEM of VAR %d subscript ",
1709-
((PLpgSQL_arrayelem *) d)->arrayparentno);
1710-
dump_expr(((PLpgSQL_arrayelem *) d)->subscript);
1711-
printf("\n");
1712-
break;
17131704
default:
17141705
printf("??? unknown data type %d\n", d->dtype);
17151706
}

‎src/pl/plpgsql/src/pl_gram.y

Lines changed: 13 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,10 @@ static void check_raise_parameters(PLpgSQL_stmt_raise *stmt);
177177
%type <list> decl_cursor_arglist
178178
%type <nsitem> decl_aliasitem
179179

180-
%type <expr> expr_until_semi expr_until_rightbracket
180+
%type <expr> expr_until_semi
181181
%type <expr> expr_until_then expr_until_loop opt_expr_until_when
182182
%type <expr> opt_exitcond
183183

184-
%type <datum> assign_var
185184
%type <var> cursor_variable
186185
%type <datum> decl_cursor_arg
187186
%type <forvariable> for_variable
@@ -1155,16 +1154,23 @@ getdiag_item :
11551154
}
11561155
;
11571156

1158-
getdiag_target : assign_var
1157+
getdiag_target : T_DATUM
11591158
{
1160-
if ($1->dtype == PLPGSQL_DTYPE_ROW ||
1161-
$1->dtype == PLPGSQL_DTYPE_REC)
1159+
/*
1160+
* In principle we should support a getdiag_target
1161+
* that is an array element, but for now we don't, so
1162+
* just throw an error if next token is '['.
1163+
*/
1164+
if ($1.datum->dtype == PLPGSQL_DTYPE_ROW ||
1165+
$1.datum->dtype == PLPGSQL_DTYPE_REC ||
1166+
plpgsql_peek() == '[')
11621167
ereport(ERROR,
11631168
(errcode(ERRCODE_SYNTAX_ERROR),
11641169
errmsg("\"%s\" is not a scalar variable",
1165-
((PLpgSQL_variable *) $1)->refname),
1170+
NameOfDatum(&($1))),
11661171
parser_errposition(@1)));
1167-
$$ = $1;
1172+
check_assignable($1.datum, @1);
1173+
$$ = $1.datum;
11681174
}
11691175
| T_WORD
11701176
{
@@ -1178,29 +1184,6 @@ getdiag_target : assign_var
11781184
}
11791185
;
11801186

1181-
1182-
assign_var : T_DATUM
1183-
{
1184-
check_assignable($1.datum, @1);
1185-
$$ = $1.datum;
1186-
}
1187-
| assign_var '[' expr_until_rightbracket
1188-
{
1189-
PLpgSQL_arrayelem *new;
1190-
1191-
new = palloc0(sizeof(PLpgSQL_arrayelem));
1192-
new->dtype = PLPGSQL_DTYPE_ARRAYELEM;
1193-
new->subscript = $3;
1194-
new->arrayparentno = $1->dno;
1195-
/* initialize cached type data to "not valid" */
1196-
new->parenttypoid = InvalidOid;
1197-
1198-
plpgsql_adddatum((PLpgSQL_datum *) new);
1199-
1200-
$$ = (PLpgSQL_datum *) new;
1201-
}
1202-
;
1203-
12041187
stmt_if : K_IF expr_until_then proc_sect stmt_elsifs stmt_else K_END K_IF ';'
12051188
{
12061189
PLpgSQL_stmt_if *new;
@@ -2471,10 +2454,6 @@ expr_until_semi :
24712454
{ $$ = read_sql_expression(';', ";"); }
24722455
;
24732456

2474-
expr_until_rightbracket :
2475-
{ $$ = read_sql_expression(']', "]"); }
2476-
;
2477-
24782457
expr_until_then :
24792458
{ $$ = read_sql_expression(K_THEN, "THEN"); }
24802459
;
@@ -3493,11 +3472,6 @@ check_assignable(PLpgSQL_datum *datum, int location)
34933472
check_assignable(plpgsql_Datums[((PLpgSQL_recfield *) datum)->recparentno],
34943473
location);
34953474
break;
3496-
case PLPGSQL_DTYPE_ARRAYELEM:
3497-
/* assignable if parent array is */
3498-
check_assignable(plpgsql_Datums[((PLpgSQL_arrayelem *) datum)->arrayparentno],
3499-
location);
3500-
break;
35013475
default:
35023476
elog(ERROR, "unrecognized dtype: %d", datum->dtype);
35033477
break;

0 commit comments

Comments
 (0)