🌐 AI搜索 & 代理 主页
Skip to content

Commit dd759b9

Browse files
tglsfdctglsfdc
committed
In plpgsql, unify duplicate variables for record and row cases.
plpgsql's function exec_move_row() handles assignment of a composite source value to either a PLpgSQL_rec or PLpgSQL_row target variable. Oddly, rather than taking a single target argument which it could do run-time type detection on, it was coded to take two separate arguments (only one of which is allowed to be non-NULL). This choice had then back-propagated into storing two separate target variables in various plpgsql statement nodes, with lots of duplicative coding and awkward interface logic to support that. Simplify matters by folding those pairs down to single variables, distinguishing the two cases only where we must ... which turns out to be only in exec_move_row itself. This is purely refactoring and should not change any behavior. In passing, remove unused field PLpgSQL_stmt_open.returntype. Discussion: https://postgr.es/m/11787.1512713374@sss.pgh.pa.us
1 parent 2d2d06b commit dd759b9

File tree

5 files changed

+106
-194
lines changed

5 files changed

+106
-194
lines changed

‎src/pl/plpgsql/src/pl_comp.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ do_compile(FunctionCallInfo fcinfo,
545545
{
546546
if (rettypeid == VOIDOID ||
547547
rettypeid == RECORDOID)
548-
/* okay */ ;
548+
/* okay */ ;
549549
else if (rettypeid == TRIGGEROID || rettypeid == EVTTRIGGEROID)
550550
ereport(ERROR,
551551
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),

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

Lines changed: 43 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -272,17 +272,15 @@ static ParamListInfo setup_unshared_param_list(PLpgSQL_execstate *estate,
272272
PLpgSQL_expr *expr);
273273
static void plpgsql_param_fetch(ParamListInfo params, int paramid);
274274
static void exec_move_row(PLpgSQL_execstate *estate,
275-
PLpgSQL_rec *rec,
276-
PLpgSQL_row *row,
275+
PLpgSQL_variable *target,
277276
HeapTuple tup, TupleDesc tupdesc);
278277
static HeapTuple make_tuple_from_row(PLpgSQL_execstate *estate,
279278
PLpgSQL_row *row,
280279
TupleDesc tupdesc);
281280
static HeapTuple get_tuple_from_datum(Datum value);
282281
static TupleDesc get_tupdesc_from_datum(Datum value);
283282
static void exec_move_row_from_datum(PLpgSQL_execstate *estate,
284-
PLpgSQL_rec *rec,
285-
PLpgSQL_row *row,
283+
PLpgSQL_variable *target,
286284
Datum value);
287285
static char *convert_value_to_string(PLpgSQL_execstate *estate,
288286
Datum value, Oid valtype);
@@ -425,13 +423,15 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo,
425423
if (!fcinfo->argnull[i])
426424
{
427425
/* Assign row value from composite datum */
428-
exec_move_row_from_datum(&estate, NULL, row,
426+
exec_move_row_from_datum(&estate,
427+
(PLpgSQL_variable *) row,
429428
fcinfo->arg[i]);
430429
}
431430
else
432431
{
433432
/* If arg is null, treat it as an empty row */
434-
exec_move_row(&estate, NULL, row, NULL, NULL);
433+
exec_move_row(&estate, (PLpgSQL_variable *) row,
434+
NULL, NULL);
435435
}
436436
/* clean up after exec_move_row() */
437437
exec_eval_cleanup(&estate);
@@ -2327,7 +2327,7 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt)
23272327
set_args.sqlstmt = stmt->argquery;
23282328
set_args.into = true;
23292329
/* XXX historically this has not been STRICT */
2330-
set_args.row = (PLpgSQL_row *)
2330+
set_args.target = (PLpgSQL_variable *)
23312331
(estate->datums[curvar->cursor_explicit_argrow]);
23322332

23332333
if (exec_stmt_execsql(estate, &set_args) != PLPGSQL_RC_OK)
@@ -3755,22 +3755,16 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
37553755
{
37563756
SPITupleTable *tuptab = SPI_tuptable;
37573757
uint64 n = SPI_processed;
3758-
PLpgSQL_rec *rec = NULL;
3759-
PLpgSQL_row *row = NULL;
3758+
PLpgSQL_variable *target;
37603759

37613760
/* If the statement did not return a tuple table, complain */
37623761
if (tuptab == NULL)
37633762
ereport(ERROR,
37643763
(errcode(ERRCODE_SYNTAX_ERROR),
37653764
errmsg("INTO used with a command that cannot return data")));
37663765

3767-
/* Determine if we assign to a record or a row */
3768-
if (stmt->rec != NULL)
3769-
rec = (PLpgSQL_rec *) (estate->datums[stmt->rec->dno]);
3770-
else if (stmt->row != NULL)
3771-
row = (PLpgSQL_row *) (estate->datums[stmt->row->dno]);
3772-
else
3773-
elog(ERROR, "unsupported target");
3766+
/* Fetch target's datum entry */
3767+
target = (PLpgSQL_variable *) estate->datums[stmt->target->dno];
37743768

37753769
/*
37763770
* If SELECT ... INTO specified STRICT, and the query didn't find
@@ -3794,7 +3788,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
37943788
errdetail ? errdetail_internal("parameters: %s", errdetail) : 0));
37953789
}
37963790
/* set the target to NULL(s) */
3797-
exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
3791+
exec_move_row(estate, target, NULL, tuptab->tupdesc);
37983792
}
37993793
else
38003794
{
@@ -3813,7 +3807,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
38133807
errdetail ? errdetail_internal("parameters: %s", errdetail) : 0));
38143808
}
38153809
/* Put the first result row into the target */
3816-
exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc);
3810+
exec_move_row(estate, target, tuptab->vals[0], tuptab->tupdesc);
38173811
}
38183812

38193813
/* Clean up */
@@ -3946,22 +3940,16 @@ exec_stmt_dynexecute(PLpgSQL_execstate *estate,
39463940
{
39473941
SPITupleTable *tuptab = SPI_tuptable;
39483942
uint64 n = SPI_processed;
3949-
PLpgSQL_rec *rec = NULL;
3950-
PLpgSQL_row *row = NULL;
3943+
PLpgSQL_variable *target;
39513944

39523945
/* If the statement did not return a tuple table, complain */
39533946
if (tuptab == NULL)
39543947
ereport(ERROR,
39553948
(errcode(ERRCODE_SYNTAX_ERROR),
39563949
errmsg("INTO used with a command that cannot return data")));
39573950

3958-
/* Determine if we assign to a record or a row */
3959-
if (stmt->rec != NULL)
3960-
rec = (PLpgSQL_rec *) (estate->datums[stmt->rec->dno]);
3961-
else if (stmt->row != NULL)
3962-
row = (PLpgSQL_row *) (estate->datums[stmt->row->dno]);
3963-
else
3964-
elog(ERROR, "unsupported target");
3951+
/* Fetch target's datum entry */
3952+
target = (PLpgSQL_variable *) estate->datums[stmt->target->dno];
39653953

39663954
/*
39673955
* If SELECT ... INTO specified STRICT, and the query didn't find
@@ -3985,7 +3973,7 @@ exec_stmt_dynexecute(PLpgSQL_execstate *estate,
39853973
errdetail ? errdetail_internal("parameters: %s", errdetail) : 0));
39863974
}
39873975
/* set the target to NULL(s) */
3988-
exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
3976+
exec_move_row(estate, target, NULL, tuptab->tupdesc);
39893977
}
39903978
else
39913979
{
@@ -4005,7 +3993,7 @@ exec_stmt_dynexecute(PLpgSQL_execstate *estate,
40053993
}
40063994

40073995
/* Put the first result row into the target */
4008-
exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc);
3996+
exec_move_row(estate, target, tuptab->vals[0], tuptab->tupdesc);
40093997
}
40103998
/* clean up after exec_move_row() */
40113999
exec_eval_cleanup(estate);
@@ -4163,7 +4151,7 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt)
41634151
set_args.sqlstmt = stmt->argquery;
41644152
set_args.into = true;
41654153
/* XXX historically this has not been STRICT */
4166-
set_args.row = (PLpgSQL_row *)
4154+
set_args.target = (PLpgSQL_variable *)
41674155
(estate->datums[curvar->cursor_explicit_argrow]);
41684156

41694157
if (exec_stmt_execsql(estate, &set_args) != PLPGSQL_RC_OK)
@@ -4221,8 +4209,6 @@ static int
42214209
exec_stmt_fetch(PLpgSQL_execstate *estate, PLpgSQL_stmt_fetch *stmt)
42224210
{
42234211
PLpgSQL_var *curvar;
4224-
PLpgSQL_rec *rec = NULL;
4225-
PLpgSQL_row *row = NULL;
42264212
long how_many = stmt->how_many;
42274213
SPITupleTable *tuptab;
42284214
Portal portal;
@@ -4269,16 +4255,7 @@ exec_stmt_fetch(PLpgSQL_execstate *estate, PLpgSQL_stmt_fetch *stmt)
42694255

42704256
if (!stmt->is_move)
42714257
{
4272-
/* ----------
4273-
* Determine if we fetch into a record or a row
4274-
* ----------
4275-
*/
4276-
if (stmt->rec != NULL)
4277-
rec = (PLpgSQL_rec *) (estate->datums[stmt->rec->dno]);
4278-
else if (stmt->row != NULL)
4279-
row = (PLpgSQL_row *) (estate->datums[stmt->row->dno]);
4280-
else
4281-
elog(ERROR, "unsupported target");
4258+
PLpgSQL_variable *target;
42824259

42834260
/* ----------
42844261
* Fetch 1 tuple from the cursor
@@ -4292,10 +4269,11 @@ exec_stmt_fetch(PLpgSQL_execstate *estate, PLpgSQL_stmt_fetch *stmt)
42924269
* Set the target appropriately.
42934270
* ----------
42944271
*/
4272+
target = (PLpgSQL_variable *) estate->datums[stmt->target->dno];
42954273
if (n == 0)
4296-
exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
4274+
exec_move_row(estate, target, NULL, tuptab->tupdesc);
42974275
else
4298-
exec_move_row(estate, rec, row, tuptab->vals[0], tuptab->tupdesc);
4276+
exec_move_row(estate, target, tuptab->vals[0], tuptab->tupdesc);
42994277

43004278
exec_eval_cleanup(estate);
43014279
SPI_freetuptable(tuptab);
@@ -4514,7 +4492,8 @@ exec_assign_value(PLpgSQL_execstate *estate,
45144492
if (isNull)
45154493
{
45164494
/* If source is null, just assign nulls to the row */
4517-
exec_move_row(estate, NULL, row, NULL, NULL);
4495+
exec_move_row(estate, (PLpgSQL_variable *) row,
4496+
NULL, NULL);
45184497
}
45194498
else
45204499
{
@@ -4523,7 +4502,8 @@ exec_assign_value(PLpgSQL_execstate *estate,
45234502
ereport(ERROR,
45244503
(errcode(ERRCODE_DATATYPE_MISMATCH),
45254504
errmsg("cannot assign non-composite value to a row variable")));
4526-
exec_move_row_from_datum(estate, NULL, row, value);
4505+
exec_move_row_from_datum(estate, (PLpgSQL_variable *) row,
4506+
value);
45274507
}
45284508
break;
45294509
}
@@ -4538,7 +4518,8 @@ exec_assign_value(PLpgSQL_execstate *estate,
45384518
if (isNull)
45394519
{
45404520
/* If source is null, just assign nulls to the record */
4541-
exec_move_row(estate, rec, NULL, NULL, NULL);
4521+
exec_move_row(estate, (PLpgSQL_variable *) rec,
4522+
NULL, NULL);
45424523
}
45434524
else
45444525
{
@@ -4547,7 +4528,8 @@ exec_assign_value(PLpgSQL_execstate *estate,
45474528
ereport(ERROR,
45484529
(errcode(ERRCODE_DATATYPE_MISMATCH),
45494530
errmsg("cannot assign non-composite value to a record variable")));
4550-
exec_move_row_from_datum(estate, rec, NULL, value);
4531+
exec_move_row_from_datum(estate, (PLpgSQL_variable *) rec,
4532+
value);
45514533
}
45524534
break;
45534535
}
@@ -5341,22 +5323,14 @@ static int
53415323
exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
53425324
Portal portal, bool prefetch_ok)
53435325
{
5344-
PLpgSQL_rec *rec = NULL;
5345-
PLpgSQL_row *row = NULL;
5326+
PLpgSQL_variable *var;
53465327
SPITupleTable *tuptab;
53475328
bool found = false;
53485329
int rc = PLPGSQL_RC_OK;
53495330
uint64 n;
53505331

5351-
/*
5352-
* Determine if we assign to a record or a row
5353-
*/
5354-
if (stmt->rec != NULL)
5355-
rec = (PLpgSQL_rec *) (estate->datums[stmt->rec->dno]);
5356-
else if (stmt->row != NULL)
5357-
row = (PLpgSQL_row *) (estate->datums[stmt->row->dno]);
5358-
else
5359-
elog(ERROR, "unsupported target");
5332+
/* Fetch loop variable's datum entry */
5333+
var = (PLpgSQL_variable *) estate->datums[stmt->var->dno];
53605334

53615335
/*
53625336
* Make sure the portal doesn't get closed by the user statements we
@@ -5379,7 +5353,7 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
53795353
*/
53805354
if (n == 0)
53815355
{
5382-
exec_move_row(estate, rec, row, NULL, tuptab->tupdesc);
5356+
exec_move_row(estate, var, NULL, tuptab->tupdesc);
53835357
exec_eval_cleanup(estate);
53845358
}
53855359
else
@@ -5397,7 +5371,7 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
53975371
/*
53985372
* Assign the tuple to the target
53995373
*/
5400-
exec_move_row(estate, rec, row, tuptab->vals[i], tuptab->tupdesc);
5374+
exec_move_row(estate, var, tuptab->vals[i], tuptab->tupdesc);
54015375
exec_eval_cleanup(estate);
54025376

54035377
/*
@@ -5949,16 +5923,17 @@ plpgsql_param_fetch(ParamListInfo params, int paramid)
59495923
*/
59505924
static void
59515925
exec_move_row(PLpgSQL_execstate *estate,
5952-
PLpgSQL_rec *rec,
5953-
PLpgSQL_row *row,
5926+
PLpgSQL_variable *target,
59545927
HeapTuple tup, TupleDesc tupdesc)
59555928
{
59565929
/*
59575930
* Record is simple - just copy the tuple and its descriptor into the
59585931
* record variable
59595932
*/
5960-
if (rec != NULL)
5933+
if (target->dtype == PLPGSQL_DTYPE_REC)
59615934
{
5935+
PLpgSQL_rec *rec = (PLpgSQL_rec *) target;
5936+
59625937
/*
59635938
* Copy input first, just in case it is pointing at variable's value
59645939
*/
@@ -6027,8 +6002,9 @@ exec_move_row(PLpgSQL_execstate *estate,
60276002
* If we have no tuple data at all, we'll assign NULL to all columns of
60286003
* the row variable.
60296004
*/
6030-
if (row != NULL)
6005+
if (target->dtype == PLPGSQL_DTYPE_ROW)
60316006
{
6007+
PLpgSQL_row *row = (PLpgSQL_row *) target;
60326008
int td_natts = tupdesc ? tupdesc->natts : 0;
60336009
int t_natts;
60346010
int fnum;
@@ -6195,8 +6171,7 @@ get_tupdesc_from_datum(Datum value)
61956171
*/
61966172
static void
61976173
exec_move_row_from_datum(PLpgSQL_execstate *estate,
6198-
PLpgSQL_rec *rec,
6199-
PLpgSQL_row *row,
6174+
PLpgSQL_variable *target,
62006175
Datum value)
62016176
{
62026177
HeapTupleHeader td = DatumGetHeapTupleHeader(value);
@@ -6217,7 +6192,7 @@ exec_move_row_from_datum(PLpgSQL_execstate *estate,
62176192
tmptup.t_data = td;
62186193

62196194
/* Do the move */
6220-
exec_move_row(estate, rec, row, &tmptup, tupdesc);
6195+
exec_move_row(estate, target, &tmptup, tupdesc);
62216196

62226197
/* Release tupdesc usage count */
62236198
ReleaseTupleDesc(tupdesc);

0 commit comments

Comments
 (0)