Thread: WITH ORDINALITY versus column definition lists

Consider the following case of a function that requires a column
definition list (example borrowed from the regression tests):

create function array_to_set(anyarray) returns setof record as $$ select i AS "index", $1[i] AS "value" from
generate_subscripts($1,1) i
 
$$ language sql strict immutable;

select * from array_to_set(array['one', 'two']) as t(f1 int,f2 text);

What if you want to add ordinality to that?  In HEAD you get:

regression=# select * from array_to_set(array['one', 'two']) with ordinality as t(f1 int,f2 text);
ERROR:  WITH ORDINALITY is not supported for functions returning "record"
LINE 1: select * from array_to_set(array['one', 'two']) with ordinal...                     ^

which is a restriction imposed by the original WITH ORDINALITY patch.
The currently-submitted patch removes this restriction (although not the
documentation about it :-(), and what you get is

regression=# select * from array_to_set(array['one', 'two']) with ordinality as t(f1 int,f2 text);f1 | f2  | ordinality

----+-----+------------ 1 | one |          1 2 | two |          2
(2 rows)

Notice that the coldef list doesn't include the ordinality column, so in
this syntax there is no way to choose a different name for the ordinality
column.  The new TABLE syntax provides an arguably-saner solution:

regression=# select * from table(array_to_set(array['one', 'two']) as (f1 int,f2 text)) with ordinality;f1 | f2  |
ordinality
 
----+-----+------------ 1 | one |          1 2 | two |          2
(2 rows)

regression=# select * from table(array_to_set(array['one', 'two']) as (f1 int,f2 text)) with ordinality as
t(a1,a2,a3);a1| a2  | a3 
 
----+-----+---- 1 | one |  1 2 | two |  2
(2 rows)

Now, it seems to me that putting WITH ORDINALITY on the same syntactic
level as the coldeflist is pretty confusing, especially since it behaves
differently than WITH ORDINALITY with a simple alias list:

regression=# select * from generate_series(1,2) with ordinality as t(f1,f2);f1 | f2 
----+---- 1 |  1 2 |  2
(2 rows)

Here, the alias list does extend to the ordinality column.

It seems to me that we don't really want this behavior of the coldeflist
not including the ordinality column.  It's operating as designed, maybe,
but it's unexpected and confusing.  We could either

1. Reinsert HEAD's prohibition against directly combining WITH ORDINALITY
with a coldeflist (with a better error message and a HINT suggesting that
you can get what you want via the TABLE syntax).

2. Change the parser so that the coldeflist is considered to include the
ordinality column, for consistency with the bare-alias case.  We'd
therefore insist that the last coldeflist item be declared as int8, and
then probably have to strip it out internally.

Thoughts?
        regards, tom lane

Tom Lane-2 wrote
> It seems to me that we don't really want this behavior of the coldeflist
> not including the ordinality column.  It's operating as designed, maybe,
> but it's unexpected and confusing.  We could either
> 
> 1. Reinsert HEAD's prohibition against directly combining WITH ORDINALITY
> with a coldeflist (with a better error message and a HINT suggesting that
> you can get what you want via the TABLE syntax).
> 
> 2. Change the parser so that the coldeflist is considered to include the
> ordinality column, for consistency with the bare-alias case.  We'd
> therefore insist that the last coldeflist item be declared as int8, and
> then probably have to strip it out internally.

#2 but I am hoping to be able to make the definition of the column optional. 
One possibility is that if you do want to provide an alias you have to make
it clear that the coldeflist item in question is only valid for a with
ordinality column alias.  Otherwise the entire coldeflist is used to alias
the record-type output and the ordinality column is provided its default
name.

Two options I came up with:

1) disallow any type specifier on the last item:  t(f1 int, f2 text, o1)
2) add a new pseudo-type, "ord":  t(f1 int, f2 text, o1 ord)

I really like option #2.  It makes it perfectly clear, entirely within the
coldeflist SQL, that the last column is different and in this case optional
both in the sense of providing an alias and also the user can drop the whole
ordinality aspect of the call as well.  The system does not need to be told,
by the user, the actual type of the ordinality column.  And given that I
would supposed most people would think to use "int" or "bigint" before using
"int8" the usability there is improved once they need and then learn that to
alias the ordinality column they use the "ord" type which would internally
resolve to the necessary output type.

Option one is somewhat simpler but the slight added verbosity makes reading
the SQL coldeflist easier, IMO, since you are already scanning name-type
pairs and recognizing the missing type is, for me, harder than reading off
"ord" and recalling its meaning.

David J.





--
View this message in context:
http://postgresql.1045698.n5.nabble.com/WITH-ORDINALITY-versus-column-definition-lists-tp5779443p5779449.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

David Johnston <polobo@yahoo.com> writes:
> Tom Lane-2 wrote
>> It seems to me that we don't really want this behavior of the coldeflist
>> not including the ordinality column.  It's operating as designed, maybe,
>> but it's unexpected and confusing.  We could either
>> 
>> 1. Reinsert HEAD's prohibition against directly combining WITH ORDINALITY
>> with a coldeflist (with a better error message and a HINT suggesting that
>> you can get what you want via the TABLE syntax).
>> 
>> 2. Change the parser so that the coldeflist is considered to include the
>> ordinality column, for consistency with the bare-alias case.  We'd
>> therefore insist that the last coldeflist item be declared as int8, and
>> then probably have to strip it out internally.

> Two options I came up with:

> 1) disallow any type specifier on the last item:  t(f1 int, f2 text, o1)
> 2) add a new pseudo-type, "ord":  t(f1 int, f2 text, o1 ord)

> I really like option #2.

I don't.  Pseudo-types have a whole lot of baggage.  #1 is a mess too.
And in either case, making coldef list items optional increases the number
of ways to make a mistake, if you accidentally omit some other column for
instance.

Basically the problem here is that it's not immediately obvious whether
the coldef list ought to include the ordinality column or not.  The user
would probably guess not (since the system knows what type ordinality
should be).  Unless he's trying to specify a column name for the ordinality
column, in which case he'll realize the syntax forces it to be there.
Any way you slice it, that's going to lead to confusion and bug reports.

The TABLE syntax is really a vastly better solution for this.  So I'm
thinking my #1 is the best answer, assuming we can come up with a good
error message.  My first attempt would be

ERROR: WITH ORDINALITY cannot be used with a column definition list
HINT: Put the function's column definition list inside TABLE() syntax.

Better ideas?
        regards, tom lane

Tom Lane-2 wrote
> David Johnston <

> polobo@

> > writes:
>> Tom Lane-2 wrote
>>> It seems to me that we don't really want this behavior of the coldeflist
>>> not including the ordinality column.  It's operating as designed, maybe,
>>> but it's unexpected and confusing.  We could either
>>> 
>>> 1. Reinsert HEAD's prohibition against directly combining WITH
>>> ORDINALITY
>>> with a coldeflist (with a better error message and a HINT suggesting
>>> that
>>> you can get what you want via the TABLE syntax).
>>> 
>>> 2. Change the parser so that the coldeflist is considered to include the
>>> ordinality column, for consistency with the bare-alias case.  We'd
>>> therefore insist that the last coldeflist item be declared as int8, and
>>> then probably have to strip it out internally.
> 
>> Two options I came up with:
> 
>> 1) disallow any type specifier on the last item:  t(f1 int, f2 text, o1)
>> 2) add a new pseudo-type, "ord":  t(f1 int, f2 text, o1 ord)
> 
>> I really like option #2.
> 
> I don't.  Pseudo-types have a whole lot of baggage.  #1 is a mess too.
> And in either case, making coldef list items optional increases the number
> of ways to make a mistake, if you accidentally omit some other column for
> instance.

I'll have to trust on the baggage/mess conclusion but if you can distinctly
and un-ambigiously identify the coldeflist item that is to be used for
ordinality column aliasing then the mistakes related to the
function-record-coldeflist are the same as now.  There may be more (be still
quite few I would think) ways for the user to make a mistake but the syntax
ones are handled anyway and so if the others can be handled reasonably well
the UI for the feature becomes more friendly.

IOW, instead of adding int8 and ignoring it we poll the last item,
conditionally discard it (like the int8 case), then handle the possibly
modified structure as planned.


> Basically the problem here is that it's not immediately obvious whether
> the coldef list ought to include the ordinality column or not.  The user
> would probably guess not (since the system knows what type ordinality
> should be).  

Yes, if the column is not made optional somehow then I dislike option #2


> The TABLE syntax is really a vastly better solution for this.  So I'm
> thinking my #1 is the best answer, assuming we can come up with a good
> error message.  My first attempt would be
> 
> ERROR: WITH ORDINALITY cannot be used with a column definition list
> HINT: Put the function's column definition list inside TABLE() syntax.
> 
> Better ideas?

Works for me if #1 is implemented.



Just to clarify we are still allowing simple aliasing:

select * from generate_series(1,2) with ordinality as t(f1,f2); 

Its only when the output of the function is "record" does the restriction of
placing the record-returning function call into TABLE (if you want ordinals)
come into play.


select * from table(array_to_set(array['one', 'two']) as (f1 int,f2 text))
with ordinality as t(a1,a2,a3); 

If we could do away with having to re-specify the record-aliases in the
outer layer (a1, a2) then I'd be more understanding but I'm thinking that is
not possible unless you force a single-column alias definition attached to
WITH ORDINALITY to mean alias the ordinality column only.


On the plus side: anyone using record-returning functions is already dealing
with considerable verbosity so this extra bit doesn't seem to be adding that
much overhead; and since the alias - t(a1,a2,a3) - is optional if you don't
care about aliasing the with ordinal column the default case is not that
verbose (just add the surrounding TABLE).

I feel like I need a flow-chart for #1...

With #2 (w/ optional) you can add in an alias for the ordinality column
anyplace you would be specifying a coldeflist OR alias list.  Favoring the
pseudo-type solution is the fact that given the prior sentence if you place
"o1 ord" in the wrong place it is possible to generate an error like "with
ordinality not present for aliasing".

#1 is simpler to implement and does not preclude #2 in the future.

Possible #3?

Not sure if this is possible at this point but really the alias for the
ordinality column would be attached directly to the ordinality keyword.

e.g., ...) with ordinality{alias} as t(a1, a2)

David J.








--
View this message in context:
http://postgresql.1045698.n5.nabble.com/WITH-ORDINALITY-versus-column-definition-lists-tp5779443p5779468.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

David Johnston <polobo@yahoo.com> writes:
> Just to clarify we are still allowing simple aliasing:

> select * from generate_series(1,2) with ordinality as t(f1,f2); 

Right, that works (and is required by spec, I believe).  It's what to
do with our column-definition-list extension that's at issue.

> Not sure if this is possible at this point but really the alias for the
> ordinality column would be attached directly to the ordinality keyword.

> e.g., ...) with ordinality{alias} as t(a1, a2)

This has no support in the standard.
        regards, tom lane

Tom Lane-2 wrote
> David Johnston <

> polobo@

> > writes:
>> Just to clarify we are still allowing simple aliasing:
> 
>> select * from generate_series(1,2) with ordinality as t(f1,f2); 
> 
> Right, that works (and is required by spec, I believe).  It's what to
> do with our column-definition-list extension that's at issue.
> 
>> Not sure if this is possible at this point but really the alias for the
>> ordinality column would be attached directly to the ordinality keyword.
> 
>> e.g., ...) with ordinality{alias} as t(a1, a2)
> 
> This has no support in the standard.

Now I'm just spinning some thoughts:

) with ordinality AS t(a1 text, a2 text | ord1)  -- type-less, but a
different separator

) with ordinality AS t(a1 text, a2 text)(ord1) -- stick it in its own
section, type-less

) with ordinality AS t(a1 text, a2 text) ordinal(ord1) --name the section
too

would probably want to extend the alias syntax to match...

Is there any precedent in other RDBMS to consider?

I don't see any obvious alternatives to the ones you listed and syntax is
really not a huge barrier.  If the implementation of an optionally specified
alias is a barrier then either someone needs to feel strongly enough to
implement it or just default to #1 for the time being.

But others really haven't had a chance to read and respond yet so I'm gonna
get off this train for a while.


David J.





--
View this message in context:
http://postgresql.1045698.n5.nabble.com/WITH-ORDINALITY-versus-column-definition-lists-tp5779443p5779473.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
Tom> 1. Reinsert HEAD's prohibition against directly combining WITHTom> ORDINALITY with a coldeflist (with a better
errormessage and aTom> HINT suggesting that you can get what you want via the TABLETom> syntax).
 

That gets my vote.

-- 
Andrew (irc:RhodiumToad)

On 20 November 2013 22:46, Andrew Gierth <andrew@tao11.riddles.org.uk> wrote:
>>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
>
>  Tom> 1. Reinsert HEAD's prohibition against directly combining WITH
>  Tom> ORDINALITY with a coldeflist (with a better error message and a
>  Tom> HINT suggesting that you can get what you want via the TABLE
>  Tom> syntax).
>
> That gets my vote.
>

Yeah that seems preferable to option #2, which just seems to open up a
whole can of worms.

However, I think I would quickly find it a PITA that it kept telling
me to wrap it in a TABLE() construct. It would seem like the "TABLE"
was just an unnecessary noise word (from a user perspective). Could we
simply support an alias list after the ORDINALITY, in addition to the
coldeflist? For example:

select * from array_to_set(array['one', 'two']) as (f1 int,f2 text)
with ordinality as t(a1,a2,a3);

That could be regarded as an implicit TABLE() construct, but the
syntax would be closer to the non-coldeflist case:

[ LATERAL ] function_name ( [ argument [, ...] ] ) [ WITH ORDINALITY ] [ [ AS ] alias [ ( column_alias [, ...] ) ] ]

[ LATERAL ] function_name ( [ argument [, ...] ] ) [ AS ] [ alias ] ( column_definition [, ...] ) [ WITH ORDINALITY ] [
[AS ] alias [ ( column_alias [, ...] ) ] ]
 

Regards,
Dean

Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> On 20 November 2013 22:46, Andrew Gierth <andrew@tao11.riddles.org.uk> wrote:
> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
>> Tom> 1. Reinsert HEAD's prohibition against directly combining WITH
>> Tom> ORDINALITY with a coldeflist (with a better error message and a
>> Tom> HINT suggesting that you can get what you want via the TABLE
>> Tom> syntax).
>> 
>> That gets my vote.

> Yeah that seems preferable to option #2, which just seems to open up a
> whole can of worms.

> However, I think I would quickly find it a PITA that it kept telling
> me to wrap it in a TABLE() construct. It would seem like the "TABLE"
> was just an unnecessary noise word (from a user perspective). Could we
> simply support an alias list after the ORDINALITY, in addition to the
> coldeflist?

This seems like way too much complication to save a couple of keystrokes
in a corner case.  Two separate AS clauses applying to the same FROM
item seems mighty confusing to me ...
        regards, tom lane