You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/11/24 12:16:52 UTC
[GitHub] [ignite] Vladsz83 opened a new pull request #9593: Ignite-15603 : Calcite. NPE on subquery returning multiple results.
Vladsz83 opened a new pull request #9593:
URL: https://github.com/apache/ignite/pull/9593
Fixed correlated pass in table_scan and index_scan in order to put exchange before correlate
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] Vladsz83 closed pull request #9593: Ignite-15603 : Calcite. NPE on subquery returning multiple results.
Posted by GitBox <gi...@apache.org>.
Vladsz83 closed pull request #9593:
URL: https://github.com/apache/ignite/pull/9593
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] alex-plekhanov commented on a change in pull request #9593: Ignite-15603 : Calcite. NPE on subquery returning multiple results.
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on a change in pull request #9593:
URL: https://github.com/apache/ignite/pull/9593#discussion_r756055705
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/LogicalScanConverterRule.java
##########
@@ -81,6 +81,12 @@
.replace(collation);
Set<CorrelationId> corrIds = RexUtils.extractCorrelationIds(rel.condition());
+
+ if (!F.isEmpty(rel.projects())) {
Review comment:
Why only for index scan? Table scan should be fixed too.
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessorTest.java
##########
@@ -1148,6 +1150,22 @@ public void testSelectWithOrdering() throws IgniteInterruptedCheckedException {
.check();
}
+ /**
+ * Checks exchnage-before-correlate and correct multy-value subquery error.
+ */
+ @Test
+ public void testSubqueryMultyValueError() throws IgniteInterruptedCheckedException {
+ sql("create table test_tbl(v INTEGER)", true);
+
+ sql("INSERT INTO test_tbl VALUES (1), (2), (3), (4), (5)", true);
+
+ Throwable err = GridTestUtils.assertThrows(log,
+ () -> sql("SELECT t0.v, (SELECT t0.v + t1.v FROM test_tbl t1) AS j FROM test_tbl t0"),
+ IgniteSQLException.class, null);
+
+ assertTrue(X.hasCause(err, "Subquery returned more than 1 value", IllegalArgumentException.class));
Review comment:
Can we write a query that is not throwing an exception after a fix, but throwing an exception without fix? For example by reducing the number of rows in test_tbl? If not, perhaps we should write planner test.
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessorTest.java
##########
@@ -1148,6 +1150,22 @@ public void testSelectWithOrdering() throws IgniteInterruptedCheckedException {
.check();
}
+ /**
+ * Checks exchnage-before-correlate and correct multy-value subquery error.
+ */
+ @Test
+ public void testSubqueryMultyValueError() throws IgniteInterruptedCheckedException {
Review comment:
The test name doesn't match javadoc.
Actually, we should check correlated queries with correlates in projects (without correlates in filters), perhaps we should name the test (and fix the javadoc) according to this check.
Also, fix typos (`multy` -> `multi`, `exchnage` -> `exchange`)
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] Vladsz83 commented on a change in pull request #9593: Ignite-15603 : Calcite. NPE on subquery returning multiple results.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #9593:
URL: https://github.com/apache/ignite/pull/9593#discussion_r756301975
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessorTest.java
##########
@@ -1148,6 +1150,22 @@ public void testSelectWithOrdering() throws IgniteInterruptedCheckedException {
.check();
}
+ /**
+ * Checks exchnage-before-correlate and correct multy-value subquery error.
+ */
+ @Test
+ public void testSubqueryMultyValueError() throws IgniteInterruptedCheckedException {
Review comment:
Spelling fixed. Test renamed.
Filters? No filter is supposed. The best approach, I believe, would be the tree analysis. Besides, the tree is one of major goals to evolve and optimize. Probably, more effective solution here is to watch NPE/exception not to appear.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] Vladsz83 commented on a change in pull request #9593: Ignite-15603 : Calcite. NPE on subquery returning multiple results.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #9593:
URL: https://github.com/apache/ignite/pull/9593#discussion_r756355388
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessorTest.java
##########
@@ -1148,6 +1150,18 @@ public void testSelectWithOrdering() throws IgniteInterruptedCheckedException {
.check();
}
+ /**
+ * Checks correlates are assigned before access.
+ */
+ @Test
+ public void testCorrelatesAssignedBeforeAccess() throws IgniteInterruptedCheckedException {
+ sql("create table test_tbl(v INTEGER)", true);
+
+ sql("INSERT INTO test_tbl VALUES (1)", true);
+
+ sql("SELECT t0.v, (SELECT t0.v + t1.v FROM test_tbl t1) AS j FROM test_tbl t0");
Review comment:
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] Vladsz83 commented on a change in pull request #9593: Ignite-15603 : Calcite. NPE on subquery returning multiple results.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #9593:
URL: https://github.com/apache/ignite/pull/9593#discussion_r756257606
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/LogicalScanConverterRule.java
##########
@@ -81,6 +81,12 @@
.replace(collation);
Set<CorrelationId> corrIds = RexUtils.extractCorrelationIds(rel.condition());
+
+ if (!F.isEmpty(rel.projects())) {
Review comment:
Yep. IDE's Misscommit. Fixed.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] alex-plekhanov commented on a change in pull request #9593: Ignite-15603 : Calcite. NPE on subquery returning multiple results.
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on a change in pull request #9593:
URL: https://github.com/apache/ignite/pull/9593#discussion_r756298024
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessorTest.java
##########
@@ -1148,6 +1150,18 @@ public void testSelectWithOrdering() throws IgniteInterruptedCheckedException {
.check();
}
+ /**
+ * Checks correlates are assigned before access.
+ */
+ @Test
+ public void testCorrelatesAssignedBeforeAccess() throws IgniteInterruptedCheckedException {
+ sql("create table test_tbl(v INTEGER)", true);
+
+ sql("INSERT INTO test_tbl VALUES (1)", true);
+
+ sql("SELECT t0.v, (SELECT t0.v + t1.v FROM test_tbl t1) AS j FROM test_tbl t0");
Review comment:
Let's assert result here
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] Vladsz83 commented on a change in pull request #9593: Ignite-15603 : Calcite. NPE on subquery returning multiple results.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #9593:
URL: https://github.com/apache/ignite/pull/9593#discussion_r756301975
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessorTest.java
##########
@@ -1148,6 +1150,22 @@ public void testSelectWithOrdering() throws IgniteInterruptedCheckedException {
.check();
}
+ /**
+ * Checks exchnage-before-correlate and correct multy-value subquery error.
+ */
+ @Test
+ public void testSubqueryMultyValueError() throws IgniteInterruptedCheckedException {
Review comment:
Spelling fixed. Test renamed.
Filters? No filter is supposed. The best approach, I believe, would be the tree analysis. Besides, the tree is one of major goals to evolve and optimize. Probably, more effective solution here is to watch NPE/exception not to appearing.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org