On Wed, Sep 14, 2011 at 05:26, Kyotaro HORIGUCHI
<horiguchi.kyot...@oss.ntt.co.jp> wrote:
> This is a review for the patch `Generate column names for
> subquery expressions'
> (https://commitfest.postgresql.org/action/patch_view?id=632)

Thanks for the review. :)

PS: When you send a review, you should add the author's email to the
"To:" line to make sure they see it. I noticed your email only today
because it was in a new thread and not addressed to me directly.

> I think this patch needs no documentation, but it is needed to
> edit the changed behaviors quoted in document. Maybe only one
> change as far as I have seen.
>
> http://www.postgresql.org/docs/9.0/static/sql-expressions.html

>> SELECT ARRAY(SELECT oid FROM pg_proc WHERE proname LIKE 'bytea%');
>>                           ?column?
>> -------------------------------------------------------------
>>  {2011,1954,1948,1952,1951,1244,1950,2005,1949,1953,2006,31}

Good catch, a new patch is attached. Apparently the results of this
query have also changed in recent versions, but I didn't touch that.

Regards,
Marti
From 0defe411697f84d26e6f23970d90c70cd057c298 Mon Sep 17 00:00:00 2001
From: Marti Raudsepp <ma...@juffo.org>
Date: Tue, 6 Sep 2011 21:17:54 +0300
Subject: [PATCH] Generate column names for subquery expressions instead of
 "?column?"

Previously these select expressions didn't have column names:
  select (SELECT colname FROM tab) => colname
  select (SELECT 1 AS foo) => foo
  select exists(SELECT 1) => exists
  select array(SELECT 1) => array

This change makes the subquery column name take precedence over "weak"
column names. Previously these two would return "int4" and "case", now
they return "foo":
  select (select 1 foo)::int
  select case when true then 1 else (select 1 as foo) end
---
 doc/src/sgml/syntax.sgml                 |    2 +-
 src/backend/parser/parse_target.c        |   33 ++++++++++++++++++++++++++++++
 src/test/regress/expected/aggregates.out |    6 ++--
 src/test/regress/expected/subselect.out  |   12 +++++-----
 src/test/regress/expected/with.out       |    4 +-
 5 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/doc/src/sgml/syntax.sgml b/doc/src/sgml/syntax.sgml
index 9119b60..ab81f78 100644
--- a/doc/src/sgml/syntax.sgml
+++ b/doc/src/sgml/syntax.sgml
@@ -2109,7 +2109,7 @@ SELECT ARRAY[]::integer[];
    bracketed) subquery. For example:
 <programlisting>
 SELECT ARRAY(SELECT oid FROM pg_proc WHERE proname LIKE 'bytea%');
-                          ?column?
+                            array
 -------------------------------------------------------------
  {2011,1954,1948,1952,1951,1244,1950,2005,1949,1953,2006,31}
 (1 row)
diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index 9d4e580..685233c 100644
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -1585,6 +1585,39 @@ FigureColnameInternal(Node *node, char **name)
 				return FigureColnameInternal(ind->arg, name);
 			}
 			break;
+		case T_SubLink:
+			switch (((SubLink *) node)->subLinkType)
+			{
+				case EXISTS_SUBLINK:
+					*name = "exists";
+					return 2;
+				case ARRAY_SUBLINK:
+					*name = "array";
+					return 2;
+				case EXPR_SUBLINK:
+					/* Get column name from the subquery's target list */
+					{
+						SubLink	   *sublink = (SubLink *) node;
+						Query	   *query = (Query *) sublink->subselect;
+						/* EXPR_SUBLINK always has a single target */
+						TargetEntry *te = (TargetEntry *) linitial(query->targetList);
+
+						/* Subqueries have already been transformed */
+						if(te->resname)
+						{
+							*name = te->resname;
+							return 2;
+						}
+					}
+					break;
+				/* As with other operator-like nodes, these don't really have names */
+				case ALL_SUBLINK:
+				case ANY_SUBLINK:
+				case ROWCOMPARE_SUBLINK:
+				case CTE_SUBLINK:
+					break;
+			}
+			break;
 		case T_FuncCall:
 			*name = strVal(llast(((FuncCall *) node)->funcname));
 			return 2;
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 4861006..69926f7 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -300,9 +300,9 @@ LINE 4:                where sum(distinct a.four + b.four) = b.four)...
 select
   (select max((select i.unique2 from tenk1 i where i.unique1 = o.unique1)))
 from tenk1 o;
- ?column? 
-----------
-     9999
+ max  
+------
+ 9999
 (1 row)
 
 --
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index e638f0a..4ea8211 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -490,20 +490,20 @@ select view_a from view_a;
 (1 row)
 
 select (select view_a) from view_a;
- ?column? 
-----------
+ view_a 
+--------
  (42)
 (1 row)
 
 select (select (select view_a)) from view_a;
- ?column? 
-----------
+ view_a 
+--------
  (42)
 (1 row)
 
 select (select (a.*)::text) from view_a a;
- ?column? 
-----------
+  a   
+------
  (42)
 (1 row)
 
diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out
index a1b0899..c4b0456 100644
--- a/src/test/regress/expected/with.out
+++ b/src/test/regress/expected/with.out
@@ -1065,7 +1065,7 @@ with cte(foo) as ( select 42 ) select * from ((select foo from cte)) q;
 select ( with cte(foo) as ( values(f1) )
          select (select foo from cte) )
 from int4_tbl;
-  ?column?   
+     foo     
 -------------
            0
       123456
@@ -1077,7 +1077,7 @@ from int4_tbl;
 select ( with cte(foo) as ( values(f1) )
           values((select foo from cte)) )
 from int4_tbl;
-  ?column?   
+   column1   
 -------------
            0
       123456
-- 
1.7.6.1

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to