Repository navigation
Fail to parse - "select * from (table_name) alias" (Snowflake) #223
Description
Activity
- changed the title
[-]Fail to parse - "select * from (table_name)"[/-][+]Fail to parse - "select * from (table_name) alias"[/+]on Jul 10, 2020 As noted in #154,
FROM (table_name) aliasis not valid SQL. Which database supports that?This becomes hard to reason about. The SQL standard allows
FROM table_name aliasandFROM (tab1 alias1 CROSS JOIN tab2 alias2). We relaxed our parser to allow omitting the JOIN inside the parens, making it acceptFROM (tab1 alias1)too. But it doesn't make sense to allowFROM (table_name alias1) alias2. So I'm not sure what grammar the SQL dialect acceptingFROM (table_name) aliasuses...I think it's reasonable not to handle this in this library, I can think of some workarounds. This is snowflake :) and just double-checked that this syntax works.
FROM (table_name alias1)works as expected ,
while
FROM (table_name alias1) alias2complain about that alias2 is duplicate.Don't sure exactly what the roles of this syntax ..
- changed the title
[-]Fail to parse - "select * from (table_name) alias"[/-][+]Fail to parse - "select * from (table_name) alias" (Snowflake)[/+]on Jul 27, 2020 Thanks @eyalleshem! I think forking
parse_table_factorfor Snowflake for now (and using a dialect-based switch in the parser - #241 to integrate it in the mainline) is the proper solution here.In my previous comment I forgot that there's also the problem of representing this in AST. @eyalleshem, as you indicated you'd like to work on this, please start by documenting the grammar the Snowflake dialect will implement here, and how it can fit into our AST.
I opened a PR - should be enable that,
I took 2 assumptions (both of them correct in snowflake ):- Table could not be re-alias (
select * from (t as a) as a1should fail ) - Alias not allowed in join of multiple table (
select * from (t1 natural join t2) as a1also should fail)
thats enable me to not change the AST , and in case of
select * from ((t)) as athe alias will be add to theTableFactor::Tablethat representt.- Table could not be re-alias (
Ah, so the only difference on the parser level is that of the three TableFactor variants (Table, Derived, or Nested), we currently allow aliases on the first two only, where your version parses it after Nested(Join) as well, but uses the logic you describe to find the inner TableFactor (Table or Derived) to store it in.
This is a smart solution, but I don't feel we gain much by not changing the AST, and it would be simpler to change NestedJoin to a
TableFactor::Nested { inner: TableWithJoins, alias: Option<TableAlias> }and leave it up to the consumer to choose:- either validate that
nested.inner.join.is_empty() && nested.alias.is_none()as required by the standard, - or implement the algorithm you described to figure out which relations an alias applies to.
- either validate that
Well,
My view is from the snowflake perspective - and from this perspective i see 2 advantage to the current solution :- Query that would not compile by snowflake - will not passed the parser
- The alias is actually for the inner table - so i think it be more easy for the consumer to use it
I agree that if we encounter other DB that allow single table in parens- but not follow the assumptions that describe above , maybe it's should be handled differently.
But if you think it's better , i don't care about changing the behaviour..
I realize that it is more convenient for consumers to have some semantic processing happen in the library, but simply parsing SQL is rather hard, so I'd rather not complicate matters by attempting to guess the semantics during parsing... Happy to hear what others think as well!
@nickolay - according to your suggestion - you would enable parsing joins with aliases even on the generic dialect ?
For example - do you think that this statement should be parsed in the generic dialect:
SELECT * from (t1 NATURAL JOIN t2 ) as a1.I asked that because the only difference that i see between snowflake and the ANSI sql here that the question if they allow "redundant" parens around single table factor , but snowflake not allow to alias "real joins" ( like the other dialect). so i don't sure that i feel comfortable to parse that on snowflake, but throw an error in the other dialect.
I think maybe i have other suggestion -
In snowflake dialect : if the
parse_table_and_joinsreturn aNestedJoin, but this object has onlyrelationwithoutjoins. we will replace theNestedJoinwith theTableFactorthat in therelation.This solution will eliminate the problem of aliasing real joins (joins with multiple tables) , as currently no database support that.
And redundant parens around single table will not treat anymore as joins (I think that what confused my most- because real join is between tables ).
Btw , this solution will have impact only on snowflake dialect...
do you think that this statement should be parsed in the generic dialect:
SELECT * from (t1 NATURAL JOIN t2 ) as a1.That's how it could work if my yesterday's suggestion were implemented, yes. There's no requirement that the generic dialect must be able to parse that, it follows naturally from the proposed design of the AST and lack of validation in the generic dialect...
Your new idea sounds promising though. If losing the information about the redundant parens is fine, I think it could be an elegant solution.
I update the PD according to the suggestion ,
I can't think about use-case that the information about the redundant parens is important to something ..- added a commit that references this issue
on Oct 13, 2020
Related to #154
sqlparser now parses
select * from (table_name). However, it does not parseselect * from (table_name) alias.Would it make sense to add this functionality? If so, would that mean adding an
aliasfield toTableWithJoins?