Change the row constructor syntax (ROW(...)) so that list elements foo.*

will be expanded to a list of their member fields, rather than creating
a nested rowtype field as formerly.  (The old behavior is still available
by omitting '.*'.)  This syntax is not allowed by the SQL spec AFAICS,
so changing its behavior doesn't violate the spec.  The new behavior is
substantially more useful since it allows, for example, triggers to check
for data changes with 'if row(new.*) is distinct from row(old.*)'.  Per
my recent proposal.

5 files changed, 192 insertions, 24 deletions

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 414afe6dfa5..17d221ac19b 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.192 2006/04/22 01:26:00 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.193 2006/06/26 17:24:41 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -29,6 +29,7 @@
 #include "parser/parse_func.h"
 #include "parser/parse_oper.h"
 #include "parser/parse_relation.h"
+#include "parser/parse_target.h"
 #include "parser/parse_type.h"
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
@@ -1289,6 +1290,43 @@ transformRowExpr(ParseState *pstate, RowExpr *r)
 		Node	   *e = (Node *) lfirst(arg);
 		Node	   *newe;
 
+		/*
+		 * Check for "something.*".  Depending on the complexity of the
+		 * "something", the star could appear as the last name in ColumnRef,
+		 * or as the last indirection item in A_Indirection.
+		 */
+		if (IsA(e, ColumnRef))
+		{
+			ColumnRef  *cref = (ColumnRef *) e;
+
+			if (strcmp(strVal(llast(cref->fields)), "*") == 0)
+			{
+				/* It is something.*, expand into multiple items */
+				newargs = list_concat(newargs,
+									  ExpandColumnRefStar(pstate, cref,
+														  false));
+				continue;
+			}
+		}
+		else if (IsA(e, A_Indirection))
+		{
+			A_Indirection *ind = (A_Indirection *) e;
+			Node	   *lastitem = llast(ind->indirection);
+
+			if (IsA(lastitem, String) &&
+				strcmp(strVal(lastitem), "*") == 0)
+			{
+				/* It is something.*, expand into multiple items */
+				newargs = list_concat(newargs,
+									  ExpandIndirectionStar(pstate, ind,
+															false));
+				continue;
+			}
+		}
+
+		/*
+		 * Not "something.*", so transform as a single expression
+		 */
 		newe = transformExpr(pstate, e);
 		newargs = lappend(newargs, newe);
 	}
diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index 86670d26794..859de3b10c6 100644
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_target.c,v 1.143 2006/06/16 18:42:22 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_target.c,v 1.144 2006/06/26 17:24:41 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -42,9 +42,7 @@ static Node *transformAssignmentIndirection(ParseState *pstate,
 							   ListCell *indirection,
 							   Node *rhs,
 							   int location);
-static List *ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref);
 static List *ExpandAllTables(ParseState *pstate);
-static List *ExpandIndirectionStar(ParseState *pstate, A_Indirection *ind);
 static int	FigureColnameInternal(Node *node, char **name);
 
 
@@ -117,7 +115,8 @@ transformTargetList(ParseState *pstate, List *targetlist)
 			{
 				/* It is something.*, expand into multiple items */
 				p_target = list_concat(p_target,
-									   ExpandColumnRefStar(pstate, cref));
+									   ExpandColumnRefStar(pstate, cref,
+														   true));
 				continue;
 			}
 		}
@@ -131,7 +130,8 @@ transformTargetList(ParseState *pstate, List *targetlist)
 			{
 				/* It is something.*, expand into multiple items */
 				p_target = list_concat(p_target,
-									   ExpandIndirectionStar(pstate, ind));
+									   ExpandIndirectionStar(pstate, ind,
+															 true));
 				continue;
 			}
 		}
@@ -696,13 +696,16 @@ checkInsertTargets(ParseState *pstate, List *cols, List **attrnos)
 
 /*
  * ExpandColumnRefStar()
- *		Turns foo.* (in the target list) into a list of targetlist entries.
+ *		Transforms foo.* into a list of expressions or targetlist entries.
  *
  * This handles the case where '*' appears as the last or only name in a
- * ColumnRef.
+ * ColumnRef.  The code is shared between the case of foo.* at the top level
+ * in a SELECT target list (where we want TargetEntry nodes in the result)
+ * and foo.* in a ROW() construct (where we want just bare expressions).
  */
-static List *
-ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref)
+List *
+ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref,
+					bool targetlist)
 {
 	List	   *fields = cref->fields;
 	int			numnames = list_length(fields);
@@ -713,7 +716,12 @@ ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref)
 		 * Target item is a bare '*', expand all tables
 		 *
 		 * (e.g., SELECT * FROM emp, dept)
+		 *
+		 * Since the grammar only accepts bare '*' at top level of SELECT,
+		 * we need not handle the targetlist==false case here.
 		 */
+		Assert(targetlist);
+
 		return ExpandAllTables(pstate);
 	}
 	else
@@ -775,13 +783,22 @@ ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref)
 
 		rtindex = RTERangeTablePosn(pstate, rte, &sublevels_up);
 
-		return expandRelAttrs(pstate, rte, rtindex, sublevels_up);
+		if (targetlist)
+			return expandRelAttrs(pstate, rte, rtindex, sublevels_up);
+		else
+		{
+			List	*vars;
+
+			expandRTE(rte, rtindex, sublevels_up, false,
+					  NULL, &vars);
+			return vars;
+		}
 	}
 }
 
 /*
  * ExpandAllTables()
- *		Turns '*' (in the target list) into a list of targetlist entries.
+ *		Transforms '*' (in the target list) into a list of targetlist entries.
  *
  * tlist entries are generated for each relation appearing in the query's
  * varnamespace.  We do not consider relnamespace because that would include
@@ -814,18 +831,22 @@ ExpandAllTables(ParseState *pstate)
 
 /*
  * ExpandIndirectionStar()
- *		Turns foo.* (in the target list) into a list of targetlist entries.
+ *		Transforms foo.* into a list of expressions or targetlist entries.
  *
  * This handles the case where '*' appears as the last item in A_Indirection.
+ * The code is shared between the case of foo.* at the top level in a SELECT
+ * target list (where we want TargetEntry nodes in the result) and foo.* in
+ * a ROW() construct (where we want just bare expressions).
  */
-static List *
-ExpandIndirectionStar(ParseState *pstate, A_Indirection *ind)
+List *
+ExpandIndirectionStar(ParseState *pstate, A_Indirection *ind,
+					  bool targetlist)
 {
+	List	   *result = NIL;
 	Node	   *expr;
 	TupleDesc	tupleDesc;
 	int			numAttrs;
 	int			i;
-	List	   *te_list = NIL;
 
 	/* Strip off the '*' to create a reference to the rowtype object */
 	ind = copyObject(ind);
@@ -860,7 +881,6 @@ ExpandIndirectionStar(ParseState *pstate, A_Indirection *ind)
 	{
 		Form_pg_attribute att = tupleDesc->attrs[i];
 		Node	   *fieldnode;
-		TargetEntry *te;
 
 		if (att->attisdropped)
 			continue;
@@ -893,14 +913,22 @@ ExpandIndirectionStar(ParseState *pstate, A_Indirection *ind)
 			fieldnode = (Node *) fselect;
 		}
 
-		te = makeTargetEntry((Expr *) fieldnode,
-							 (AttrNumber) pstate->p_next_resno++,
-							 pstrdup(NameStr(att->attname)),
-							 false);
-		te_list = lappend(te_list, te);
+		if (targetlist)
+		{
+			/* add TargetEntry decoration */
+			TargetEntry *te;
+
+			te = makeTargetEntry((Expr *) fieldnode,
+								 (AttrNumber) pstate->p_next_resno++,
+								 pstrdup(NameStr(att->attname)),
+								 false);
+			result = lappend(result, te);
+		}
+		else
+			result = lappend(result, fieldnode);
 	}
 
-	return te_list;
+	return result;
 }
 
 /*
diff --git a/src/include/parser/parse_target.h b/src/include/parser/parse_target.h
index f1cb2bd8a1c..6080696baa0 100644
--- a/src/include/parser/parse_target.h
+++ b/src/include/parser/parse_target.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/parser/parse_target.h,v 1.39 2006/03/23 00:19:30 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/parser/parse_target.h,v 1.40 2006/06/26 17:24:41 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -22,6 +22,10 @@ extern void markTargetListOrigins(ParseState *pstate, List *targetlist);
 extern TargetEntry *transformTargetEntry(ParseState *pstate,
 					 Node *node, Node *expr,
 					 char *colname, bool resjunk);
+extern List *ExpandColumnRefStar(ParseState *pstate, ColumnRef *cref,
+								 bool targetlist);
+extern List *ExpandIndirectionStar(ParseState *pstate, A_Indirection *ind,
+								   bool targetlist);
 extern void updateTargetListEntry(ParseState *pstate, TargetEntry *tle,
 					  char *colname, int attrno,
 					  List *indirection,
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index d8b0594c2bd..1af4d9e2890 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -487,3 +487,53 @@ DROP TRIGGER show_trigger_data_trig on trigger_test;
       
 DROP FUNCTION trigger_data();
 DROP TABLE trigger_test;
+--
+-- Test use of row comparisons on OLD/NEW
+--
+CREATE TABLE trigger_test (f1 int, f2 text, f3 text);
+-- this is the obvious (and wrong...) way to compare rows
+CREATE FUNCTION mytrigger() RETURNS trigger LANGUAGE plpgsql as $$
+begin
+	if row(old.*) = row(new.*) then
+		raise notice 'row % not changed', new.f1;
+	else
+		raise notice 'row % changed', new.f1;
+	end if;
+	return new;
+end$$;
+CREATE TRIGGER t
+BEFORE UPDATE ON trigger_test
+FOR EACH ROW EXECUTE PROCEDURE mytrigger();
+INSERT INTO trigger_test VALUES(1, 'foo', 'bar');
+INSERT INTO trigger_test VALUES(2, 'baz', 'quux');
+UPDATE trigger_test SET f3 = 'bar';
+NOTICE:  row 1 not changed
+NOTICE:  row 2 changed
+UPDATE trigger_test SET f3 = NULL;
+NOTICE:  row 1 changed
+NOTICE:  row 2 changed
+-- this demonstrates that the above isn't really working as desired:
+UPDATE trigger_test SET f3 = NULL;
+NOTICE:  row 1 changed
+NOTICE:  row 2 changed
+-- the right way when considering nulls is
+CREATE OR REPLACE FUNCTION mytrigger() RETURNS trigger LANGUAGE plpgsql as $$
+begin
+	if row(old.*) is distinct from row(new.*) then
+		raise notice 'row % changed', new.f1;
+	else
+		raise notice 'row % not changed', new.f1;
+	end if;
+	return new;
+end$$;
+UPDATE trigger_test SET f3 = 'bar';
+NOTICE:  row 1 changed
+NOTICE:  row 2 changed
+UPDATE trigger_test SET f3 = NULL;
+NOTICE:  row 1 changed
+NOTICE:  row 2 changed
+UPDATE trigger_test SET f3 = NULL;
+NOTICE:  row 1 not changed
+NOTICE:  row 2 not changed
+DROP TABLE trigger_test;
+DROP FUNCTION mytrigger();
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 909a7d68d9b..647c4c448ea 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -367,3 +367,51 @@ DROP TRIGGER show_trigger_data_trig on trigger_test;
 DROP FUNCTION trigger_data();
 
 DROP TABLE trigger_test;
+
+--
+-- Test use of row comparisons on OLD/NEW
+--
+
+CREATE TABLE trigger_test (f1 int, f2 text, f3 text);
+
+-- this is the obvious (and wrong...) way to compare rows
+CREATE FUNCTION mytrigger() RETURNS trigger LANGUAGE plpgsql as $$
+begin
+	if row(old.*) = row(new.*) then
+		raise notice 'row % not changed', new.f1;
+	else
+		raise notice 'row % changed', new.f1;
+	end if;
+	return new;
+end$$;
+
+CREATE TRIGGER t
+BEFORE UPDATE ON trigger_test
+FOR EACH ROW EXECUTE PROCEDURE mytrigger();
+
+INSERT INTO trigger_test VALUES(1, 'foo', 'bar');
+INSERT INTO trigger_test VALUES(2, 'baz', 'quux');
+
+UPDATE trigger_test SET f3 = 'bar';
+UPDATE trigger_test SET f3 = NULL;
+-- this demonstrates that the above isn't really working as desired:
+UPDATE trigger_test SET f3 = NULL;
+
+-- the right way when considering nulls is
+CREATE OR REPLACE FUNCTION mytrigger() RETURNS trigger LANGUAGE plpgsql as $$
+begin
+	if row(old.*) is distinct from row(new.*) then
+		raise notice 'row % changed', new.f1;
+	else
+		raise notice 'row % not changed', new.f1;
+	end if;
+	return new;
+end$$;
+
+UPDATE trigger_test SET f3 = 'bar';
+UPDATE trigger_test SET f3 = NULL;
+UPDATE trigger_test SET f3 = NULL;
+
+DROP TABLE trigger_test;
+
+DROP FUNCTION mytrigger();