You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2022/09/17 11:39:10 UTC
[GitHub] [calcite] l4wei opened a new pull request, #2910: [CALCITE-5265] Select operator' parentheses should be same with Union operator
l4wei opened a new pull request, #2910:
URL: https://github.com/apache/calcite/pull/2910
remove necessary parentheses on select clause.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] libenchao closed pull request #2910: [CALCITE-5265] Select operator' parentheses should be same with Union operator
Posted by GitBox <gi...@apache.org>.
libenchao closed pull request #2910: [CALCITE-5265] Select operator' parentheses should be same with Union operator
URL: https://github.com/apache/calcite/pull/2910
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] libenchao commented on pull request #2910: [CALCITE-5265] Select operator' parentheses should be same with Union operator
Posted by GitBox <gi...@apache.org>.
libenchao commented on PR #2910:
URL: https://github.com/apache/calcite/pull/2910#issuecomment-1260301791
@l4wei I didn't notice Julian has another comment when I commented with "LGTM", could you add another test which addresses Julian's comment?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] libenchao commented on a diff in pull request #2910: [CALCITE-5265] Select operator' parentheses should be same with Union operator
Posted by GitBox <gi...@apache.org>.
libenchao commented on code in PR #2910:
URL: https://github.com/apache/calcite/pull/2910#discussion_r981235738
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -6352,6 +6352,20 @@ private void checkLiteral2(String expression, String expected) {
.withCalcite().ok(expectedCalciteX);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5265">[CALCITE-5265]
+ * Select operator' parentheses should be same with Union operator</a>. */
+ @Test void testInsertValueWithDynamicParams() {
Review Comment:
Are there any tests which cover what Julian has mentioned in the JIRA comment?
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -6352,6 +6352,20 @@ private void checkLiteral2(String expression, String expected) {
.withCalcite().ok(expectedCalciteX);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5265">[CALCITE-5265]
+ * Select operator' parentheses should be same with Union operator</a>. */
Review Comment:
The comment should also be adapted according to the changes in JIRA title.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] l4wei commented on a diff in pull request #2910: [CALCITE-5265] Select operator' parentheses should be same with Union operator
Posted by GitBox <gi...@apache.org>.
l4wei commented on code in PR #2910:
URL: https://github.com/apache/calcite/pull/2910#discussion_r981264826
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -6352,6 +6352,20 @@ private void checkLiteral2(String expression, String expected) {
.withCalcite().ok(expectedCalciteX);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5265">[CALCITE-5265]
+ * Select operator' parentheses should be same with Union operator</a>. */
Review Comment:
OK, done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] l4wei commented on pull request #2910: [CALCITE-5265] Select operator' parentheses should be same with Union operator
Posted by GitBox <gi...@apache.org>.
l4wei commented on PR #2910:
URL: https://github.com/apache/calcite/pull/2910#issuecomment-1260304022
>
OK
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] l4wei commented on a diff in pull request #2910: [CALCITE-5265] Select operator' parentheses should be same with Union operator
Posted by GitBox <gi...@apache.org>.
l4wei commented on code in PR #2910:
URL: https://github.com/apache/calcite/pull/2910#discussion_r981260935
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -6352,6 +6352,20 @@ private void checkLiteral2(String expression, String expected) {
.withCalcite().ok(expectedCalciteX);
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-5265">[CALCITE-5265]
+ * Select operator' parentheses should be same with Union operator</a>. */
+ @Test void testInsertValueWithDynamicParams() {
Review Comment:
There are many unit tests cover "insert into aTable select ? from bTable" in SqlParserTest.java. And RelToSqlConverterTest::testInsertValuesWithDynamicParams will cover "insert into aTable select ? from bTable union all select ? from cTable" mentioned by Julian in JIRA comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] l4wei commented on pull request #2910: [CALCITE-5265] Select operator' parentheses should be same with Union operator
Posted by GitBox <gi...@apache.org>.
l4wei commented on PR #2910:
URL: https://github.com/apache/calcite/pull/2910#issuecomment-1260323271
>
I have added test that mentioned by Julian' last comment in third commit in this PR. FYI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org