pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedSelecting fields from a RowExpr
2+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2019-10-28 8b7a0f1d11 2f02d4a2b9 ece1154f4c Allow extracting fields from a ROW() expression in more cases.
* Selecting fields from a RowExpr
@ 2019-10-27 18:46 Tom Lane <tgl@sss.pgh.pa.us>
2019-10-27 18:59 ` Re: Selecting fields from a RowExpr Pavel Stehule <pavel.stehule@gmail.com>
0 siblings, 1 reply; 2+ messages in thread
From: Tom Lane @ 2019-10-27 18:46 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
At pgconf.eu, someone whose name I've forgotten pointed out to me
that this doesn't work:
regression=# select (row(1, 2.0)).f1;
ERROR: could not identify column "f1" in record data type
LINE 1: select (row(1, 2.0)).f1;
^
The fields of an anonymous rowtype are certainly named f1, f2, etc,
so it seems like this *should* work. A related case is
regression=# select (row(1, 2.0)).*;
ERROR: record type has not been registered
Admittedly, these probably aren't terribly useful cases in practice,
but it's unfortunate that they don't work as one would expect.
So I propose the attached patch to make them work.
The underlying reason for both of these failures is that RowExpr
doesn't carry a typmod, so if it's of type RECORD then
get_expr_result_type doesn't know how to find a tupdesc for it.
The minimum-code solution is to teach get_expr_result_type to build
a tupdesc directly from the RowExpr, and that seems to be necessary
for complicated cases like
select (r).f1 from (select row(1, 2.0) as r) ss;
In an earlier version of the patch I chose to add in some fast-path
logic in ParseComplexProjection and ExpandRowReference, so as to
make the really simple cases shown above a bit less inefficient.
But on second thought, these are such corner cases that it doesn't
seem worth carrying extra code for them. The cases that are more
likely to arise in practice are like that last example, and we
can't optimize that in the parser. (The planner will optimize
FieldSelect-from-RowExpr after flattening subqueries, which is
probably as much as we really need to do here.)
I don't feel a need to back-patch this, but I would like to push
it into HEAD.
Thoughts?
regards, tom lane
Attachments:
allow-selecting-from-RowExprs-in-more-cases.patch (text/x-diff, 3.3K) download
diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c
index b7fac5d..4688fbc 100644
--- a/src/backend/utils/fmgr/funcapi.c
+++ b/src/backend/utils/fmgr/funcapi.c
@@ -227,6 +227,38 @@ get_expr_result_type(Node *expr,
NULL,
resultTypeId,
resultTupleDesc);
+ else if (expr && IsA(expr, RowExpr) &&
+ ((RowExpr *) expr)->row_typeid == RECORDOID)
+ {
+ /* We can resolve the record type by generating the tupdesc directly */
+ RowExpr *rexpr = (RowExpr *) expr;
+ TupleDesc tupdesc;
+ AttrNumber i = 1;
+ ListCell *lcc,
+ *lcn;
+
+ tupdesc = CreateTemplateTupleDesc(list_length(rexpr->args));
+ Assert(list_length(rexpr->args) == list_length(rexpr->colnames));
+ forboth(lcc, rexpr->args, lcn, rexpr->colnames)
+ {
+ Node *col = (Node *) lfirst(lcc);
+ char *colname = strVal(lfirst(lcn));
+
+ TupleDescInitEntry(tupdesc, i,
+ colname,
+ exprType(col),
+ exprTypmod(col),
+ 0);
+ TupleDescInitEntryCollation(tupdesc, i,
+ exprCollation(col));
+ i++;
+ }
+ if (resultTypeId)
+ *resultTypeId = rexpr->row_typeid;
+ if (resultTupleDesc)
+ *resultTupleDesc = BlessTupleDesc(tupdesc);
+ return TYPEFUNC_COMPOSITE;
+ }
else
{
/* handle as a generic expression; no chance to resolve RECORD */
diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out
index a272305..2a273f8 100644
--- a/src/test/regress/expected/rowtypes.out
+++ b/src/test/regress/expected/rowtypes.out
@@ -436,6 +436,45 @@ where i8 in (row(123,456)::int8_tbl, '(4567890123456789,123)');
4567890123456789 | 123
(2 rows)
+-- Check ability to select columns from an anonymous rowtype
+select (row(1, 2.0)).f1;
+ f1
+----
+ 1
+(1 row)
+
+select (row(1, 2.0)).f2;
+ f2
+-----
+ 2.0
+(1 row)
+
+select (row(1, 2.0)).nosuch; -- fail
+ERROR: could not identify column "nosuch" in record data type
+LINE 1: select (row(1, 2.0)).nosuch;
+ ^
+select (row(1, 2.0)).*;
+ f1 | f2
+----+-----
+ 1 | 2.0
+(1 row)
+
+select (r).f1 from (select row(1, 2.0) as r) ss;
+ f1
+----
+ 1
+(1 row)
+
+select (r).f3 from (select row(1, 2.0) as r) ss; -- fail
+ERROR: could not identify column "f3" in record data type
+LINE 1: select (r).f3 from (select row(1, 2.0) as r) ss;
+ ^
+select (r).* from (select row(1, 2.0) as r) ss;
+ f1 | f2
+----+-----
+ 1 | 2.0
+(1 row)
+
-- Check some corner cases involving empty rowtypes
select ROW();
row
diff --git a/src/test/regress/sql/rowtypes.sql b/src/test/regress/sql/rowtypes.sql
index 7e080c0..83cf4a1 100644
--- a/src/test/regress/sql/rowtypes.sql
+++ b/src/test/regress/sql/rowtypes.sql
@@ -171,6 +171,15 @@ where i8 in (row(123,456)::int8_tbl, '(4567890123456789,123)');
select * from int8_tbl i8
where i8 in (row(123,456)::int8_tbl, '(4567890123456789,123)');
+-- Check ability to select columns from an anonymous rowtype
+select (row(1, 2.0)).f1;
+select (row(1, 2.0)).f2;
+select (row(1, 2.0)).nosuch; -- fail
+select (row(1, 2.0)).*;
+select (r).f1 from (select row(1, 2.0) as r) ss;
+select (r).f3 from (select row(1, 2.0) as r) ss; -- fail
+select (r).* from (select row(1, 2.0) as r) ss;
+
-- Check some corner cases involving empty rowtypes
select ROW();
select ROW() IS NULL;
* Re: Selecting fields from a RowExpr
2019-10-27 18:46 Selecting fields from a RowExpr Tom Lane <tgl@sss.pgh.pa.us>
@ 2019-10-27 18:59 ` Pavel Stehule <pavel.stehule@gmail.com>
0 siblings, 0 replies; 2+ messages in thread
From: Pavel Stehule @ 2019-10-27 18:59 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Hi
ne 27. 10. 2019 v 19:47 odesÃlatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
> At pgconf.eu, someone whose name I've forgotten pointed out to me
> that this doesn't work:
>
> regression=# select (row(1, 2.0)).f1;
> ERROR: could not identify column "f1" in record data type
> LINE 1: select (row(1, 2.0)).f1;
> ^
>
> The fields of an anonymous rowtype are certainly named f1, f2, etc,
> so it seems like this *should* work. A related case is
>
> regression=# select (row(1, 2.0)).*;
> ERROR: record type has not been registered
>
> Admittedly, these probably aren't terribly useful cases in practice,
> but it's unfortunate that they don't work as one would expect.
> So I propose the attached patch to make them work.
>
> The underlying reason for both of these failures is that RowExpr
> doesn't carry a typmod, so if it's of type RECORD then
> get_expr_result_type doesn't know how to find a tupdesc for it.
> The minimum-code solution is to teach get_expr_result_type to build
> a tupdesc directly from the RowExpr, and that seems to be necessary
> for complicated cases like
>
> select (r).f1 from (select row(1, 2.0) as r) ss;
>
> In an earlier version of the patch I chose to add in some fast-path
> logic in ParseComplexProjection and ExpandRowReference, so as to
> make the really simple cases shown above a bit less inefficient.
> But on second thought, these are such corner cases that it doesn't
> seem worth carrying extra code for them. The cases that are more
> likely to arise in practice are like that last example, and we
> can't optimize that in the parser. (The planner will optimize
> FieldSelect-from-RowExpr after flattening subqueries, which is
> probably as much as we really need to do here.)
>
> I don't feel a need to back-patch this, but I would like to push
> it into HEAD.
>
some times I hit this limit, an can be nice more consistent behave of
composite types.
It's new feature - and there is not a reason for back-patching
Regards
Pavel
>
> Thoughts?
>
> regards, tom lane
>
>
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2019-10-27 18:59 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-10-27 18:46 Selecting fields from a RowExpr Tom Lane <tgl@sss.pgh.pa.us>
2019-10-27 18:59 ` Pavel Stehule <pavel.stehule@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox