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.

diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
index 8e7097c9619160931d9beeac19dbcf8635a077e9..dd4ea9ccdeb6154325ac2be304caaae07cc102de 100644 (file)
--- a/src/pl/plpgsql/src/gram.y
+++ b/src/pl/plpgsql/src/gram.y
@@ -4,7 +4,7 @@
  *                                               procedural language
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.82 2005/10/13 15:34:19 tgl Exp $
+ *       $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.82.2.1 2006/03/23 04:22:44 tgl Exp $
  *
  *       This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -1666,6 +1666,44 @@ lno                              :
 %%
 
 
+#define MAX_EXPR_PARAMS  1024
+
+/*
+ * determine the expression parameter position to use for a plpgsql datum
+ *
+ * It is important that any given plpgsql datum map to just one parameter.
+ * We used to be sloppy and assign a separate parameter for each occurrence
+ * of a datum reference, but that fails for situations such as "select DATUM
+ * from ... group by DATUM".
+ *
+ * The params[] array must be of size MAX_EXPR_PARAMS.
+ */
+static int
+assign_expr_param(int dno, int *params, int *nparams)
+{
+       int             i;
+
+       /* already have an instance of this dno? */
+       for (i = 0; i < *nparams; i++)
+       {
+               if (params[i] == dno)
+                       return i+1;
+       }
+       /* check for array overflow */
+       if (*nparams >= MAX_EXPR_PARAMS)
+       {
+               plpgsql_error_lineno = plpgsql_scanner_lineno();
+               ereport(ERROR,
+                               (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+                                errmsg("too many variables specified in SQL statement")));
+       }
+       /* add new parameter dno to array */
+       params[*nparams] = dno;
+       (*nparams)++;
+       return *nparams;
+}
+
+
 PLpgSQL_expr *
 plpgsql_read_expression(int until, const char *expected)
 {
@@ -1704,7 +1742,7 @@ read_sql_construct(int until,
        PLpgSQL_dstring         ds;
        int                                     parenlevel = 0;
        int                                     nparams = 0;
-       int                                     params[1024];
+       int                                     params[MAX_EXPR_PARAMS];
        char                            buf[32];
        PLpgSQL_expr            *expr;
 
@@ -1756,32 +1794,26 @@ read_sql_construct(int until,
                if (plpgsql_SpaceScanned)
                        plpgsql_dstring_append(&ds, " ");
 
-               /* Check for array overflow */
-               if (nparams >= 1024)
-               {
-                       plpgsql_error_lineno = lno;
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-                                        errmsg("too many variables specified in SQL statement")));
-               }
-
                switch (tok)
                {
                        case T_SCALAR:
-                               params[nparams] = yylval.scalar->dno;
-                               snprintf(buf, sizeof(buf), " $%d ", ++nparams);
+                               snprintf(buf, sizeof(buf), " $%d ",
+                                                assign_expr_param(yylval.scalar->dno,
+                                                                                  params, &nparams));
                                plpgsql_dstring_append(&ds, buf);
                                break;
 
                        case T_ROW:
-                               params[nparams] = yylval.row->rowno;
-                               snprintf(buf, sizeof(buf), " $%d ", ++nparams);
+                               snprintf(buf, sizeof(buf), " $%d ",
+                                                assign_expr_param(yylval.row->rowno,
+                                                                                  params, &nparams));
                                plpgsql_dstring_append(&ds, buf);
                                break;
 
                        case T_RECORD:
-                               params[nparams] = yylval.rec->recno;
-                               snprintf(buf, sizeof(buf), " $%d ", ++nparams);
+                               snprintf(buf, sizeof(buf), " $%d ",
+                                                assign_expr_param(yylval.rec->recno,
+                                                                                  params, &nparams));
                                plpgsql_dstring_append(&ds, buf);
                                break;
 
@@ -1879,7 +1911,7 @@ make_select_stmt(void)
 {
        PLpgSQL_dstring         ds;
        int                                     nparams = 0;
-       int                                     params[1024];
+       int                                     params[MAX_EXPR_PARAMS];
        char                            buf[32];
        PLpgSQL_expr            *expr;
        PLpgSQL_row                     *row = NULL;
@@ -1942,32 +1974,26 @@ make_select_stmt(void)
                if (plpgsql_SpaceScanned)
                        plpgsql_dstring_append(&ds, " ");
 
-               /* Check for array overflow */
-               if (nparams >= 1024)
-               {
-                       plpgsql_error_lineno = plpgsql_scanner_lineno();
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-                                        errmsg("too many parameters specified in SQL statement")));
-               }
-
                switch (tok)
                {
                        case T_SCALAR:
-                               params[nparams] = yylval.scalar->dno;
-                               snprintf(buf, sizeof(buf), " $%d ", ++nparams);
+                               snprintf(buf, sizeof(buf), " $%d ",
+                                                assign_expr_param(yylval.scalar->dno,
+                                                                                  params, &nparams));
                                plpgsql_dstring_append(&ds, buf);
                                break;
 
                        case T_ROW:
-                               params[nparams] = yylval.row->rowno;
-                               snprintf(buf, sizeof(buf), " $%d ", ++nparams);
+                               snprintf(buf, sizeof(buf), " $%d ",
+                                                assign_expr_param(yylval.row->rowno,
+                                                                                  params, &nparams));
                                plpgsql_dstring_append(&ds, buf);
                                break;
 
                        case T_RECORD:
-                               params[nparams] = yylval.rec->recno;
-                               snprintf(buf, sizeof(buf), " $%d ", ++nparams);
+                               snprintf(buf, sizeof(buf), " $%d ",
+                                                assign_expr_param(yylval.rec->recno,
+                                                                                  params, &nparams));
                                plpgsql_dstring_append(&ds, buf);
                                break;
 

diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index aef73e1eee79611cb0f43d08a26e5510e36bdc3a..961fe9f46ee25210dc50f01d2e5998a24fda5d4d 100644 (file)
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -2721,3 +2721,21 @@ end;
 $$ language plpgsql;
 ERROR:  end label "outer_label" specified for unlabelled block
 CONTEXT:  compile of PL/pgSQL function "end_label4" near line 5
+-- regression test: verify that multiple uses of same plpgsql datum within
+-- a SQL command all get mapped to the same $n parameter.  The return value
+-- of the SELECT is not important, we only care that it doesn't fail with
+-- a complaint about an ungrouped column reference.
+create function multi_datum_use(p1 int) returns bool as $$
+declare
+  x int;
+  y int;
+begin
+  select into x,y unique1/p1, unique1/$1 from tenk1 group by unique1/p1;
+  return x = y;
+end$$ language plpgsql;
+select multi_datum_use(42);
+ multi_datum_use 
+-----------------
+ t
+(1 row)
+

diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index fdb2f46ff89d720bf24ff3989f64ea8c8ff85475..6bffb3e13b619d079d1c4afc1c668ec4d31f5305 100644 (file)
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -2280,3 +2280,18 @@ begin
   end loop outer_label;
 end;
 $$ language plpgsql;
+
+-- regression test: verify that multiple uses of same plpgsql datum within
+-- a SQL command all get mapped to the same $n parameter.  The return value
+-- of the SELECT is not important, we only care that it doesn't fail with
+-- a complaint about an ungrouped column reference.
+create function multi_datum_use(p1 int) returns bool as $$
+declare
+  x int;
+  y int;
+begin
+  select into x,y unique1/p1, unique1/$1 from tenk1 group by unique1/p1;
+  return x = y;
+end$$ language plpgsql;
+
+select multi_datum_use(42);