From ca0d2197ca1ed1e72243c5a3466d93e4954d30b0 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 26 Jun 2006 17:24:41 +0000
Subject: [PATCH] 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.
---
doc/src/sgml/syntax.sgml | 29 ++++++++++-
src/backend/parser/parse_expr.c | 40 +++++++++++++-
src/backend/parser/parse_target.c | 72 ++++++++++++++++++--------
src/include/parser/parse_target.h | 6 ++-
src/test/regress/expected/triggers.out | 50 ++++++++++++++++++
src/test/regress/sql/triggers.sql | 48 +++++++++++++++++
6 files changed, 219 insertions(+), 26 deletions(-)
diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml
index 97020845b50..74cc813a681 100644
--- a/doc/src/sgml/syntax.sgml
+++ b/doc/src/sgml/syntax.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/syntax.sgml,v 1.106 2006/03/10 19:10:49 momjian Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/syntax.sgml,v 1.107 2006/06/26 17:24:40 tgl Exp $ -->
<chapter id="sql-syntax">
<title>SQL Syntax</title>
@@ -1570,6 +1570,31 @@ SELECT ROW(1,2.5,'this is a test');
expression in the list.
</para>
+ <para>
+ A row constructor can include the syntax
+ <replaceable>rowvalue</replaceable><literal>.*</literal>,
+ which will be expanded to a list of the elements of the row value,
+ just as occurs when the <literal>.*</> syntax is used at the top level
+ of a <command>SELECT</> list. For example, if table <literal>t</> has
+ columns <literal>f1</> and <literal>f2</>, these are the same:
+<programlisting>
+SELECT ROW(t.*, 42) FROM t;
+SELECT ROW(t.f1, t.f2, 42) FROM t;
+</programlisting>
+ </para>
+
+ <note>
+ <para>
+ Before <productname>PostgreSQL</productname> 8.2, the
+ <literal>.*</literal> syntax was not expanded, so that writing
+ <literal>ROW(t.*, 42)</> created a two-field row whose first field
+ was another row value. The new behavior is usually more useful.
+ If you need the old behavior of nested row values, write the inner
+ row value without <literal>.*</literal>, for instance
+ <literal>ROW(t, 42)</>.
+ </para>
+ </note>
+
<para>
By default, the value created by a <literal>ROW</> expression is of
an anonymous record type. If necessary, it can be cast to a named
@@ -1619,7 +1644,7 @@ SELECT getf1(CAST(ROW(11,'this is a test',2.5) AS myrowtype));
<programlisting>
SELECT ROW(1,2.5,'this is a test') = ROW(1, 3, 'not the same');
-SELECT ROW(a, b, c) IS NOT NULL FROM table;
+SELECT ROW(table.*) IS NULL FROM table; -- detect all-null rows
</programlisting>
For more detail see <xref linkend="functions-comparisons">.
Row constructors can also be used in connection with subqueries,
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();
--
GitLab