[CORE][VL] Fix BatchScanExec filter pushdown logic #4132
Conversation
|
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/oap-project/gluten/issues Then could you also rename commit message and pull request title in the following format? See also: |
|
Run Gluten Clickhouse CI |
liujiayi771
force-pushed
the
batchscan-filter-pushdown
branch
from
28d4438
to
b8f0173
Compare
|
Run Gluten Clickhouse CI |
|
Before(Filter has been executed): |
|
@liujiayi771 just pushed the Velox rebase for 12-20, you may need to do a rebase -yuan |
liujiayi771
force-pushed
the
batchscan-filter-pushdown
branch
from
b8f0173
to
5ee37fc
Compare
|
Run Gluten Clickhouse CI |
1 similar comment
|
Run Gluten Clickhouse CI |
|
@rui-mo Could you help review? |
liujiayi771
force-pushed
the
batchscan-filter-pushdown
branch
from
bed0275
to
40eb698
Compare
|
Run Gluten Clickhouse CI |
| @@ -392,7 +393,45 @@ object FilterHandler extends PredicateHelper { | |||
| fileSourceScan, | |||
| reuseSubquery, | |||
| extraFilters = leftFilters) | |||
| case batchScan: BatchScanExec => | |||
| val leftFilters = batchScan.scan match { | |||
There was a problem hiding this comment.
I think "remaining" is more formal and less misleading than "left". Could you revise the naming here and at some other places? cc @rui-mo
|
Run Gluten Clickhouse CI |
1 similar comment
|
Run Gluten Clickhouse CI |
| case other => | ||
| throw new UnsupportedOperationException(s"${other.getClass.toString} is not supported.") | ||
| } | ||
| } | ||
|
|
||
| object ScanHandler { | ||
| def getBatchScanTransformer( |
There was a problem hiding this comment.
Can we move this method into BatchScanExecTransformer.scala? Thanks!
There was a problem hiding this comment.
I think it is better to put it in ScanTransformerFactory, I will organize the code and only provide a createBatchScanTransformer method in the ScanTransformerFactory.
liujiayi771
force-pushed
the
batchscan-filter-pushdown
branch
from
adb9a78
to
a63185d
Compare
|
Run Gluten Clickhouse CI |
liujiayi771
force-pushed
the
batchscan-filter-pushdown
branch
from
a63185d
to
a1fe34c
Compare
|
Run Gluten Clickhouse CI |
liujiayi771
force-pushed
the
batchscan-filter-pushdown
branch
from
a1fe34c
to
04eef50
Compare
|
Run Gluten Clickhouse CI |
|
===== Performance report for TPCH SF2000 with Velox backend, for reference only ====
|
What changes were proposed in this pull request?
Before #3843 the filters pushdown was incorporated into the
runtimeFiltersofBatchScanExecTransformer, which was ineffective. Hence, #3843 removed this logic. Now, the logic to pushdown filters inBatchScanExecTransformerhas been restored and correctly set into a newpushdownFiltersvariable. This will be accessed when thefilterExprsmethod is called, and afterwards, it can be pushdown into Velox'sTableScan.How was this patch tested?
Add new
VeloxScanSuitetest case to test subquery filter pushdown in tpch q22.