Re: [PATCH] parser: optionally warn about missing AS for column and table aliases

From: David G Johnston <david(dot)g(dot)johnston(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] parser: optionally warn about missing AS for column and table aliases
Date: 2014-09-05 21:19:37
Message-ID: 1409951977558-5817980.post@n5.nabble.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Marko Tiikkaja-4 wrote
> On 2014-09-05 22:38, Oskari Saarenmaa wrote:
>> I wrote the attached patch to optionally emit warnings when column or
>> table
>> aliases are used without the AS keyword after errors caused by typos in
>> statements turning unintended things into aliases came up twice this
>> week.
>> First in a discussion with a colleague who was surprised by a 1 row
>> result
>> for the query 'SELECT COUNT(*) files' and again in the "pl/pgsql 2"
>> thread
>> as plpgsql currently doesn't throw an error if there are more result
>> columns
>> than output columns (SELECT a b INTO f1, f2).
>>
>> The patch is still missing documentation and it needs another patch to
>> modify all the statements in psql & co to use AS so you can use things
>> like
>> \d and tab-completion without triggering the warnings. I can implement
>> those changes if others think this patch makes sense.
>
> I think this is only problematic for column aliases. I wouldn't want to
> put these two to be put into the same category, as I always omit the AS
> keyword for tables aliases (and will continue to do so), but never omit
> it for column aliases.

I agree on being able to pick the behavior independently for select-list
items and the FROM clause items.

Using this to mitigate the pl/pgsql column mis-match issue seems like too
broad of a solution.

I probably couldn't mount a convincing defense of my opinion but at first
blush I'd say we should pass on this. Not with prejudice - looking at the
issue periodically has merit - but right now it seems to be mostly adding
clutter to the code without significant benefit.

Tangential - I'd rather see something like "EXPLAIN (STATIC)" that would
allow a user to quickly invoke a built-in static SQL analyzer on their query
and have it point this kind of thing out. Adding a catalog for STATIC
configurations in much the same way we have text search configurations would
allow extensions and users to define their own rulesets that could be
attached to their ROLE "GUC: default_static_analyzer_ruleset" and also
passed in as a modifier in the EXPLAIN invocation.

Stuff like correlated subqueries without alias use could be part of that (to
avoid typos that result in constant true) and also duplicate column names
floating to the top-most select-list, or failure to use an alias on a
function call result. There are probably others though I'm stretching to
even find these...

Anyway, the idea of using a GUC and evaluating every query that is written
(the added if statements), is not that appealing even if the impact of one
more check is likely to be insignificant (is it?)

David J.

--
View this message in context: http://postgresql.1045698.n5.nabble.com/PATCH-parser-optionally-warn-about-missing-AS-for-column-and-table-aliases-tp5817971p5817980.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David G Johnston 2014-09-05 21:33:00 Re: [PATCH] parser: optionally warn about missing AS for column and table aliases
Previous Message Oskari Saarenmaa 2014-09-05 21:12:35 Re: [PATCH] Filter error log statements by sqlstate