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

On Fri, Sep 5, 2014 at 6:27 PM, Marko Tiikkaja <marko(at)joh(dot)to> wrote:

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

​This option implemented in this specific manner.​

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

​My follow-on post posited a solution that still uses the EXPLAIN mechanism
but configured in a way so users can have it be always on if desired.​

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

​If you add lots more checks that is lots more ifs compared to a single if
to see if static analysis should be attempted in the first place. For a
single option its largely a wash. Beyond that I don't know enough to say
whether having the parser dispatch the query differently based upon it
being preceded with "EXPLAIN (STATIC)" or a standard query would be any
faster than a single IF for a single check.​

​The more options you can think of for a "novice" mode the more appealing a
formal static analysis interface becomes - both for execution and also
because the number of options being introduced and managed can be made into
formal configurations instead of an independent but related set of GUC
variables.

​David J.