Skip to content

Commit 4fb9271

Browse files
tglsfdctglsfdc
committed
Fix plpgsql to pass only one copy of any given plpgsql variable into a SQL
command or expression, rather than one copy for each textual occurrence as it did before. This might result in some small performance improvement, but the compelling reason to do it is that not doing so can result in unexpected grouping failures because the main SQL parser won't see different parameter numbers as equivalent. Add a regression test for the failure case. Per report from Robert Davidson.
1 parent 19956e0 commit 4fb9271

File tree

3 files changed

+92
-33
lines changed

3 files changed

+92
-33
lines changed

‎src/pl/plpgsql/src/gram.y‎

Lines changed: 59 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.87 2006/03/09 21:29:36 momjian Exp $
12+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.88 2006/03/23 04:22:36 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -1714,6 +1714,44 @@ lno :
17141714
%%
17151715

17161716

1717+
#define MAX_EXPR_PARAMS 1024
1718+
1719+
/*
1720+
* determine the expression parameter position to use for a plpgsql datum
1721+
*
1722+
* It is important that any given plpgsql datum map to just one parameter.
1723+
* We used to be sloppy and assign a separate parameter for each occurrence
1724+
* of a datum reference, but that fails for situations such as "select DATUM
1725+
* from ... group by DATUM".
1726+
*
1727+
* The params[] array must be of size MAX_EXPR_PARAMS.
1728+
*/
1729+
static int
1730+
assign_expr_param(int dno, int *params, int *nparams)
1731+
{
1732+
int i;
1733+
1734+
/* already have an instance of this dno? */
1735+
for (i = 0; i < *nparams; i++)
1736+
{
1737+
if (params[i] == dno)
1738+
return i+1;
1739+
}
1740+
/* check for array overflow */
1741+
if (*nparams >= MAX_EXPR_PARAMS)
1742+
{
1743+
plpgsql_error_lineno = plpgsql_scanner_lineno();
1744+
ereport(ERROR,
1745+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
1746+
errmsg("too many variables specified in SQL statement")));
1747+
}
1748+
/* add new parameter dno to array */
1749+
params[*nparams] = dno;
1750+
(*nparams)++;
1751+
return *nparams;
1752+
}
1753+
1754+
17171755
PLpgSQL_expr *
17181756
plpgsql_read_expression(int until, const char *expected)
17191757
{
@@ -1752,7 +1790,7 @@ read_sql_construct(int until,
17521790
PLpgSQL_dstring ds;
17531791
int parenlevel = 0;
17541792
int nparams = 0;
1755-
int params[1024];
1793+
int params[MAX_EXPR_PARAMS];
17561794
char buf[32];
17571795
PLpgSQL_expr *expr;
17581796

@@ -1804,32 +1842,26 @@ read_sql_construct(int until,
18041842
if (plpgsql_SpaceScanned)
18051843
plpgsql_dstring_append(&ds, " ");
18061844

1807-
/* Check for array overflow */
1808-
if (nparams >= 1024)
1809-
{
1810-
plpgsql_error_lineno = lno;
1811-
ereport(ERROR,
1812-
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
1813-
errmsg("too many variables specified in SQL statement")));
1814-
}
1815-
18161845
switch (tok)
18171846
{
18181847
case T_SCALAR:
1819-
params[nparams] = yylval.scalar->dno;
1820-
snprintf(buf, sizeof(buf), " $%d ", ++nparams);
1848+
snprintf(buf, sizeof(buf), " $%d ",
1849+
assign_expr_param(yylval.scalar->dno,
1850+
params, &nparams));
18211851
plpgsql_dstring_append(&ds, buf);
18221852
break;
18231853

18241854
case T_ROW:
1825-
params[nparams] = yylval.row->rowno;
1826-
snprintf(buf, sizeof(buf), " $%d ", ++nparams);
1855+
snprintf(buf, sizeof(buf), " $%d ",
1856+
assign_expr_param(yylval.row->rowno,
1857+
params, &nparams));
18271858
plpgsql_dstring_append(&ds, buf);
18281859
break;
18291860

18301861
case T_RECORD:
1831-
params[nparams] = yylval.rec->recno;
1832-
snprintf(buf, sizeof(buf), " $%d ", ++nparams);
1862+
snprintf(buf, sizeof(buf), " $%d ",
1863+
assign_expr_param(yylval.rec->recno,
1864+
params, &nparams));
18331865
plpgsql_dstring_append(&ds, buf);
18341866
break;
18351867

@@ -1927,7 +1959,7 @@ make_select_stmt(void)
19271959
{
19281960
PLpgSQL_dstring ds;
19291961
int nparams = 0;
1930-
int params[1024];
1962+
int params[MAX_EXPR_PARAMS];
19311963
char buf[32];
19321964
PLpgSQL_expr *expr;
19331965
PLpgSQL_row *row = NULL;
@@ -1992,32 +2024,26 @@ make_select_stmt(void)
19922024
if (plpgsql_SpaceScanned)
19932025
plpgsql_dstring_append(&ds, " ");
19942026

1995-
/* Check for array overflow */
1996-
if (nparams >= 1024)
1997-
{
1998-
plpgsql_error_lineno = plpgsql_scanner_lineno();
1999-
ereport(ERROR,
2000-
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2001-
errmsg("too many parameters specified in SQL statement")));
2002-
}
2003-
20042027
switch (tok)
20052028
{
20062029
case T_SCALAR:
2007-
params[nparams] = yylval.scalar->dno;
2008-
snprintf(buf, sizeof(buf), " $%d ", ++nparams);
2030+
snprintf(buf, sizeof(buf), " $%d ",
2031+
assign_expr_param(yylval.scalar->dno,
2032+
params, &nparams));
20092033
plpgsql_dstring_append(&ds, buf);
20102034
break;
20112035

20122036
case T_ROW:
2013-
params[nparams] = yylval.row->rowno;
2014-
snprintf(buf, sizeof(buf), " $%d ", ++nparams);
2037+
snprintf(buf, sizeof(buf), " $%d ",
2038+
assign_expr_param(yylval.row->rowno,
2039+
params, &nparams));
20152040
plpgsql_dstring_append(&ds, buf);
20162041
break;
20172042

20182043
case T_RECORD:
2019-
params[nparams] = yylval.rec->recno;
2020-
snprintf(buf, sizeof(buf), " $%d ", ++nparams);
2044+
snprintf(buf, sizeof(buf), " $%d ",
2045+
assign_expr_param(yylval.rec->recno,
2046+
params, &nparams));
20212047
plpgsql_dstring_append(&ds, buf);
20222048
break;
20232049

‎src/test/regress/expected/plpgsql.out‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2776,3 +2776,21 @@ NOTICE: 4 bb cc
27762776

27772777
(1 row)
27782778

2779+
-- regression test: verify that multiple uses of same plpgsql datum within
2780+
-- a SQL command all get mapped to the same $n parameter. The return value
2781+
-- of the SELECT is not important, we only care that it doesn't fail with
2782+
-- a complaint about an ungrouped column reference.
2783+
create function multi_datum_use(p1 int) returns bool as $$
2784+
declare
2785+
x int;
2786+
y int;
2787+
begin
2788+
select into x,y unique1/p1, unique1/$1 from tenk1 group by unique1/p1;
2789+
return x = y;
2790+
end$$ language plpgsql;
2791+
select multi_datum_use(42);
2792+
multi_datum_use
2793+
-----------------
2794+
t
2795+
(1 row)
2796+

‎src/test/regress/sql/plpgsql.sql‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2309,3 +2309,18 @@ end;
23092309
$proc$ language plpgsql;
23102310

23112311
select for_vect();
2312+
2313+
-- regression test: verify that multiple uses of same plpgsql datum within
2314+
-- a SQL command all get mapped to the same $n parameter. The return value
2315+
-- of the SELECT is not important, we only care that it doesn't fail with
2316+
-- a complaint about an ungrouped column reference.
2317+
create function multi_datum_use(p1 int) returns bool as $$
2318+
declare
2319+
x int;
2320+
y int;
2321+
begin
2322+
select into x,y unique1/p1, unique1/$1 from tenk1 group by unique1/p1;
2323+
return x = y;
2324+
end$$ language plpgsql;
2325+
2326+
select multi_datum_use(42);

0 commit comments

Comments
 (0)