On 21 June 2013 08:31, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> On 21 June 2013 08:02, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>> On 21 June 2013 06:54, David Fetter <david@fetter.org> wrote:
>>>> For example "SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS file"
>>>
>>> The spec is pretty specific about the "all or none" nature of naming
>>> in the AS clause...unless we can figure out a way of getting around it
>>> somehow.
>>
>> We already support and document the ability to provide a subset of the
>> columns in the alias. I didn't realise that was beyond the spec, but
>> since we already have it...
>>
>>
>>> I'm weighing in on the side of a name that's like ?columnN? elsewhere
>>> in the code, i.e. one that couldn't sanely be an actual column name,
>>> whether in table, view, or SRF.
>>
>> I don't think we need to be overly concerned with coming up with a
>> unique name for the column. Duplicate column names are fine, except if
>> the user wants to refer to them elsewhere in the query, in which case
>> they need to provide aliases to distinguish them, otherwise the query
>> won't be accepted.
>>
>> I'd be happy if you just replaced "?column?" with ordinality in a
>> couple of places in your original patch.
>>
>
> Expanding on that, I think it's perfectly acceptable to allow
> potentially duplicate column names in this context. For the majority
> of simple queries the user wouldn't have to (and wouldn't feel
> compelled to) supply aliases. Where there was ambiguity they would be
> forced to do so, but people are already used to that.
>
> If I write a simple query that selects from a single unnest() with or
> without ordinality, I probably won't want to supply aliases.
>
> If I select from 2 unnest()'s *without* ordinality, I already have to
> provide aliases.
>
> If I select from 2 SRF's functions with ordinality, I won't be too
> surprised if I am also forced to provide aliases.
>
No one else has expressed an opinion about the naming of the new
column. All other review comments have been addressed, and the patch
looks to be in good shape, so I'm marking this as ready for committer.
IMO it's a very useful piece of new functionality. Good job!
Regards,
Dean