Fail to parse - "select * from (table_name) alias" (Snowflake)

[hwchen]

Related to #154

sqlparser now parses select * from (table_name). However, it does not parse select * from (table_name) alias.

Would it make sense to add this functionality? If so, would that mean adding an alias field to TableWithJoins?

[hwchen]

It looks like select * from (select * from table_name) alias works.

Edit: Ah, of course this is expected, as seen in #154.

[nickolay]

As noted in #154, FROM (table_name) alias is not valid SQL. Which database supports that?

This becomes hard to reason about. The SQL standard allows FROM table_name alias and FROM (tab1 alias1 CROSS JOIN tab2 alias2). We relaxed our parser to allow omitting the JOIN inside the parens, making it accept FROM (tab1 alias1) too. But it doesn't make sense to allow FROM (table_name alias1) alias2. So I'm not sure what grammar the SQL dialect acceptingFROM (table_name) alias uses...

[hwchen]

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.

[nickolay]

Its docs don't explain how it handles this.. Out of curiosity, how does it react to FROM (table_name alias1) and FROM (table_name alias1) alias2?

[eyalleshem]

FROM (table_name alias1) works as expected ,
while
FROM (table_name alias1) alias2 complain about that alias2 is duplicate.

Don't sure exactly what the roles of this syntax ..

[nickolay]

Thanks @eyalleshem! I think forking parse_table_factor for Snowflake for now (and using a dialect-based switch in the parser - #241 to integrate it in the mainline) is the proper solution here.

[nickolay]

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.

[eyalleshem]

I opened a PR - should be enable that,
I took 2 assumptions (both of them correct in snowflake ):

1. Table could not be re-alias (select * from (t as a) as a1 should fail )
2. Alias not allowed in join of multiple table (select * from (t1 natural join t2) as a1 also should fail)

thats enable me to not change the AST , and in case of select * from ((t)) as a the alias will be add to the TableFactor::Table that represent t .

[nickolay]

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.

[eyalleshem]

Well,
My view is from the snowflake perspective - and from this perspective i see 2 advantage to the current solution :

1. Query that would not compile by snowflake - will not passed the parser
2. 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..

[nickolay]

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!