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/02/23 10:08:05 UTC
[GitHub] [calcite] IceMimosa opened a new pull request #2729: [CALCITE-5019] Avoid multiple scans when table is ProjectableFilterableTable
IceMimosa opened a new pull request #2729:
URL: https://github.com/apache/calcite/pull/2729
```sql
select a from ProjectableFilterableTable where b = 'xx'
```
The ProjectableFilterableTable will scan twice times when projections and filters are different columns.
--
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] IceMimosa commented on pull request #2729: [CALCITE-5019] Avoid multiple scans when table is ProjectableFilterableTable
Posted by GitBox <gi...@apache.org>.
IceMimosa commented on pull request #2729:
URL: https://github.com/apache/calcite/pull/2729#issuecomment-1058156109
@rubenada Done, thanks for reviewing
--
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] rubenada commented on a change in pull request #2729: [CALCITE-5019] Avoid multiple scans when table is ProjectableFilterableTable
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2729:
URL: https://github.com/apache/calcite/pull/2729#discussion_r812957927
##########
File path: core/src/test/java/org/apache/calcite/test/ScannableTableTest.java
##########
@@ -155,7 +155,7 @@
"j=Paul");
// Only 2 rows came out of the table. If the value is 4, it means that the
// planner did not pass the filter down.
- assertThat(buf.toString(), is("returnCount=2, filter=<0, 4>, projects=[1]"));
+ assertThat(buf.toString(), is("returnCount=2, filter=<0, 4>, projects=[1, 0]"));
Review comment:
Is it expected to have this side effect (and the one in line 191)?
--
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] IceMimosa commented on a change in pull request #2729: [CALCITE-5019] Avoid multiple scans when table is ProjectableFilterableTable
Posted by GitBox <gi...@apache.org>.
IceMimosa commented on a change in pull request #2729:
URL: https://github.com/apache/calcite/pull/2729#discussion_r812742187
##########
File path: core/src/main/java/org/apache/calcite/interpreter/TableScanNode.java
##########
@@ -227,6 +225,8 @@ private static TableScanNode createProjectableFilterable(Compiler compiler,
continue;
Review comment:
If `changeCount > 0`, the scan will trigger again.
--
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] IceMimosa commented on a change in pull request #2729: [CALCITE-5019] Avoid multiple scans when table is ProjectableFilterableTable
Posted by GitBox <gi...@apache.org>.
IceMimosa commented on a change in pull request #2729:
URL: https://github.com/apache/calcite/pull/2729#discussion_r813527856
##########
File path: core/src/test/java/org/apache/calcite/test/ScannableTableTest.java
##########
@@ -155,7 +155,7 @@
"j=Paul");
// Only 2 rows came out of the table. If the value is 4, it means that the
// planner did not pass the filter down.
- assertThat(buf.toString(), is("returnCount=2, filter=<0, 4>, projects=[1]"));
+ assertThat(buf.toString(), is("returnCount=2, filter=<0, 4>, projects=[1, 0]"));
Review comment:
@JiajunBernoulli This test wants to pushdown filter to table scan, but it's not suitable here, it should use RelRule to do this in optimize phase. `TableScan's projects` and `ProjectableFilterableTable's projects` are not the same, The test here is `select j where i`, in physical execution, the ProjectableFilterableTable's projects must contain `j` and `i` if you don't change the AST.
--
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] rubenada merged pull request #2729: [CALCITE-5019] Avoid multiple scans when table is ProjectableFilterableTable
Posted by GitBox <gi...@apache.org>.
rubenada merged pull request #2729:
URL: https://github.com/apache/calcite/pull/2729
--
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] JiajunBernoulli commented on a change in pull request #2729: [CALCITE-5019] Avoid multiple scans when table is ProjectableFilterableTable
Posted by GitBox <gi...@apache.org>.
JiajunBernoulli commented on a change in pull request #2729:
URL: https://github.com/apache/calcite/pull/2729#discussion_r813519646
##########
File path: core/src/test/java/org/apache/calcite/test/ScannableTableTest.java
##########
@@ -155,7 +155,7 @@
"j=Paul");
// Only 2 rows came out of the table. If the value is 4, it means that the
// planner did not pass the filter down.
- assertThat(buf.toString(), is("returnCount=2, filter=<0, 4>, projects=[1]"));
+ assertThat(buf.toString(), is("returnCount=2, filter=<0, 4>, projects=[1, 0]"));
Review comment:
@IceMimosa Do you know what projetcs means?
--
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] IceMimosa commented on a change in pull request #2729: [CALCITE-5019] Avoid multiple scans when table is ProjectableFilterableTable
Posted by GitBox <gi...@apache.org>.
IceMimosa commented on a change in pull request #2729:
URL: https://github.com/apache/calcite/pull/2729#discussion_r812965000
##########
File path: core/src/test/java/org/apache/calcite/test/ScannableTableTest.java
##########
@@ -155,7 +155,7 @@
"j=Paul");
// Only 2 rows came out of the table. If the value is 4, it means that the
// planner did not pass the filter down.
- assertThat(buf.toString(), is("returnCount=2, filter=<0, 4>, projects=[1]"));
+ assertThat(buf.toString(), is("returnCount=2, filter=<0, 4>, projects=[1, 0]"));
Review comment:
@rubenada Yes, it's expected. The test ignored this case.
--
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] rubenada commented on pull request #2729: [CALCITE-5019] Avoid multiple scans when table is ProjectableFilterableTable
Posted by GitBox <gi...@apache.org>.
rubenada commented on pull request #2729:
URL: https://github.com/apache/calcite/pull/2729#issuecomment-1058080865
@IceMimosa the change LGTM, could you please squash commits into a single one?
May I suggest as commit message something along the lines "[CALCITE-5019] Avoid multiple scans when table is ProjectableFilterableTable and projections and filters act on different columns"
--
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