From ae1b7e298cd99e808ec8410a808fcb2e44f8520e Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 16 Jul 2007 17:01:11 +0000
Subject: [PATCH] Allow plpgsql function parameter names to be qualified with
 the function's name.  With this patch, it is always possible for the user to
 qualify a plpgsql variable name if needed to avoid ambiguity.  While there is
 much more work to be done in this area, this simple change removes one
 unnecessary incompatibility with Oracle.  Per discussion.

---
 doc/src/sgml/plpgsql.sgml             | 42 ++++++++++++++++++++++-----
 src/pl/plpgsql/src/gram.y             |  4 +--
 src/pl/plpgsql/src/pl_comp.c          |  8 +++--
 src/pl/plpgsql/src/pl_funcs.c         | 22 +++++++-------
 src/pl/plpgsql/src/plpgsql.h          |  6 ++--
 src/test/regress/expected/plpgsql.out | 28 ++++++++++++++++++
 src/test/regress/sql/plpgsql.sql      | 22 ++++++++++++++
 7 files changed, 104 insertions(+), 28 deletions(-)

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 59ec377ea9d..24bcf52a4f7 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/plpgsql.sgml,v 1.114 2007/07/15 00:45:16 tgl Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/plpgsql.sgml,v 1.115 2007/07/16 17:01:10 tgl Exp $ -->
 
 <chapter id="plpgsql"> 
   <title><application>PL/pgSQL</application> - <acronym>SQL</acronym> Procedural Language</title>
@@ -249,10 +249,23 @@ $$ LANGUAGE plpgsql;
 </programlisting>
     </para>
 
+    <note>
+     <para>
+      There is actually a hidden <quote>outer block</> surrounding the body
+      of any <application>PL/pgSQL</> function.  This block provides the
+      declarations of the function's parameters (if any), as well as some
+      special variables such as <literal>FOUND</literal> (see
+      <xref linkend="plpgsql-statements-diagnostics">).  The outer block is
+      labeled with the function's name, meaning that parameters and special
+      variables can be qualified with the function's name.
+     </para>
+    </note>
+
     <para>
      It is important not to confuse the use of
      <command>BEGIN</>/<command>END</> for grouping statements in
-     <application>PL/pgSQL</> with the database commands for transaction
+     <application>PL/pgSQL</> with the similarly-named SQL commands
+     for transaction
      control.  <application>PL/pgSQL</>'s <command>BEGIN</>/<command>END</>
      are only for grouping; they do not start or end a transaction.
      Functions and trigger procedures are always executed within a transaction
@@ -370,6 +383,19 @@ BEGIN
 END;
 $$ LANGUAGE plpgsql;
 </programlisting>
+     </para>
+
+    <note>
+     <para>
+      These two examples are not perfectly equivalent.  In the first case,
+      <literal>subtotal</> could be referenced as
+      <literal>sales_tax.subtotal</>, but in the second case it could not.
+      (Had we attached a label to the block, <literal>subtotal</> could
+      be qualified with that label, instead.)
+     </para>
+    </note>
+
+     <para>
       Some more examples:
 <programlisting>
 CREATE FUNCTION instr(varchar, integer) RETURNS integer AS $$
@@ -3618,12 +3644,12 @@ a_output := a_output || $$ if v_$$ || referrer_keys.kind || $$ like '$$
 
      <listitem>
       <para>
-       You cannot use parameter names that are the same as columns
-       that are referenced in the function. Oracle allows you to do this
-       if you qualify the parameter name using
-       <literal>function_name.parameter_name</>.
-       In <application>PL/pgSQL</>, you can instead avoid a conflict by
-       qualifying the column or table name.
+       If a name used in a SQL command could be either a column name of a
+       table or a reference to a variable of the function,
+       <application>PL/SQL</> treats it as a column name, while
+       <application>PL/pgSQL</> treats it as a variable name.  It's best
+       to avoid such ambiguities in the first place, but if necessary you
+       can fix them by properly qualifying the ambiguous name.
        (See <xref linkend="plpgsql-var-subst">.)
       </para>
      </listitem>
diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
index 0f41c999724..795d3c4c4bd 100644
--- a/src/pl/plpgsql/src/gram.y
+++ b/src/pl/plpgsql/src/gram.y
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.103 2007/07/15 02:15:04 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.104 2007/07/16 17:01:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -366,7 +366,7 @@ decl_statement	: decl_varname decl_const decl_datatype decl_notnull decl_defval
 						plpgsql_ns_rename($2, $4);
 					}
 				| decl_varname opt_scrollable K_CURSOR
-					{ plpgsql_ns_push(NULL); }
+					{ plpgsql_ns_push($1.name); }
 				  decl_cursor_args decl_is_for decl_cursor_query
 					{
 						PLpgSQL_var *new;
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 0293bc3fe07..db150632f05 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_comp.c,v 1.116 2007/06/26 16:48:09 alvherre Exp $
+ *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_comp.c,v 1.117 2007/07/16 17:01:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -306,10 +306,12 @@ do_compile(FunctionCallInfo fcinfo,
 	error_context_stack = &plerrcontext;
 
 	/*
-	 * Initialize the compiler
+	 * Initialize the compiler, particularly the namespace stack.  The
+	 * outermost namespace contains function parameters and other special
+	 * variables (such as FOUND), and is named after the function itself.
 	 */
 	plpgsql_ns_init();
-	plpgsql_ns_push(NULL);
+	plpgsql_ns_push(NameStr(procStruct->proname));
 	plpgsql_DumpExecTree = false;
 
 	datums_alloc = 128;
diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c
index 4d5eba73e3b..da128daa7f7 100644
--- a/src/pl/plpgsql/src/pl_funcs.c
+++ b/src/pl/plpgsql/src/pl_funcs.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_funcs.c,v 1.60 2007/07/15 02:15:04 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_funcs.c,v 1.61 2007/07/16 17:01:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -147,15 +147,14 @@ plpgsql_ns_setlocal(bool flag)
  * ----------
  */
 void
-plpgsql_ns_push(char *label)
+plpgsql_ns_push(const char *label)
 {
 	PLpgSQL_ns *new;
 
 	if (label == NULL)
 		label = "";
 
-	new = palloc(sizeof(PLpgSQL_ns));
-	memset(new, 0, sizeof(PLpgSQL_ns));
+	new = palloc0(sizeof(PLpgSQL_ns));
 	new->upper = ns_current;
 	ns_current = new;
 
@@ -224,7 +223,7 @@ plpgsql_ns_additem(int itemtype, int itemno, const char *name)
  * ----------
  */
 PLpgSQL_nsitem *
-plpgsql_ns_lookup(char *name, char *label)
+plpgsql_ns_lookup(const char *name, const char *label)
 {
 	PLpgSQL_ns *ns;
 	int			i;
@@ -236,11 +235,11 @@ plpgsql_ns_lookup(char *name, char *label)
 	{
 		for (ns = ns_current; ns != NULL; ns = ns->upper)
 		{
-			if (!strcmp(ns->items[0]->name, label))
+			if (strcmp(ns->items[0]->name, label) == 0)
 			{
 				for (i = 1; i < ns->items_used; i++)
 				{
-					if (!strcmp(ns->items[i]->name, name))
+					if (strcmp(ns->items[i]->name, name) == 0)
 						return ns->items[i];
 				}
 				return NULL;	/* name not found in specified label */
@@ -254,7 +253,7 @@ plpgsql_ns_lookup(char *name, char *label)
 	 */
 	for (ns = ns_current; ns != NULL; ns = ns->upper)
 	{
-		if (!strcmp(ns->items[0]->name, name))
+		if (strcmp(ns->items[0]->name, name) == 0)
 			return ns->items[0];
 	}
 
@@ -265,7 +264,7 @@ plpgsql_ns_lookup(char *name, char *label)
 	{
 		for (i = 1; i < ns->items_used; i++)
 		{
-			if (!strcmp(ns->items[i]->name, name))
+			if (strcmp(ns->items[i]->name, name) == 0)
 				return ns->items[i];
 		}
 		if (ns_localmode)
@@ -288,14 +287,13 @@ plpgsql_ns_rename(char *oldname, char *newname)
 	int			i;
 
 	/*
-	 * Lookup name in the namestack; do the lookup in the current namespace
-	 * only.
+	 * Lookup name in the namestack
 	 */
 	for (ns = ns_current; ns != NULL; ns = ns->upper)
 	{
 		for (i = 1; i < ns->items_used; i++)
 		{
-			if (!strcmp(ns->items[i]->name, oldname))
+			if (strcmp(ns->items[i]->name, oldname) == 0)
 			{
 				newitem = palloc(sizeof(PLpgSQL_nsitem) + strlen(newname));
 				newitem->itemtype = ns->items[i]->itemtype;
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index dce7be2b1bd..6ffb8b41958 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.89 2007/07/15 02:15:04 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.90 2007/07/16 17:01:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -770,10 +770,10 @@ extern char *plpgsql_dstring_get(PLpgSQL_dstring *ds);
  */
 extern void plpgsql_ns_init(void);
 extern bool plpgsql_ns_setlocal(bool flag);
-extern void plpgsql_ns_push(char *label);
+extern void plpgsql_ns_push(const char *label);
 extern void plpgsql_ns_pop(void);
 extern void plpgsql_ns_additem(int itemtype, int itemno, const char *name);
-extern PLpgSQL_nsitem *plpgsql_ns_lookup(char *name, char *nsname);
+extern PLpgSQL_nsitem *plpgsql_ns_lookup(const char *name, const char *nsname);
 extern void plpgsql_ns_rename(char *oldname, char *newname);
 
 /* ----------
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index ddb44a72527..6d97b63e5c9 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -3051,3 +3051,31 @@ select * from sc_test();
 (3 rows)
 
 drop function sc_test();
+-- test qualified variable names
+create function pl_qual_names (param1 int) returns void as $$
+<<outerblock>>
+declare
+  param1 int := 1;
+begin
+  <<innerblock>>
+  declare
+    param1 int := 2;
+  begin
+    raise notice 'param1 = %', param1;
+    raise notice 'pl_qual_names.param1 = %', pl_qual_names.param1;
+    raise notice 'outerblock.param1 = %', outerblock.param1;
+    raise notice 'innerblock.param1 = %', innerblock.param1;
+  end;
+end;
+$$ language plpgsql;
+select pl_qual_names(42);
+NOTICE:  param1 = 2
+NOTICE:  pl_qual_names.param1 = 42
+NOTICE:  outerblock.param1 = 1
+NOTICE:  innerblock.param1 = 2
+ pl_qual_names 
+---------------
+ 
+(1 row)
+
+drop function pl_qual_names(int);
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index ee9de0a5838..d1c715e8d4c 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -2535,3 +2535,25 @@ select * from sc_test();
 
 drop function sc_test();
 
+-- test qualified variable names
+
+create function pl_qual_names (param1 int) returns void as $$
+<<outerblock>>
+declare
+  param1 int := 1;
+begin
+  <<innerblock>>
+  declare
+    param1 int := 2;
+  begin
+    raise notice 'param1 = %', param1;
+    raise notice 'pl_qual_names.param1 = %', pl_qual_names.param1;
+    raise notice 'outerblock.param1 = %', outerblock.param1;
+    raise notice 'innerblock.param1 = %', innerblock.param1;
+  end;
+end;
+$$ language plpgsql;
+
+select pl_qual_names(42);
+
+drop function pl_qual_names(int);
-- 
GitLab