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

From: Marko Tiikkaja <marko(at)joh(dot)to>
To: David G Johnston <david(dot)g(dot)johnston(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Re: [PATCH] parser: optionally warn about missing AS for column and table aliases
Date: 2014-09-05 22:27:21
Message-ID: 540A38C9.8060301@joh.to
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2014-09-05 11:19 PM, David G Johnston wrote:
> Marko Tiikkaja-4 wrote
>> 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.

The PL/PgSQL column can be solved via other methods; see for example my
suggestion of "PL/PgSQL warning - num_into_expressions" in the current
commit fest (and the non-backwards-compatible solution of throwing a
run-time error I suggested). But some people run SQL queries from their
apps, and I've seen people make this mistake and get confused.

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

Are you talking about this particular option, or the notion of parser
warnings in general? Because if we're going to add a handful of
warnings, I don't see why this couldn't be on that list.

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

If you knew there's a good chance you might make a mistake, you could
probably avoid doing it in the first place. I make mistakes all the
time, would I then always execute two commands when I want to do
something? Having to run a special "check my query please" command
seems silly.

> 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?)

I'm not sure how you would do this differently in the EXPLAIN (STATIC)
case. Those ifs have to be somewhere, and that's always a non-zero cost.

That being said, yes, performance costs of course have to be evaluated
carefully.

.marko

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Johnston 2014-09-05 22:35:05 Re: Re: [PATCH] parser: optionally warn about missing AS for column and table aliases
Previous Message Bruce Momjian 2014-09-05 21:54:05 Re: Re: [PATCH] parser: optionally warn about missing AS for column and table aliases