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/04/22 03:26:09 UTC
[GitHub] [calcite] xy2953396112 opened a new pull request #1936: [CALCITE-3928] trim unused fields before materialized view matching (…
xy2953396112 opened a new pull request #1936:
URL: https://github.com/apache/calcite/pull/1936
…xzh)
JIRA: https://issues.apache.org/jira/browse/CALCITE-3928
----------------------------------------------------------------
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] xy2953396112 commented on pull request #1936: [CALCITE-3928] Trim unused fields before materialized view matching (…
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #1936:
URL: https://github.com/apache/calcite/pull/1936#issuecomment-622329835
> ```
> Expected: "LogicalProject(deptno=[CAST($0):TINYINT], count_sal=[$1])\r\n LogicalTableScan(table=[[mv0]])\r\n"
> but: was "LogicalProject(deptno=[CAST($0):TINYINT], count_sal=[$1])\n LogicalTableScan(table=[[mv0]])\n"
> ```
>
> Can you fix the test failure?
> ```
> Expected: "LogicalProject(deptno=[CAST($0):TINYINT], count_sal=[$1])\r\n LogicalTableScan(table=[[mv0]])\r\n"
> but: was "LogicalProject(deptno=[CAST($0):TINYINT], count_sal=[$1])\n LogicalTableScan(table=[[mv0]])\n"
> ```
>
> Can you fix the test failure?
Thanks, i have fixed the test failure.
----------------------------------------------------------------
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] jinxing64 commented on a change in pull request #1936: [CALCITE-3928] Trim unused fields before materialized view matching (…
Posted by GitBox <gi...@apache.org>.
jinxing64 commented on a change in pull request #1936:
URL: https://github.com/apache/calcite/pull/1936#discussion_r412644512
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptMaterializations.java
##########
@@ -192,6 +195,8 @@
// Push filters to the bottom, and combine projects on top.
RelNode target = materialization.queryRel;
+ // try to trim unused field in relational expressions.
+ root = trimUnusedfields(root);
Review comment:
Shall we also do trimming on `target` for canonicalization ?
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptMaterializations.java
##########
@@ -223,6 +228,19 @@
return new SubstitutionVisitor(target, root).go(materialization.tableRel);
}
+ /**
+ * trim unused fields in relational expressions.
+ */
+ private static RelNode trimUnusedfields(RelNode relNode) {
+ final List<RelOptTable> relOptTables = RelOptUtil.findAllTables(relNode);
Review comment:
What if there's no `TableScan` in the tree of `relNode` ?
----------------------------------------------------------------
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] hsyuan commented on pull request #1936: [CALCITE-3928] Trim unused fields before materialized view matching (…
Posted by GitBox <gi...@apache.org>.
hsyuan commented on pull request #1936:
URL: https://github.com/apache/calcite/pull/1936#issuecomment-622215002
```
Expected: "LogicalProject(deptno=[CAST($0):TINYINT], count_sal=[$1])\r\n LogicalTableScan(table=[[mv0]])\r\n"
but: was "LogicalProject(deptno=[CAST($0):TINYINT], count_sal=[$1])\n LogicalTableScan(table=[[mv0]])\n"
```
Can you fix the test failure?
----------------------------------------------------------------
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] xy2953396112 commented on a change in pull request #1936: [CALCITE-3928] Trim unused fields before materialized view matching (…
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #1936:
URL: https://github.com/apache/calcite/pull/1936#discussion_r412752519
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptMaterializations.java
##########
@@ -192,6 +195,8 @@
// Push filters to the bottom, and combine projects on top.
RelNode target = materialization.queryRel;
+ // try to trim unused field in relational expressions.
+ root = trimUnusedfields(root);
Review comment:
update it.
----------------------------------------------------------------
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] xy2953396112 commented on a change in pull request #1936: [CALCITE-3928] Trim unused fields before materialized view matching (…
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on a change in pull request #1936:
URL: https://github.com/apache/calcite/pull/1936#discussion_r412745003
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptMaterializations.java
##########
@@ -223,6 +228,19 @@
return new SubstitutionVisitor(target, root).go(materialization.tableRel);
}
+ /**
+ * trim unused fields in relational expressions.
+ */
+ private static RelNode trimUnusedfields(RelNode relNode) {
+ final List<RelOptTable> relOptTables = RelOptUtil.findAllTables(relNode);
Review comment:
yeah, you are right, if there's no TableScan, it will return itself.
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptMaterializations.java
##########
@@ -223,6 +228,19 @@
return new SubstitutionVisitor(target, root).go(materialization.tableRel);
}
+ /**
+ * trim unused fields in relational expressions.
Review comment:
Thanks , will update it.
----------------------------------------------------------------
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] xy2953396112 commented on pull request #1936: [CALCITE-3928] Trim unused fields before materialized view matching (…
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #1936:
URL: https://github.com/apache/calcite/pull/1936#issuecomment-622329984
> ```
> Expected: "LogicalProject(deptno=[CAST($0):TINYINT], count_sal=[$1])\r\n LogicalTableScan(table=[[mv0]])\r\n"
> but: was "LogicalProject(deptno=[CAST($0):TINYINT], count_sal=[$1])\n LogicalTableScan(table=[[mv0]])\n"
> ```
>
> Can you fix the test failure?
Thanks, i have fixed the test failure.
----------------------------------------------------------------
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] jinxing64 commented on a change in pull request #1936: [CALCITE-3928] Trim unused fields before materialized view matching (…
Posted by GitBox <gi...@apache.org>.
jinxing64 commented on a change in pull request #1936:
URL: https://github.com/apache/calcite/pull/1936#discussion_r412665447
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptMaterializations.java
##########
@@ -223,6 +228,19 @@
return new SubstitutionVisitor(target, root).go(materialization.tableRel);
}
+ /**
+ * trim unused fields in relational expressions.
Review comment:
Upper case -- "Trim ..."
----------------------------------------------------------------
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] jinxing64 commented on a change in pull request #1936: [CALCITE-3928] Trim unused fields before materialized view matching (…
Posted by GitBox <gi...@apache.org>.
jinxing64 commented on a change in pull request #1936:
URL: https://github.com/apache/calcite/pull/1936#discussion_r412899044
##########
File path: core/src/main/java/org/apache/calcite/plan/RelOptMaterializations.java
##########
@@ -223,6 +228,19 @@
return new SubstitutionVisitor(target, root).go(materialization.tableRel);
}
+ /**
+ * trim unused fields in relational expressions.
+ */
+ private static RelNode trimUnusedfields(RelNode relNode) {
+ final List<RelOptTable> relOptTables = RelOptUtil.findAllTables(relNode);
Review comment:
When no `TableScan`, shall we use `null` to construct a `RelBuilder` and do the trimming ?
----------------------------------------------------------------
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] xy2953396112 removed a comment on pull request #1936: [CALCITE-3928] Trim unused fields before materialized view matching (…
Posted by GitBox <gi...@apache.org>.
xy2953396112 removed a comment on pull request #1936:
URL: https://github.com/apache/calcite/pull/1936#issuecomment-622329835
> ```
> Expected: "LogicalProject(deptno=[CAST($0):TINYINT], count_sal=[$1])\r\n LogicalTableScan(table=[[mv0]])\r\n"
> but: was "LogicalProject(deptno=[CAST($0):TINYINT], count_sal=[$1])\n LogicalTableScan(table=[[mv0]])\n"
> ```
>
> Can you fix the test failure?
> ```
> Expected: "LogicalProject(deptno=[CAST($0):TINYINT], count_sal=[$1])\r\n LogicalTableScan(table=[[mv0]])\r\n"
> but: was "LogicalProject(deptno=[CAST($0):TINYINT], count_sal=[$1])\n LogicalTableScan(table=[[mv0]])\n"
> ```
>
> Can you fix the test failure?
Thanks, i have fixed the test failure.
----------------------------------------------------------------
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