Sorry, link the image: https://issues.apache.org/jira/secure/attachment/13076077/image-2025-04-17-14-16-29-776.png
From: Zhang, Juntao <juntzh...@ebay.com.INVALID> Date: Thursday, April 17, 2025 at 14:13 To: dev@calcite.apache.org <dev@calcite.apache.org>, mbu...@gmail.com <mbu...@gmail.com>, jhyde.apa...@gmail.com <jhyde.apa...@gmail.com> Subject: Re: [DISCUSS] （CALCITE-6944）Cannot parse parenthesized partition by in Table Function Hi all, Thanks Julian and Mihai, glad to hear your guidance, sorry for the late reply. Because I took the time to gain a better understanding of the related knowledge, and I appreciate your patience. After research I find that SQL 2016 standard(chapter 9 invocation)[1] introduced a way of defining PTF including: define <table primary>, <SQL argument list>,<SQL argument>,<table argument>... here is a sample to illustrate table argument, which show <table argument> wrapper by parentheses is not the standard: [izcNyYm+kSIECAAAECBAgQIECAwIIEiu85XpCBqRAgQIAAAQIECBAgQIBA4wLCceMLwPQJECBAgAABAgQIECBAIATh2CogQIAAAQIECBAgQIAAgeYFhOPmlwAAAgQIECBAgAABAgQIEBCOrQECBAgQIECAAAECBAgQaF5AOG5+CQAgQIAAAQIECBAgQIAAAeHYGiBAgAABAgQIECBAgACB5gW+AavW0IJ37v2pAAAAAElFTkSuQmCC] And I try this in Oracle, find it can enclose table name in parenthesis, but not the analytic clause[2]. And this sample[3] demonstrate that <table argument> with parentheses in oracle is not working. I also try it in Trino, which follow the SQL 2016 standard [1], not support parentheses wrapper by parentheses. My conclusion : 1. topn( (TABLE orders PARTITION BY productid) ) as I mentioned before, this parsing should not be supported in Calcite. 2. The toSqlString method should not return an unparsable and nonstandard result to users, the parentheses need be removed from the <table argument> Should: topn( TABLE orders PARTITION BY productid ) NOT : topn( (TABLE orders PARTITION BY productid) ) 1. As the example shows <table argument proper> TABLE(orders) is defined in SQL 2016 [1] and supported by Trino[4] and Oracle[2], need to support in Calcite, I am not sure why we use TABLE order instead of TABLE(orders)? Anybody help to explain? Feel free to share any doubts or questions you have. I’m glad to discuss with you. [1] SO/IEC 19075-7:2021<https://www.iso.org/standard/78938.html>(SQL 2016 standard) [2] https://docs.oracle.com/en/database/oracle/oracle-database/23/lnpls/overview-polymorphic-table-functions.html#GUID-4847CB51-6939-44C4-9913-CC3CE13B6730 [3] https://forums.oracle.com/ords/apexds/post/polymorphic-table-function-with-parentheses-in-partition-by-1913 [4] https://trino.io/docs/current//functions/table.html<https://trino.io/docs/current/functions/table.html> Thanks Juntao From: Julian Hyde <jhyde.apa...@gmail.com> Date: Wednesday, April 16, 2025 at 02:29 To: dev@calcite.apache.org <dev@calcite.apache.org> Subject: Re: [DISCUSS] （CALCITE-6944）Cannot parse parenthesized partition by in Table Function External Email 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%7C29c730f82e73432586f508dd7c4b785c%7C46326bff992841a0baca17c16c94ea99%7C0%7C0%7C638803385788799583%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=%2BD93maj0JmjxVNkyvL%2FamvMJpl4zzvP0kxd8UoKqXyg%3D&reserved=0<https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FCALCITE-6944&data=05%7C02%7Cjuntzhang%40ebay.com%7C29c730f82e73432586f508dd7c4b785c%7C46326bff992841a0baca17c16c94ea99%7C0%7C0%7C638803385788834577%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=fOIiY5zPx4XdY6njmJ0k60C1YXEs8l1pPMNStUFAHyg%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%7C29c730f82e73432586f508dd7c4b785c%7C46326bff992841a0baca17c16c94ea99%7C0%7C0%7C638803385788852119%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=wJUUDIgur7dtVIZFvhRxpd5QwqzbB04SrT0ZLcy%2BqBU%3D&reserved=0<https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcalcite%2Fpull%2F4295&data=05%7C02%7Cjuntzhang%40ebay.com%7C29c730f82e73432586f508dd7c4b785c%7C46326bff992841a0baca17c16c94ea99%7C0%7C0%7C638803385788871732%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=Vtk8Pm%2F7SZMGnDXLBmCT%2FSswXxqs%2BOYuGEIqjjtZ30k%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
