Thread

Commits

Improve the error message for an inappropriate column definition list.
- ce90f075f0d8 14.0 landed

Please provide examples of rows from

The Post Office <noreply@postgresql.org> — 2020-04-08T21:50:44Z

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.

Re: Please provide examples of rows from

Bruce Momjian <bruce@momjian.us> — 2020-09-04T01:46:18Z

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

Re: Please provide examples of rows from

David G. Johnston <david.g.johnston@gmail.com> — 2020-09-15T01:49:22Z

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.

Re: Please provide examples of rows from

Bruce Momjian <bruce@momjian.us> — 2020-09-20T00:39:10Z

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 catdesc 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);

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

Re: Please provide examples of rows from

Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-20T00:49:53Z

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 catdesc 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.

			regards, tom lane

Re: Please provide examples of rows from

Bruce Momjian <bruce@momjian.us> — 2020-09-20T01:38:46Z

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, OUT catdesc 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

Re: Please provide examples of rows from

Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-20T14:57:14Z

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

Re: Please provide examples of rows from

Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-20T18:09:13Z

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

Re: Please provide examples of rows from

Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-20T18:14:00Z

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

Re: Please provide examples of rows from

Bruce Momjian <bruce@momjian.us> — 2020-09-24T18:52:19Z

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

Re: Please provide examples of rows from

Bruce Momjian <bruce@momjian.us> — 2020-09-24T18:55:11Z

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

Re: Please provide examples of rows from

Tom Lane <tgl@sss.pgh.pa.us> — 2020-09-24T19:05:24Z

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

Re: Please provide examples of rows from

Bruce Momjian <bruce@momjian.us> — 2020-09-24T19:37:14Z

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

Re: Please provide examples of rows from

David G. Johnston <david.g.johnston@gmail.com> — 2020-09-30T03:53:51Z

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.

Re: Please provide examples of rows from

Bruce Momjian <bruce@momjian.us> — 2020-09-30T17:54:32Z

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

Re: Please provide examples of rows from

Bruce Momjian <bruce@momjian.us> — 2020-10-05T20:28:28Z

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