[CORE] Remove pushed down filter in FilterExecTransformer #11076
Conversation
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
1 similar comment
|
Run Gluten Clickhouse CI on x86 |
| } else { | ||
| val remainingFilters = | ||
| FilterHandler.getRemainingFilters(scanFilters, splitConjunctivePredicates(filter.cond)) | ||
| val newCond = remainingFilters.reduceLeftOption(And).orNull |
There was a problem hiding this comment.
Perhaps we could remove the filterNode if newCond is null. If guys agree, I can include this change in this PR CC @zhztheplayer
There was a problem hiding this comment.
Perhaps we could remove the filterNode if newCond is null.
I previously attempted something similar, but ended up facing some attribute mismatch errors from Catalyst.
I didn't look into that issue too much, so feel free to try from your end if you're interested.
There was a problem hiding this comment.
facing some attribute mismatch errors from Catalyst.
@zhztheplayer I checked the logic of FilterExec and confirmed that its output indeed changes—it equals the child's output plus some NOT NULL attributes added by the condition. Therefore, I made the following modifications:
- Extracted a helper method
buildNewOutput(output, conds)from FilterExec. - If pushedFilter nonEmpty:
- Set ScanNode's newOutput = buildNewOutput(output, pushedFilter).
- Set FilterNode's newCond = cond -- pushedFilter. (If newCond == null, remove the filter node.)
I believe this better reflects Node's correct behavior, although perhaps all golden files should be updated.
|
Do |
The logic should be the same as before, because we were already using getRemainingCondition to generate the Filter Transform previously. I just made the execution plan more accurately reflect its actual behavior. For example, users might notice there's a filter node, but in reality, it doesn't do anything. |
Got it, thank you for your explanation. |
Zouxxyy
force-pushed
the
dev/remove-filter
branch
from
93b704b to
017aefc
Compare
|
Run Gluten Clickhouse CI on x86 |
Zouxxyy
force-pushed
the
dev/remove-filter
branch
from
017aefc to
c0e1190
Compare
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
3 participants
What changes are proposed in this pull request?
Before we use
getRemainingConditionto caculate the real fitler condiction in FilterExecTransformer, in this pr, just remove pushed down filter inFilterExecTransformerto reflect the node's actual behavior.How was this patch tested?