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 2020/06/26 21:09:24 UTC
[GitHub] [calcite] James-Jeyun-Kim opened a new pull request #2046: [CALCITE-4092] NPE using WITH clause without a corresponding SELECT F…
James-Jeyun-Kim opened a new pull request #2046:
URL: https://github.com/apache/calcite/pull/2046
…ROM (James Kim)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] James-Jeyun-Kim commented on pull request #2046: [CALCITE-4092] NPE using WITH clause without a corresponding SELECT F…
Posted by GitBox <gi...@apache.org>.
James-Jeyun-Kim commented on pull request #2046:
URL: https://github.com/apache/calcite/pull/2046#issuecomment-650405565
I deleted the overridden function "resolve" in WithScope.java because that was adding the view from the WITH clause into the scope of the main statement. This made the query pass by validation when the view was accessed from the main statement later without the necessary SELECT FROM [view].
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] laurentgo commented on a change in pull request #2046: [CALCITE-4092] NPE using WITH clause without a corresponding SELECT FROM (James Kim)
Posted by GitBox <gi...@apache.org>.
laurentgo commented on a change in pull request #2046:
URL: https://github.com/apache/calcite/pull/2046#discussion_r449251024
##########
File path: core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
##########
@@ -875,6 +877,20 @@ public final Sql sql(String sql) {
sql(sql).ok();
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-4092">[CALCITE-4092]
+ * NPE using WITH clause without a corresponding SELECT FROM</a>. */
+ @Test void testWithNotSelected() {
+ final String sql = "with emp2 as (select max(empno) as empno from emp where empno > 10)\n"
+ + "select * from emp where empno < emp2.empno";
+ try {
+ sql(sql).ok();
Review comment:
This test probably belongs more to `SqlValidatorTest` which has support to test for error (and validate error messages).`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] James-Jeyun-Kim edited a comment on pull request #2046: [CALCITE-4092] NPE using WITH clause without a corresponding SELECT FROM (James Kim)
Posted by GitBox <gi...@apache.org>.
James-Jeyun-Kim edited a comment on pull request #2046:
URL: https://github.com/apache/calcite/pull/2046#issuecomment-650403210
This PR catches an invalid WITH clause during the query validation step so that it doesn't lead to a NPE later on in the sqlToRelConverter stage. An invalid WITH clause query in question here is when a query defines a view using WITH but later the view is not SELECT FROM'd and referenced in the main query. In such cases, it should give a table not found error.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] James-Jeyun-Kim commented on pull request #2046: [CALCITE-4092] NPE using WITH clause without a corresponding SELECT F…
Posted by GitBox <gi...@apache.org>.
James-Jeyun-Kim commented on pull request #2046:
URL: https://github.com/apache/calcite/pull/2046#issuecomment-650403210
This PR catches an invalid WITH clause during the query validation step so that it doesn't lead to a NPE later on in the sqlToRelConverter stage.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] laurentgo merged pull request #2046: [CALCITE-4092] NPE using WITH clause without a corresponding SELECT FROM (James Kim)
Posted by GitBox <gi...@apache.org>.
laurentgo merged pull request #2046:
URL: https://github.com/apache/calcite/pull/2046
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] James-Jeyun-Kim commented on a change in pull request #2046: [CALCITE-4092] NPE using WITH clause without a corresponding SELECT FROM (James Kim)
Posted by GitBox <gi...@apache.org>.
James-Jeyun-Kim commented on a change in pull request #2046:
URL: https://github.com/apache/calcite/pull/2046#discussion_r449267835
##########
File path: core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
##########
@@ -875,6 +877,20 @@ public final Sql sql(String sql) {
sql(sql).ok();
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-4092">[CALCITE-4092]
+ * NPE using WITH clause without a corresponding SELECT FROM</a>. */
+ @Test void testWithNotSelected() {
+ final String sql = "with emp2 as (select max(empno) as empno from emp where empno > 10)\n"
+ + "select * from emp where empno < emp2.empno";
+ try {
+ sql(sql).ok();
Review comment:
You're right. I moved the test to SqlValidatorTest and made some adjustments accordingly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org