Обсуждение: Please provide examples of rows from
The following documentation comment has been logged on the website: Page: https://www.postgresql.org/docs/12/queries-table-expressions.html Description: The explanation of ROWS FROM is fairly terse and no examples are given. As it is essentially impossible to usefully search for this phrase, it would be helpful if at least a few examples were given.
On Wed, Apr 8, 2020 at 09:50:44PM +0000, PG Doc comments form wrote: > The following documentation comment has been logged on the website: > > Page: https://www.postgresql.org/docs/12/queries-table-expressions.html > Description: > > The explanation of ROWS FROM is fairly terse and no examples are given. > > As it is essentially impossible to usefully search for this phrase, it would > be helpful if at least a few examples were given. Looking at this suggestion, this was all I could think of: SELECT * FROM ROWS FROM (GENERATE_SERIES(1,10), GENERATE_SERIES(1, 15)); generate_series | generate_series -----------------+----------------- 1 | 1 2 | 2 3 | 3 4 | 4 5 | 5 6 | 6 7 | 7 8 | 8 9 | 9 10 | 10 (null) | 11 (null) | 12 (null) | 13 (null) | 14 (null) | 15 The issue with adding an example is that it is hard to do something simple and have it illustrate anything. Does this help? -- Bruce Momjian <bruce@momjian.us> https://momjian.us EnterpriseDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee
On Thu, Sep 3, 2020 at 6:46 PM Bruce Momjian <bruce@momjian.us> wrote:
On Wed, Apr 8, 2020 at 09:50:44PM +0000, PG Doc comments form wrote:
> The following documentation comment has been logged on the website:
>
> Page: https://www.postgresql.org/docs/12/queries-table-expressions.html
> Description:
>
> The explanation of ROWS FROM is fairly terse and no examples are given.
>
> As it is essentially impossible to usefully search for this phrase, it would
> be helpful if at least a few examples were given.
Looking at this suggestion, this was all I could think of:
SELECT * FROM ROWS FROM (GENERATE_SERIES(1,10), GENERATE_SERIES(1, 15));
generate_series | generate_series
-----------------+-----------------
1 | 1
2 | 2
3 | 3
4 | 4
5 | 5
6 | 6
7 | 7
8 | 8
9 | 9
10 | 10
(null) | 11
(null) | 12
(null) | 13
(null) | 14
(null) | 15
The issue with adding an example is that it is hard to do something
simple and have it illustrate anything. Does this help?
That documents one of the two variants - and incorporating the column alias feature seems worthwhile for the chosen example. I do think this is worth adding.
The more complicated one is the second:
ROWS FROM( ... function_call AS (column_definition [, ... ]) [, ... ] )
First, what's with the first set of "..."? It doesn't appear in the reference documentation.
I was looking at the "Queries" doc comment a little bit ago and am wondering if there is some principle by which we would decide to place any new examples in this section versus the examples part of the SELECT reference section?
I would note that part of the confusion probably stems from not defining "column definition" in this chapter. It probably could be defined more prominently in the SELECT reference as well.
Basically, aliases outside the ROWS FROM, column definitions within, unless there is only a single "record" returning function involved (and without ORDINALITY) in which case the external aliases can be instead a complete column definition.
For the simple solution to the complaint I would thus suggest three examples, but added to the SELECT reference, covering those three situations (mutli-typed-aliased, multi-record, single-rows-from-record-using-outside-columndef), and pointing the user to the SELECT reference for said examples. That would be in addition to the one example (another multi-typed-aliased) above being added to the queries section.
A more involved patch would need, IMO, some structure to make the queries section sufficient but less complex while relegating much of the complexity to the reference section. That seems to be doing a better job describing this reality presently anyway.
David J.
On Mon, Sep 14, 2020 at 06:49:22PM -0700, David G. Johnston wrote:
> That documents one of the two variants - and incorporating the column alias
> feature seems worthwhile for the chosen example. I do think this is worth
> adding.
>
> The more complicated one is the second:
>
> ROWS FROM( ... function_call AS (column_definition [, ... ]) [, ... ] )
>
> First, what's with the first set of "..."? It doesn't appear in the reference
> documentation.
>
> I was looking at the "Queries" doc comment a little bit ago and am wondering if
> there is some principle by which we would decide to place any new examples in
> this section versus the examples part of the SELECT reference section?
>
> I would note that part of the confusion probably stems from not defining
> "column definition" in this chapter. It probably could be defined more
> prominently in the SELECT reference as well.
>
> Basically, aliases outside the ROWS FROM, column definitions within, unless
> there is only a single "record" returning function involved (and without
> ORDINALITY) in which case the external aliases can be instead a complete column
> definition.
>
> For the simple solution to the complaint I would thus suggest three examples,
> but added to the SELECT reference, covering those three situations
> (mutli-typed-aliased, multi-record,
> single-rows-from-record-using-outside-columndef), and pointing the user to the
> SELECT reference for said examples. That would be in addition to the one
> example (another multi-typed-aliased) above being added to the queries section.
>
> A more involved patch would need, IMO, some structure to make the queries
> section sufficient but less complex while relegating much of the complexity to
> the reference section. That seems to be doing a better job describing this
> reality presently anyway.
I spent some time on this. First, since ROWS FROM is a Postgres
extension, it is certainly our job to document it clearly. I started
looking at using system tables that return RECORD for the examples, but
found that this did not work, even without ROWS FROM:
test=> \df pg_get_keywords
List of functions
Schema | Name | Result data type | Argument data types
| Type
------------+-----------------+------------------+-----------------------------------------------------------------------------------------------+------
pg_catalog | pg_get_keywords | SETOF record | OUT word text, OUT catcode "char", OUT barelabel boolean, OUT
catdesctext, OUT baredesc text | func
(1 row)
test=> select * from pg_get_keywords() AS f(word text);
--> ERROR: a column definition list is only allowed for functions returning "record"
LINE 1: select * from pg_get_keywords() AS f(word text);
Oddly, dblink did work:
test=> \df dblink
List of functions
Schema | Name | Result data type | Argument data types | Type
--------+--------+------------------+---------------------+------
public | dblink | SETOF record | text | func
public | dblink | SETOF record | text, boolean | func
public | dblink | SETOF record | text, text | func
public | dblink | SETOF record | text, text, boolean | func
(4 rows)
test=> SELECT *
FROM dblink('dbname=mydb', 'SELECT proname, prosrc FROM pg_proc')
AS t1(proname name, prosrc text);
ERROR: could not establish connection
DETAIL: FATAL: database "mydb" does not exist
Is it because dblink() does not use OUT parameters?
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
Bruce Momjian <bruce@momjian.us> writes:
> test=> \df pg_get_keywords
> List of functions
> Schema | Name | Result data type | Argument data types
| Type
>
------------+-----------------+------------------+-----------------------------------------------------------------------------------------------+------
> pg_catalog | pg_get_keywords | SETOF record | OUT word text, OUT catcode "char", OUT barelabel boolean, OUT
catdesctext, OUT baredesc text | func
> (1 row)
> test=> select * from pg_get_keywords() AS f(word text);
> --> ERROR: a column definition list is only allowed for functions returning "record"
> LINE 1: select * from pg_get_keywords() AS f(word text);
Yeah, this error message needs some help. With a function having
multiple OUT parameters, the prorettype is indeed "record", but
the specific record type is implied by the OUT parameters so you
do not need to (and can't) specify it in the query.
The point of the AS feature is to allow specifying the concrete
record type for record-returning functions that don't have a
predefined result record type, like dblink().
I think this error text was written before we had multiple OUT
parameters, so it was okay at the time; but now it needs to be
more precise.
regards, tom lane
On Sat, Sep 19, 2020 at 08:49:53PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > test=> \df pg_get_keywords
> > List of functions
> > Schema | Name | Result data type | Argument data types
| Type
> >
------------+-----------------+------------------+-----------------------------------------------------------------------------------------------+------
> > pg_catalog | pg_get_keywords | SETOF record | OUT word text, OUT catcode "char", OUT barelabel boolean,
OUTcatdesc text, OUT baredesc text | func
> > (1 row)
>
> > test=> select * from pg_get_keywords() AS f(word text);
> > --> ERROR: a column definition list is only allowed for functions returning "record"
> > LINE 1: select * from pg_get_keywords() AS f(word text);
>
> Yeah, this error message needs some help. With a function having
> multiple OUT parameters, the prorettype is indeed "record", but
> the specific record type is implied by the OUT parameters so you
> do not need to (and can't) specify it in the query.
>
> The point of the AS feature is to allow specifying the concrete
> record type for record-returning functions that don't have a
> predefined result record type, like dblink().
>
> I think this error text was written before we had multiple OUT
> parameters, so it was okay at the time; but now it needs to be
> more precise.
OK, thanks. It seems this area needs some work, in general.
Unfortunately I don't see any system functions that return RECORD and
don't use OUT parameters, except dblink(), json(b)_to_record(),
json(b)_to_recordset(), and record_*. This is going to be hard to
illustrate. :-( I did get this working:
test=> select * FROM json_to_record('{"a": 1, "b": 2}'::json) as (b
text);
b
---
2
but doing this to illustrate ROWS FROM is going to be complex.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
Bruce Momjian <bruce@momjian.us> writes:
> but doing this to illustrate ROWS FROM is going to be complex.
You're right that the only suitable core function is going to be
json[b]_to_recordset, but I don't see why you can't extend the
existing example for that. Something like
# select * from rows from (json_to_recordset('[{"a":1,"b":"foo"},{"a":"2","c":"bar"}]') as (x int, y text),
generate_series(1,3))as x(a,b,s) ;
a | b | s
---+-----+---
1 | foo | 1
2 | | 2
| | 3
(3 rows)
would illustrate all the principles.
regards, tom lane
I wrote:
> Yeah, this error message needs some help. With a function having
> multiple OUT parameters, the prorettype is indeed "record", but
> the specific record type is implied by the OUT parameters so you
> do not need to (and can't) specify it in the query.
> The point of the AS feature is to allow specifying the concrete
> record type for record-returning functions that don't have a
> predefined result record type, like dblink().
> I think this error text was written before we had multiple OUT
> parameters, so it was okay at the time; but now it needs to be
> more precise.
Concretely, perhaps like the attached. I was unsurprised to find
that this error condition isn't exercised in our regression tests.
I added some coverage, but maybe that's overkill?
regards, tom lane
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index b875a50646..a56bd86181 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -1737,16 +1737,46 @@ addRangeTableEntryForFunction(ParseState *pstate,
/*
* A coldeflist is required if the function returns RECORD and hasn't
- * got a predetermined record type, and is prohibited otherwise.
+ * got a predetermined record type, and is prohibited otherwise. This
+ * can be a bit confusing, so we expend some effort on delivering a
+ * relevant error message.
*/
if (coldeflist != NIL)
{
- if (functypclass != TYPEFUNC_RECORD)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("a column definition list is only allowed for functions returning \"record\""),
- parser_errposition(pstate,
- exprLocation((Node *) coldeflist))));
+ switch (functypclass)
+ {
+ case TYPEFUNC_RECORD:
+ /* ok */
+ break;
+ case TYPEFUNC_COMPOSITE:
+ case TYPEFUNC_COMPOSITE_DOMAIN:
+
+ /*
+ * If the function's raw result type is RECORD, we must
+ * have resolved it using its OUT parameters. Otherwise,
+ * it must have a named composite type.
+ */
+ if (exprType(funcexpr) == RECORDOID)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("a column definition list is redundant for a function with OUT parameters"),
+ parser_errposition(pstate,
+ exprLocation((Node *) coldeflist))));
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("a column definition list is redundant for a function returning a named
compositetype"),
+ parser_errposition(pstate,
+ exprLocation((Node *) coldeflist))));
+ break;
+ default:
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("a column definition list is only allowed for functions returning \"record\""),
+ parser_errposition(pstate,
+ exprLocation((Node *) coldeflist))));
+ break;
+ }
}
else
{
diff --git a/src/test/regress/expected/rangefuncs.out b/src/test/regress/expected/rangefuncs.out
index 7eced28452..e618aec2eb 100644
--- a/src/test/regress/expected/rangefuncs.out
+++ b/src/test/regress/expected/rangefuncs.out
@@ -2109,6 +2109,19 @@ select * from testrngfunc();
7.136178 | 7.14
(1 row)
+-- Check a couple of error cases while we're here
+select * from testrngfunc() as t(f1 int8,f2 int8); -- fail, composite result
+ERROR: a column definition list is redundant for a function returning a named composite type
+LINE 1: select * from testrngfunc() as t(f1 int8,f2 int8);
+ ^
+select * from pg_get_keywords() as t(f1 int8,f2 int8); -- fail, OUT params
+ERROR: a column definition list is redundant for a function with OUT parameters
+LINE 1: select * from pg_get_keywords() as t(f1 int8,f2 int8);
+ ^
+select * from sin(3) as t(f1 int8,f2 int8); -- fail, scalar result type
+ERROR: a column definition list is only allowed for functions returning "record"
+LINE 1: select * from sin(3) as t(f1 int8,f2 int8);
+ ^
drop type rngfunc_type cascade;
NOTICE: drop cascades to function testrngfunc()
--
diff --git a/src/test/regress/sql/rangefuncs.sql b/src/test/regress/sql/rangefuncs.sql
index ae3119a959..5f41cb2d8d 100644
--- a/src/test/regress/sql/rangefuncs.sql
+++ b/src/test/regress/sql/rangefuncs.sql
@@ -629,6 +629,11 @@ explain (verbose, costs off)
select * from testrngfunc();
select * from testrngfunc();
+-- Check a couple of error cases while we're here
+select * from testrngfunc() as t(f1 int8,f2 int8); -- fail, composite result
+select * from pg_get_keywords() as t(f1 int8,f2 int8); -- fail, OUT params
+select * from sin(3) as t(f1 int8,f2 int8); -- fail, scalar result type
+
drop type rngfunc_type cascade;
--
I wrote:
> You're right that the only suitable core function is going to be
> json[b]_to_recordset, but I don't see why you can't extend the
> existing example for that. Something like
Meh, I was too hasty and pushed "send" with a broken example. Better
=# select * from rows from (json_to_recordset('[{"a":1,"b":"foo"},{"a":"2","b":"bar"}]') as (a int, b text),
generate_series(1,3))as x(p,q,s);
p | q | s
---+-----+---
1 | foo | 1
2 | bar | 2
| | 3
(3 rows)
regards, tom lane
On Mon, Sep 14, 2020 at 06:49:22PM -0700, David G. Johnston wrote: > On Thu, Sep 3, 2020 at 6:46 PM Bruce Momjian <bruce@momjian.us> wrote: > The issue with adding an example is that it is hard to do something > simple and have it illustrate anything. Does this help? > > That documents one of the two variants - and incorporating the column alias > feature seems worthwhile for the chosen example. I do think this is worth > adding. > > The more complicated one is the second: > > ROWS FROM( ... function_call AS (column_definition [, ... ]) [, ... ] ) > > First, what's with the first set of "..."? It doesn't appear in the reference > documentation. I have removed these dots in a patch I just posted to this thread. > I was looking at the "Queries" doc comment a little bit ago and am wondering if > there is some principle by which we would decide to place any new examples in > this section versus the examples part of the SELECT reference section? Agreed. Let me know if my patch needs more information. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EnterpriseDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee
On Sun, Sep 20, 2020 at 02:14:00PM -0400, Tom Lane wrote:
> I wrote:
> > You're right that the only suitable core function is going to be
> > json[b]_to_recordset, but I don't see why you can't extend the
> > existing example for that. Something like
>
> Meh, I was too hasty and pushed "send" with a broken example. Better
>
> =# select * from rows from (json_to_recordset('[{"a":1,"b":"foo"},{"a":"2","b":"bar"}]') as (a int, b text),
generate_series(1,3))as x(p,q,s);
> p | q | s
> ---+-----+---
> 1 | foo | 1
> 2 | bar | 2
> | | 3
> (3 rows)
Yes, this is very helpful. I was afraid the JSON would overwhelm the
example, but this looks good. I wrote the attached doc patch which I
think improves this. I plan to apply it to all supported versions.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com
The usefulness of a cup is in its emptiness, Bruce Lee
Вложения
Bruce Momjian <bruce@momjian.us> writes:
> Yes, this is very helpful. I was afraid the JSON would overwhelm the
> example, but this looks good. I wrote the attached doc patch which I
> think improves this. I plan to apply it to all supported versions.
Couple thoughts:
* Taking the initial ... out of the syntax synopsis is not an improvement.
It makes it look like you can only apply AS to the first function of a
ROWS FROM.
* I think the ORDER BY adds nothing to the example except complication
and confusion.
* Maybe the other sentence of explanation would read better as
json_to_recordset() is instructed to return two columns,
the first integer and the second text. The result of
generate_series() is used directly.
regards, tom lane
On Thu, Sep 24, 2020 at 03:05:24PM -0400, Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Yes, this is very helpful. I was afraid the JSON would overwhelm the > > example, but this looks good. I wrote the attached doc patch which I > > think improves this. I plan to apply it to all supported versions. > > Couple thoughts: > > * Taking the initial ... out of the syntax synopsis is not an improvement. > It makes it look like you can only apply AS to the first function of a > ROWS FROM. Oh, so the dots represent optional non-column_definition function calls. I can't think if a cleaner way to show that, so I guess "..." will have to do. > * I think the ORDER BY adds nothing to the example except complication > and confusion. I wanted to highlight that the column_definition specifies the data type of the column in other parts of the query. > * Maybe the other sentence of explanation would read better as > > json_to_recordset() is instructed to return two columns, > the first integer and the second text. The result of > generate_series() is used directly. OK, better. New patch attached. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EnterpriseDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee
Вложения
On Thu, Sep 24, 2020 at 12:37 PM Bruce Momjian <bruce@momjian.us> wrote:
On Thu, Sep 24, 2020 at 03:05:24PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Yes, this is very helpful. I was afraid the JSON would overwhelm the
> > example, but this looks good. I wrote the attached doc patch which I
> > think improves this. I plan to apply it to all supported versions.
>
> Couple thoughts:
>
> * Taking the initial ... out of the syntax synopsis is not an improvement.
> It makes it look like you can only apply AS to the first function of a
> ROWS FROM.
Oh, so the dots represent optional non-column_definition function calls.
I can't think if a cleaner way to show that, so I guess "..." will have
to do.
My original comment was a question predicated upon the fact that the SELECT reference page doesn't include the leading; probably should add them there as part of this patch.
OK, better. New patch attached.
LGTM
David J.
On Tue, Sep 29, 2020 at 08:53:51PM -0700, David G. Johnston wrote: > On Thu, Sep 24, 2020 at 12:37 PM Bruce Momjian <bruce@momjian.us> wrote: > > On Thu, Sep 24, 2020 at 03:05:24PM -0400, Tom Lane wrote: > > Bruce Momjian <bruce@momjian.us> writes: > > > Yes, this is very helpful. I was afraid the JSON would overwhelm the > > > example, but this looks good. I wrote the attached doc patch which I > > > think improves this. I plan to apply it to all supported versions. > > > > Couple thoughts: > > > > * Taking the initial ... out of the syntax synopsis is not an > improvement. > > It makes it look like you can only apply AS to the first function of a > > ROWS FROM. > > Oh, so the dots represent optional non-column_definition function calls. > I can't think if a cleaner way to show that, so I guess "..." will have > to do. > > > My original comment was a question predicated upon the fact that the SELECT > reference page doesn't include the leading; probably should add them there as > part of this patch. I looked at that but in the SELECT manual page case, the example is already part of a larger list of options so I don't think it is needed. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EnterpriseDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee
On Thu, Sep 24, 2020 at 03:37:14PM -0400, Bruce Momjian wrote: > On Thu, Sep 24, 2020 at 03:05:24PM -0400, Tom Lane wrote: > > Bruce Momjian <bruce@momjian.us> writes: > > > Yes, this is very helpful. I was afraid the JSON would overwhelm the > > > example, but this looks good. I wrote the attached doc patch which I > > > think improves this. I plan to apply it to all supported versions. > > > > Couple thoughts: > > > > * Taking the initial ... out of the syntax synopsis is not an improvement. > > It makes it look like you can only apply AS to the first function of a > > ROWS FROM. > > Oh, so the dots represent optional non-column_definition function calls. > I can't think if a cleaner way to show that, so I guess "..." will have > to do. > > > * I think the ORDER BY adds nothing to the example except complication > > and confusion. > > I wanted to highlight that the column_definition specifies the data type > of the column in other parts of the query. > > > * Maybe the other sentence of explanation would read better as > > > > json_to_recordset() is instructed to return two columns, > > the first integer and the second text. The result of > > generate_series() is used directly. > > OK, better. New patch attached. Patch applied to all supported versions. Thanks to Tom for proposing the example query, and to the original reporter. :-) -- Bruce Momjian <bruce@momjian.us> https://momjian.us EnterpriseDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee