[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16592563#comment-16592563
]
ASF GitHub Bot commented on FLINK-10172:
fhueske commented on a change in pull request #6585:
[FLINK-10172][table]re-adding asc & desc suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585#discussion_r212797015
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/expressions/ExpressionParser.scala
##
@@ -324,6 +332,8 @@ object ExpressionParser extends JavaTokenParsers with
PackratParsers {
suffixToDate |
// expression for log
suffixLog |
+// expression for ordering
+suffixAsc | suffixDesc |
Review comment:
Naming the function `asicii` does not sound right to me. I'd never look for
this name if I'd was looking for ASCII conversion. There should be a proper fix
for this.
Please move this discussion to the ascii function PR.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.3.3, 1.4.2, 1.5.3, 1.6.0, 1.7.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.3.3, 1.4.3, 1.6.1, 1.7.0, 1.5.4
>
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16592552#comment-16592552
]
ASF GitHub Bot commented on FLINK-10172:
yanghua commented on a change in pull request #6585:
[FLINK-10172][table]re-adding asc & desc suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585#discussion_r212796106
##
File path:
flink-libraries/flink-table/src/main/scala/org/apache/flink/table/expressions/ExpressionParser.scala
##
@@ -324,6 +332,8 @@ object ExpressionParser extends JavaTokenParsers with
PackratParsers {
suffixToDate |
// expression for log
suffixLog |
+// expression for ordering
+suffixAsc | suffixDesc |
Review comment:
@walterddr when you add the keyword `ASC` and allow it as a suffix, my ASCII
scalar function test case :
```
testAllApis(
'f0.ascii(),
"f0.ascii()", //here
"ASCII(f0)",
"84")
```
will report this error:
```
org.apache.flink.table.api.ExpressionParserException: Could not parse
expression at column 7: string matching regex `(?i)\Qas\E' expected but `i'
found F0.ascii()
```
After commenting `suffixAsc |` , my test case will pass, but your test will
throw an exception.
I think I should rename `ASCII` to other name, such as `ASICII`? cc
@fhueske @twalthr @xccui
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.3.3, 1.4.2, 1.5.3, 1.6.0, 1.7.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.3.3, 1.4.3, 1.6.1, 1.7.0, 1.5.4
>
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16589876#comment-16589876
]
ASF GitHub Bot commented on FLINK-10172:
twalthr commented on issue #6585: [FLINK-10172][table]re-adding asc & desc
suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585#issuecomment-415329206
Thank you @walterddr. I would open an issue for this minor thing to at least
track it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.3.3, 1.4.2, 1.5.3, 1.6.0, 1.7.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.3.3, 1.4.3, 1.6.1, 1.7.0, 1.5.4
>
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16589439#comment-16589439
]
ASF GitHub Bot commented on FLINK-10172:
walterddr edited a comment on issue #6585: [FLINK-10172][table]re-adding asc &
desc suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585#issuecomment-415214473
@twalthr I just did a cross-check and all touched expressions are covered in
testings. :-) The only thing not covered is `"w.start()"` in addition to
`"start(w)"`. which I think we can leave it as it is not necessary to treat as
a special expression.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.3.3, 1.4.2, 1.5.3, 1.6.0, 1.7.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.3.3, 1.4.3, 1.6.1, 1.7.0, 1.5.4
>
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16589438#comment-16589438
]
ASF GitHub Bot commented on FLINK-10172:
walterddr commented on issue #6585: [FLINK-10172][table]re-adding asc & desc
suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585#issuecomment-415214473
@twalthr I just did a cross-check and all touched expressions are covered in
testings. :-)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.3.3, 1.4.2, 1.5.3, 1.6.0, 1.7.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.3.3, 1.4.3, 1.6.1, 1.7.0, 1.5.4
>
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16588986#comment-16588986
]
ASF GitHub Bot commented on FLINK-10172:
twalthr commented on issue #6585: [FLINK-10172][table]re-adding asc & desc
suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585#issuecomment-415064636
Thanks for the explanation @fhueske and @walterddr. Feel free to add more
tests if they make sense. But actually most of the operators should already
have tests.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.3.3, 1.4.2, 1.5.3, 1.6.0, 1.7.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.3.3, 1.4.3, 1.6.1, 1.7.0, 1.5.4
>
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16588945#comment-16588945
]
ASF GitHub Bot commented on FLINK-10172:
walterddr edited a comment on issue #6585: [FLINK-10172][table]re-adding asc &
desc suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585#issuecomment-415057802
@twalthr yes @fhueske is correct the 2 functions are special expressions
which requires special expression parsing. So far I am looking over the diff in
https://github.com/apache/flink/pull/1926/files and didn't find anything
abnormal but I will add additional tests for the lists of functions originally
in the `ExpressionParser` class. Should I open a JIRA for it? (haven't find a
bug yet)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.3.3, 1.4.2, 1.5.3, 1.6.0, 1.7.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.3.3, 1.4.3, 1.6.1, 1.7.0, 1.5.4
>
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16588944#comment-16588944
]
ASF GitHub Bot commented on FLINK-10172:
walterddr commented on issue #6585: [FLINK-10172][table]re-adding asc & desc
suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585#issuecomment-415057802
@twalthr yes @fhueske is correct the 2 functions are special expressions
which requires special expression parsing. So far I am looking over the diff in
https://github.com/apache/flink/pull/1926/files and didn't find anything
abnormal but I will add additional tests for the lists of functions originally
in the `ExpressionParser` class. Should I open a JIRA for it (haven't find a
bug yet)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.3.3, 1.4.2, 1.5.3, 1.6.0, 1.7.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.3.3, 1.4.3, 1.6.1, 1.7.0, 1.5.4
>
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16588463#comment-16588463
]
ASF GitHub Bot commented on FLINK-10172:
fhueske commented on issue #6585: [FLINK-10172][table]re-adding asc & desc
suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585#issuecomment-414936045
`asc` and `desc` are not regular functions. They are indicators for the sort
order and converted into special `Ordering` expressions. Without special
treatment, they are converted into `Call("asc", fieldRef)` and therefore not
correctly handled by `Table.orderBy()` which expects `Ordering` expressions.
This PR adds code that was (accidentially?) dropped when refactoring the
`ExpressionParser` in 9244106b334ef54ba3e39a3f2c0c76f46ae4ecd3.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.3.3, 1.4.2, 1.5.3, 1.6.0, 1.7.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.3.3, 1.4.3, 1.6.1, 1.7.0, 1.5.4
>
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16588429#comment-16588429
]
ASF GitHub Bot commented on FLINK-10172:
twalthr edited a comment on issue #6585: [FLINK-10172][table]re-adding asc &
desc suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585#issuecomment-414922702
@walterddr This PR fixes the issue but the question is why did the parser
not resolve the `asc`/`desc` correctly?
Actually, there is nothing special about these two functions.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.3.3, 1.4.2, 1.5.3, 1.6.0, 1.7.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.3.3, 1.4.3, 1.6.1, 1.7.0, 1.5.4
>
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16588428#comment-16588428
]
ASF GitHub Bot commented on FLINK-10172:
twalthr commented on issue #6585: [FLINK-10172][table]re-adding asc & desc
suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585#issuecomment-414922702
@walterddr This PR fixes the issue but the question is why did the parser
not resolve the `asc`/`desc` correctly?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.3.3, 1.4.2, 1.5.3, 1.6.0, 1.7.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
> Fix For: 1.3.3, 1.4.3, 1.6.1, 1.7.0, 1.5.4
>
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16588085#comment-16588085
]
ASF GitHub Bot commented on FLINK-10172:
asfgit closed pull request #6585: [FLINK-10172][table]re-adding asc & desc
suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/flink-libraries/flink-table/src/main/scala/org/apache/flink/table/api/scala/expressionDsl.scala
b/flink-libraries/flink-table/src/main/scala/org/apache/flink/table/api/scala/expressionDsl.scala
index dfe69cb0411..12f956b1a57 100644
---
a/flink-libraries/flink-table/src/main/scala/org/apache/flink/table/api/scala/expressionDsl.scala
+++
b/flink-libraries/flink-table/src/main/scala/org/apache/flink/table/api/scala/expressionDsl.scala
@@ -236,7 +236,18 @@ trait ImplicitExpressionOperations {
*/
def as(name: Symbol, extraNames: Symbol*) = Alias(expr, name.name,
extraNames.map(_.name))
+ /**
+* Specifies ascending order of an expression i.e. a field for orderBy call.
+*
+* @return ascend expression
+*/
def asc = Asc(expr)
+
+ /**
+* Specifies descending order of an expression i.e. a field for orderBy
call.
+*
+* @return descend expression
+*/
def desc = Desc(expr)
/**
diff --git
a/flink-libraries/flink-table/src/main/scala/org/apache/flink/table/expressions/ExpressionParser.scala
b/flink-libraries/flink-table/src/main/scala/org/apache/flink/table/expressions/ExpressionParser.scala
index 4b2440cf673..c0a577dccfd 100644
---
a/flink-libraries/flink-table/src/main/scala/org/apache/flink/table/expressions/ExpressionParser.scala
+++
b/flink-libraries/flink-table/src/main/scala/org/apache/flink/table/expressions/ExpressionParser.scala
@@ -48,6 +48,8 @@ object ExpressionParser extends JavaTokenParsers with
PackratParsers {
// Keyword
lazy val AS: Keyword = Keyword("as")
lazy val CAST: Keyword = Keyword("cast")
+ lazy val ASC: Keyword = Keyword("asc")
+ lazy val DESC: Keyword = Keyword("desc")
lazy val NULL: Keyword = Keyword("Null")
lazy val IF: Keyword = Keyword("?")
lazy val TO_DATE: Keyword = Keyword("toDate")
@@ -216,6 +218,12 @@ object ExpressionParser extends JavaTokenParsers with
PackratParsers {
// suffix operators
+ lazy val suffixAsc : PackratParser[Expression] =
+composite <~ "." ~ ASC ~ opt("()") ^^ { e => Asc(e) }
+
+ lazy val suffixDesc : PackratParser[Expression] =
+composite <~ "." ~ DESC ~ opt("()") ^^ { e => Desc(e) }
+
lazy val suffixCast: PackratParser[Expression] =
composite ~ "." ~ CAST ~ "(" ~ dataType ~ ")" ^^ {
case e ~ _ ~ _ ~ _ ~ dt ~ _ => Cast(e, dt)
@@ -324,6 +332,8 @@ object ExpressionParser extends JavaTokenParsers with
PackratParsers {
suffixToDate |
// expression for log
suffixLog |
+// expression for ordering
+suffixAsc | suffixDesc |
// expressions that take enumerations
suffixCast | suffixTrim | suffixTrimWithoutArgs | suffixExtract |
suffixFloor | suffixCeil |
// expressions that take literals
diff --git
a/flink-libraries/flink-table/src/test/scala/org/apache/flink/table/api/batch/table/stringexpr/SortStringExpressionTest.scala
b/flink-libraries/flink-table/src/test/scala/org/apache/flink/table/api/batch/table/stringexpr/SortStringExpressionTest.scala
new file mode 100644
index 000..204ec7726aa
--- /dev/null
+++
b/flink-libraries/flink-table/src/test/scala/org/apache/flink/table/api/batch/table/stringexpr/SortStringExpressionTest.scala
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.api.batch.table.stringexpr
+
+import org.apache.flink.api.scala._
+import org.apache.flink.table.api.scala._
+import org.apache.flink.table.utils.TableTestBase
+import org.junit.Test
+
+class SortStringExpressionTest extends Tabl
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16587978#comment-16587978
]
ASF GitHub Bot commented on FLINK-10172:
fhueske commented on issue #6585: [FLINK-10172][table]re-adding asc & desc
suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585#issuecomment-414814112
merging
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.6.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16586182#comment-16586182
]
ASF GitHub Bot commented on FLINK-10172:
fhueske commented on a change in pull request #6585:
[FLINK-10172][table]re-adding asc & desc suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585#discussion_r211326743
##
File path:
flink-libraries/flink-table/src/test/scala/org/apache/flink/table/api/batch/table/stringexpr/CalcStringExpressionTest.scala
##
@@ -85,6 +85,39 @@ class CalcStringExpressionTest extends TableTestBase {
verifyTableEquals(t1, t2)
}
+ @Test
+ def testSelectWithOrdering(): Unit = {
Review comment:
Can you move these tests into a new class `SortStringExpressionTest`?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.6.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16586173#comment-16586173
]
ASF GitHub Bot commented on FLINK-10172:
walterddr opened a new pull request #6585: [FLINK-10172][table]re-adding asc &
desc suffix expression to expressionParser
URL: https://github.com/apache/flink/pull/6585
## What is the purpose of the change
Fix Java expressionParser not correctly resolving `asc` and `desc` keyword.
## Brief change log
- *Adding back `suffixAsc` and `suffixDesc`*
## Verifying this change
- *Added table string expression test*
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (no)
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: (no)
- The serializers: (no)
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
- The S3 file system connector: (no)
## Documentation
- Does this pull request introduce a new feature? (no)
- If yes, how is the feature documented? (n/a)
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.6.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
> Labels: pull-request-available
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16586167#comment-16586167
]
Rong Rong commented on FLINK-10172:
---
Thanks for the pointer. I will take another look and add back test cases,
especially ones which are only tested under Scala.
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.6.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16586164#comment-16586164
]
Fabian Hueske commented on FLINK-10172:
---
This was broken when refactoring the expression parser in commit
[9244106b334ef54ba3e39a3f2c0c76f46ae4ecd3|https://github.com/apache/flink/commit/9244106b334ef54ba3e39a3f2c0c76f46ae4ecd3].
We should double check the changes and add tests for the broken features.
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.6.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16586092#comment-16586092
]
Rong Rong commented on FLINK-10172:
---
[~fhueske] yes, there were specific handling for {{ASC}} and {DESC}} in
{{ExpressionParser}} in previous Flink releases but somehow it was deleted. See
https://github.com/apache/flink/pull/1926/files. I was wondering if it is
possible that some other inconsistencies could have slipped through during the
restructuring fo the code recently.
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.6.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (FLINK-10172) Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
[
https://issues.apache.org/jira/browse/FLINK-10172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16585566#comment-16585566
]
Fabian Hueske commented on FLINK-10172:
---
[~walterddr] is correct. The problem is that the {{ExpressionParser}}
translates the {{"id.asc"}} into {{Call("ASC",
UnresolvedFieldReference("id"))}} instead of
{{ASC(UnresolvedFieldReference("id"))}}, i.e., the {{asc}} is not treated as a
special case but handled as generic function call.
> Inconsistentcy in ExpressionParser and ExpressionDsl for order by asc/desc
> --
>
> Key: FLINK-10172
> URL: https://issues.apache.org/jira/browse/FLINK-10172
> Project: Flink
> Issue Type: Bug
> Components: Table API & SQL
>Affects Versions: 1.6.0
>Reporter: Rong Rong
>Assignee: Rong Rong
>Priority: Major
>
> The following expression throws an exception in parsing {{"id.asc"}} term.
> {code:java}
> Table allOrders = orderTable
> .select("id,order_date,amount,customer_id")
> .orderBy("id.asc");
> {code}
> while it is correctly parsed for Scala:
> {code:scala}
> val allOrders:Table = orderTable
> .select('id, 'order_date, 'amount, 'customer_id)
> .orderBy('id.asc)
> {code}
> Anticipated some inconsistency between ExpressionParser and ExpressionDsl
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
