I agree with Mihai, but in some cases there are several correct representations. For example, the expression “x + y * z” can be represented as “x + y * z”, “x + (y * z)” and “(x + (y * z))”. All are valid representations, and the unparser has a mode where it will generate the last one, with lots of parentheses, because you can read off the AST without having to know the precedence of each operator.
For SQL constructs that are not expressions, parentheses may not be valid, and therefore there are fewer correct representations. For example, “select * from emp order by deptno desc” is valid but “select * from emp order by (deptno desc)” is not. In these cases the unparser must never generate parentheses. The unparser test helps ensure that. Julian > On Apr 15, 2025, at 10:09 AM, Mihai Budiu <mbu...@gmail.com> wrote: > > toSqlString emits a SQL program that should be valid in a specified dialect. > The dialect could be ANSI SQL, but it could also be a different one. > > Calcite is very flexible; it can output SQL in different dialects; it also > has a very flexible and configurable parser (in fact, several parsers, > including core, server, and Babel), which can accept constructs from multiple > dialects. > > So try to forget about Calcite itself, and state what you want to achieve. > E.g., > "the following statement ... should be accepted by the parser", or "the > following construct in dialect X should be emitted as ..." > > Mihai > ________________________________ > From: Zhang, Juntao <juntzh...@ebay.com> > Sent: Tuesday, April 15, 2025 12:01 AM > To: dev@calcite.apache.org <dev@calcite.apache.org>; mbu...@gmail.com > <mbu...@gmail.com> > Subject: Re: [DISCUSS] （CALCITE-6944）Cannot parse parenthesized partition by > in Table Function > > > Hi Mihai, > > Thank you very much for letting me know the community principle. > > There is another question, does toSqlString follow the sql standard, or just > plain display use. > > Do you think it is reasonable to fix it in toSqlString side, currently the > key is that after toSqlString cause the parsing exception. > > > > > > From: Mihai Budiu <mbu...@gmail.com> > Date: Tuesday, April 15, 2025 at 12:15 > To: dev@calcite.apache.org <dev@calcite.apache.org> > Subject: Re: [DISCUSS] （CALCITE-6944）Cannot parse parenthesized partition by > in Table Function > > External Email > > There are a lot of open PRs, so it's easy for one to fall through the cracks. > > I think you should answer a simple question: what is the correct grammar? You > are making it look like you are tweaking the grammar to allow it to parse > what the output of the compiler is. But that's the wrong question. The > compiler should output what the correct output is. > > "Correct" is defined as in "specified by the standard" or "supported by other > databases". > > I didn't review your PR because I don't know the answer to this last question. > > Mihai > > ________________________________ > From: Zhang, Juntao > Sent: Monday, April 14, 2025 9:03 PM > To: dev@calcite.apache.org > Subject: [DISCUSS] （CALCITE-6944）Cannot parse parenthesized partition by in > Table Function > > Hi team, > For > https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FCALCITE-6944&data=05%7C02%7Cjuntzhang%40ebay.com%7C9e3371ad1ffa4fc4fbd308dd7bd420a0%7C46326bff992841a0baca17c16c94ea99%7C0%7C0%7C638802873230509688%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=qPfNTxJprnl2oaGQ2AyO12y14L3F3ElFYQF6FQtN5SU%3D&reserved=0<https://issues.apache.org/jira/browse/CALCITE-6944>, > I’m happy to make the GitHub Pull Request > #4295<https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcalcite%2Fpull%2F4295&data=05%7C02%7Cjuntzhang%40ebay.com%7C9e3371ad1ffa4fc4fbd308dd7bd420a0%7C46326bff992841a0baca17c16c94ea99%7C0%7C0%7C638802873230537427%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=uFvCqvuDbCc%2FsnNW835ZxYHNp6pmFgQNz8shvHo7Q9w%3D&reserved=0<https://github.com/apache/calcite/pull/4295>> > to fix it as well if we think its worth fixing. Can someone tell me how to > move forward? > > This the issue of an extra parenthesis in the table function. > Why there is an extra parenthesis? > Currently toSqlString this method will add parentheses, which means that the > SQL after toSqlString cannot be parsed. > > Test example: > String sqlExpected = "f(a => TABLE t PARTITION BY f1 ORDER BY f2, b => 1)"; > String sqlActual = parseExpression(sqlExpected) > .toSqlString(new AnsiSqlDialect(SqlDialect.EMPTY_CONTEXT)).getSql(); > parseExpression(sqlActual); > > Thanks > Juntao
