You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "askalt (via GitHub)" <gi...@apache.org> on 2026/01/26 08:06:10 UTC
[PR] Make scan filter push-down idempotent [datafusion]
askalt opened a new pull request, #20003:
URL: https://github.com/apache/datafusion/pull/20003
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
- Closes https://github.com/apache/datafusion/issues/19929
## Rationale for this change
Non-idempotent rule logic leads to the artifacts described in the issue above if several the rule is applied several times.
## What changes are included in this PR?
- Pass filters pushed earlier to `supports_filter_pushdown(...)` call.
- Extend a scan projection if some pushed filters become unsupported.
## Are these changes tested?
There are unit tests.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make scan filter push-down idempotent [datafusion]
Posted by "askalt (via GitHub)" <gi...@apache.org>.
askalt commented on PR #20003:
URL: https://github.com/apache/datafusion/pull/20003#issuecomment-3798479482
Will fix CI soon.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make scan filter push-down idempotent [datafusion]
Posted by "askalt (via GitHub)" <gi...@apache.org>.
askalt commented on code in PR #20003:
URL: https://github.com/apache/datafusion/pull/20003#discussion_r2735141165
##########
datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part:
##########
@@ -64,7 +64,7 @@ logical_plan
06)----------Inner Join: Filter: CAST(customer.c_acctbal AS Decimal128(19, 6)) > __scalar_sq_2.avg(customer.c_acctbal)
07)------------Projection: customer.c_phone, customer.c_acctbal
08)--------------LeftAnti Join: customer.c_custkey = __correlated_sq_1.o_custkey
-09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")])
+09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")]) AND Boolean(true)
Review Comment:
It is an incorrect behavior of filter-push-down to forgot this `true` in FilterExec (just a coincidence that it is trivial filter).
Currently, in `main` branch we have the following:
https://github.com/apache/datafusion/blob/16368983bdefca40ff0f7fd968ed2a0c6aa21452/datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part#L67-L68
We can notice that here `, Boolean(true)` in the `partial_filters` section. As this filter is not supported fully, it should be re-checked in filter node, so if it would non-trivial filter (push-down does not know if it is) it would lead to incorrect selection result.
##########
datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part:
##########
@@ -64,7 +64,7 @@ logical_plan
06)----------Inner Join: Filter: CAST(customer.c_acctbal AS Decimal128(19, 6)) > __scalar_sq_2.avg(customer.c_acctbal)
07)------------Projection: customer.c_phone, customer.c_acctbal
08)--------------LeftAnti Join: customer.c_custkey = __correlated_sq_1.o_custkey
-09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")])
+09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")]) AND Boolean(true)
Review Comment:
It is an incorrect behavior of filter-push-down to omit this `true` in FilterExec (just a coincidence that it is trivial filter).
Currently, in `main` branch we have the following:
https://github.com/apache/datafusion/blob/16368983bdefca40ff0f7fd968ed2a0c6aa21452/datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part#L67-L68
We can notice that here `, Boolean(true)` in the `partial_filters` section. As this filter is not supported fully, it should be re-checked in filter node, so if it would non-trivial filter (push-down does not know if it is) it would lead to incorrect selection result.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make scan filter push-down idempotent [datafusion]
Posted by "askalt (via GitHub)" <gi...@apache.org>.
askalt commented on code in PR #20003:
URL: https://github.com/apache/datafusion/pull/20003#discussion_r2735141165
##########
datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part:
##########
@@ -64,7 +64,7 @@ logical_plan
06)----------Inner Join: Filter: CAST(customer.c_acctbal AS Decimal128(19, 6)) > __scalar_sq_2.avg(customer.c_acctbal)
07)------------Projection: customer.c_phone, customer.c_acctbal
08)--------------LeftAnti Join: customer.c_custkey = __correlated_sq_1.o_custkey
-09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")])
+09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")]) AND Boolean(true)
Review Comment:
It is an incorrect behavior of filter-push-down to omit this `true` in `Filter` (just a coincidence that it is trivial filter).
Currently, in `main` branch we have the following:
https://github.com/apache/datafusion/blob/16368983bdefca40ff0f7fd968ed2a0c6aa21452/datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part#L67-L68
We can notice that here `, Boolean(true)` in the `partial_filters` section. As this filter is not supported fully, it should be re-checked in filter node, so if it would non-trivial filter (push-down does not know if it is) it would lead to incorrect selection result.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make scan filter push-down idempotent [datafusion]
Posted by "askalt (via GitHub)" <gi...@apache.org>.
askalt commented on code in PR #20003:
URL: https://github.com/apache/datafusion/pull/20003#discussion_r2735141165
##########
datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part:
##########
@@ -64,7 +64,7 @@ logical_plan
06)----------Inner Join: Filter: CAST(customer.c_acctbal AS Decimal128(19, 6)) > __scalar_sq_2.avg(customer.c_acctbal)
07)------------Projection: customer.c_phone, customer.c_acctbal
08)--------------LeftAnti Join: customer.c_custkey = __correlated_sq_1.o_custkey
-09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")])
+09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")]) AND Boolean(true)
Review Comment:
It is an incorrect behavior to forgot this `true` in FilterExec (just a coincidence that it is trivial filter).
Currently, in `main` branch we have the following:
https://github.com/apache/datafusion/blob/16368983bdefca40ff0f7fd968ed2a0c6aa21452/datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part#L67-L68
We can notice that here `, Boolean(true)` in the `partial_filters` section. As this filter is not supported fully, it should be re-checked in filter node, so if it would non-trivial filter (push-down does not know if it is) it would lead to incorrect selection result.
##########
datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part:
##########
@@ -64,7 +64,7 @@ logical_plan
06)----------Inner Join: Filter: CAST(customer.c_acctbal AS Decimal128(19, 6)) > __scalar_sq_2.avg(customer.c_acctbal)
07)------------Projection: customer.c_phone, customer.c_acctbal
08)--------------LeftAnti Join: customer.c_custkey = __correlated_sq_1.o_custkey
-09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")])
+09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")]) AND Boolean(true)
Review Comment:
It is an incorrect behavior to forgot this `true` in FilterExec (just a coincidence that it is a trivial filter).
Currently, in `main` branch we have the following:
https://github.com/apache/datafusion/blob/16368983bdefca40ff0f7fd968ed2a0c6aa21452/datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part#L67-L68
We can notice that here `, Boolean(true)` in the `partial_filters` section. As this filter is not supported fully, it should be re-checked in filter node, so if it would non-trivial filter (push-down does not know if it is) it would lead to incorrect selection result.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make scan filter push-down idempotent [datafusion]
Posted by "askalt (via GitHub)" <gi...@apache.org>.
askalt commented on code in PR #20003:
URL: https://github.com/apache/datafusion/pull/20003#discussion_r2735141165
##########
datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part:
##########
@@ -64,7 +64,7 @@ logical_plan
06)----------Inner Join: Filter: CAST(customer.c_acctbal AS Decimal128(19, 6)) > __scalar_sq_2.avg(customer.c_acctbal)
07)------------Projection: customer.c_phone, customer.c_acctbal
08)--------------LeftAnti Join: customer.c_custkey = __correlated_sq_1.o_custkey
-09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")])
+09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")]) AND Boolean(true)
Review Comment:
It is an incorrect behavior to forgot this `true` in FilterExec (just a coincidence that it is trivial filter).
Currently, in `main` branch we have the following:
https://github.com/apache/datafusion/blob/16368983bdefca40ff0f7fd968ed2a0c6aa21452/datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part#L67-L68
We can notice that here `, Boolean(true)` in the `partial_filters` section. As this filter is not supported fully, it should be re-checked in filter node, so if it would non-trivial filter (push-down does not know if it is) it would lead to incorrect selection result.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make scan filter push-down idempotent [datafusion]
Posted by "askalt (via GitHub)" <gi...@apache.org>.
askalt commented on code in PR #20003:
URL: https://github.com/apache/datafusion/pull/20003#discussion_r2731044365
##########
datafusion/optimizer/src/push_down_filter.rs:
##########
@@ -1154,34 +1160,109 @@ impl OptimizerRule for PushDownFilter {
.map(|(pred, _)| pred);
// Add new scan filters
- let new_scan_filters: Vec<Expr> = scan
- .filters
- .iter()
- .chain(new_scan_filters)
- .unique()
- .cloned()
- .collect();
+ let new_scan_filters: Vec<Expr> =
+ new_scan_filters.unique().cloned().collect();
+
+ let source_schema = scan.source.schema();
+ let mut additional_projection = HashSet::new();
// Compose predicates to be of `Unsupported` or `Inexact` pushdown type, and also include volatile filters
let new_predicate: Vec<Expr> = zip
- .filter(|(_, res)| res != &TableProviderFilterPushDown::Exact)
+ .filter(|(expr, res)| {
+ if *res == TableProviderFilterPushDown::Exact {
+ return false;
+ }
+ // For each not exactly supported filter we must ensure that all columns are projected,
+ // so we collect all columns which are not currently projected.
+ expr.apply(|expr| {
+ if let Expr::Column(column) = expr
+ && let Ok(idx) = source_schema.index_of(column.name())
+ && scan
+ .projection
+ .as_ref()
+ .is_some_and(|p| !p.contains(&idx))
+ {
+ additional_projection.insert(idx);
+ }
+ Ok(TreeNodeRecursion::Continue)
+ })
+ .unwrap();
+ true
+ })
.map(|(pred, _)| pred)
.chain(volatile_filters)
.cloned()
.collect();
- let new_scan = LogicalPlan::TableScan(TableScan {
- filters: new_scan_filters,
- ..scan
- });
-
- Transformed::yes(new_scan).transform_data(|new_scan| {
- if let Some(predicate) = conjunction(new_predicate) {
- make_filter(predicate, Arc::new(new_scan)).map(Transformed::yes)
+ // Wraps with a filter if some filters are not supported exactly.
+ let filtered = move |plan| {
+ if let Some(new_predicate) = conjunction(new_predicate) {
+ Filter::try_new(new_predicate, Arc::new(plan))
+ .map(LogicalPlan::Filter)
} else {
- Ok(Transformed::no(new_scan))
+ Ok(plan)
}
- })
+ };
+
+ if additional_projection.is_empty() {
+ // No additional projection is required.
+ let new_scan = LogicalPlan::TableScan(TableScan {
+ filters: new_scan_filters,
+ ..scan
+ });
+ return filtered(new_scan).map(Transformed::yes);
+ }
+
+ let scan_table_name = &scan.table_name;
+ let new_scan = filtered(
+ LogicalPlanBuilder::scan_with_filters_fetch(
+ scan_table_name.clone(),
+ Arc::clone(&scan.source),
+ scan.projection.clone().map(|mut projection| {
+ // Extend a projection.
+ projection.extend(additional_projection);
Review Comment:
It seems for me that the change only touches correctness of the rule (not performance), here we must extend projection (because filter is explicitly not supported by the provider).
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make scan filter push-down idempotent [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #20003: Make scan filter push-down idempotent
URL: https://github.com/apache/datafusion/pull/20003
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make scan filter push-down idempotent [datafusion]
Posted by "xudong963 (via GitHub)" <gi...@apache.org>.
xudong963 commented on code in PR #20003:
URL: https://github.com/apache/datafusion/pull/20003#discussion_r2730956061
##########
datafusion/optimizer/src/push_down_filter.rs:
##########
@@ -1154,34 +1160,109 @@ impl OptimizerRule for PushDownFilter {
.map(|(pred, _)| pred);
// Add new scan filters
- let new_scan_filters: Vec<Expr> = scan
- .filters
- .iter()
- .chain(new_scan_filters)
- .unique()
- .cloned()
- .collect();
+ let new_scan_filters: Vec<Expr> =
+ new_scan_filters.unique().cloned().collect();
+
+ let source_schema = scan.source.schema();
+ let mut additional_projection = HashSet::new();
// Compose predicates to be of `Unsupported` or `Inexact` pushdown type, and also include volatile filters
let new_predicate: Vec<Expr> = zip
- .filter(|(_, res)| res != &TableProviderFilterPushDown::Exact)
+ .filter(|(expr, res)| {
+ if *res == TableProviderFilterPushDown::Exact {
+ return false;
+ }
+ // For each not exactly supported filter we must ensure that all columns are projected,
+ // so we collect all columns which are not currently projected.
+ expr.apply(|expr| {
+ if let Expr::Column(column) = expr
+ && let Ok(idx) = source_schema.index_of(column.name())
+ && scan
+ .projection
+ .as_ref()
+ .is_some_and(|p| !p.contains(&idx))
+ {
+ additional_projection.insert(idx);
+ }
+ Ok(TreeNodeRecursion::Continue)
+ })
+ .unwrap();
+ true
+ })
.map(|(pred, _)| pred)
.chain(volatile_filters)
.cloned()
.collect();
- let new_scan = LogicalPlan::TableScan(TableScan {
- filters: new_scan_filters,
- ..scan
- });
-
- Transformed::yes(new_scan).transform_data(|new_scan| {
- if let Some(predicate) = conjunction(new_predicate) {
- make_filter(predicate, Arc::new(new_scan)).map(Transformed::yes)
+ // Wraps with a filter if some filters are not supported exactly.
+ let filtered = move |plan| {
+ if let Some(new_predicate) = conjunction(new_predicate) {
+ Filter::try_new(new_predicate, Arc::new(plan))
+ .map(LogicalPlan::Filter)
} else {
- Ok(Transformed::no(new_scan))
+ Ok(plan)
}
- })
+ };
+
+ if additional_projection.is_empty() {
+ // No additional projection is required.
+ let new_scan = LogicalPlan::TableScan(TableScan {
+ filters: new_scan_filters,
+ ..scan
+ });
+ return filtered(new_scan).map(Transformed::yes);
+ }
+
+ let scan_table_name = &scan.table_name;
+ let new_scan = filtered(
+ LogicalPlanBuilder::scan_with_filters_fetch(
+ scan_table_name.clone(),
+ Arc::clone(&scan.source),
+ scan.projection.clone().map(|mut projection| {
+ // Extend a projection.
+ projection.extend(additional_projection);
Review Comment:
There should be a tradeoff between fetching an extra column from storage vs the selectivity of the related filter
##########
datafusion/optimizer/src/push_down_filter.rs:
##########
@@ -1154,34 +1160,109 @@ impl OptimizerRule for PushDownFilter {
.map(|(pred, _)| pred);
// Add new scan filters
- let new_scan_filters: Vec<Expr> = scan
- .filters
- .iter()
- .chain(new_scan_filters)
- .unique()
- .cloned()
- .collect();
+ let new_scan_filters: Vec<Expr> =
+ new_scan_filters.unique().cloned().collect();
+
+ let source_schema = scan.source.schema();
+ let mut additional_projection = HashSet::new();
// Compose predicates to be of `Unsupported` or `Inexact` pushdown type, and also include volatile filters
let new_predicate: Vec<Expr> = zip
- .filter(|(_, res)| res != &TableProviderFilterPushDown::Exact)
+ .filter(|(expr, res)| {
+ if *res == TableProviderFilterPushDown::Exact {
+ return false;
+ }
+ // For each not exactly supported filter we must ensure that all columns are projected,
+ // so we collect all columns which are not currently projected.
+ expr.apply(|expr| {
+ if let Expr::Column(column) = expr
+ && let Ok(idx) = source_schema.index_of(column.name())
+ && scan
+ .projection
+ .as_ref()
+ .is_some_and(|p| !p.contains(&idx))
+ {
+ additional_projection.insert(idx);
+ }
+ Ok(TreeNodeRecursion::Continue)
+ })
+ .unwrap();
+ true
+ })
.map(|(pred, _)| pred)
.chain(volatile_filters)
.cloned()
.collect();
- let new_scan = LogicalPlan::TableScan(TableScan {
- filters: new_scan_filters,
- ..scan
- });
-
- Transformed::yes(new_scan).transform_data(|new_scan| {
- if let Some(predicate) = conjunction(new_predicate) {
- make_filter(predicate, Arc::new(new_scan)).map(Transformed::yes)
+ // Wraps with a filter if some filters are not supported exactly.
+ let filtered = move |plan| {
+ if let Some(new_predicate) = conjunction(new_predicate) {
+ Filter::try_new(new_predicate, Arc::new(plan))
+ .map(LogicalPlan::Filter)
} else {
- Ok(Transformed::no(new_scan))
+ Ok(plan)
}
- })
+ };
+
+ if additional_projection.is_empty() {
Review Comment:
The branch of scan is expanding, any thoughts to split the branch
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make scan filter push-down idempotent [datafusion]
Posted by "askalt (via GitHub)" <gi...@apache.org>.
askalt commented on code in PR #20003:
URL: https://github.com/apache/datafusion/pull/20003#discussion_r2735141165
##########
datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part:
##########
@@ -64,7 +64,7 @@ logical_plan
06)----------Inner Join: Filter: CAST(customer.c_acctbal AS Decimal128(19, 6)) > __scalar_sq_2.avg(customer.c_acctbal)
07)------------Projection: customer.c_phone, customer.c_acctbal
08)--------------LeftAnti Join: customer.c_custkey = __correlated_sq_1.o_custkey
-09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")])
+09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")]) AND Boolean(true)
Review Comment:
It is an incorrect behavior of filter-push-down to omit this `true` in `Filter` (just a coincidence that it is trivial filter).
Currently, in `main` branch we have the following:
https://github.com/apache/datafusion/blob/16368983bdefca40ff0f7fd968ed2a0c6aa21452/datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part#L67-L68
We can notice that here `, Boolean(true)` in the `partial_filters` section. As this filter is not supported fully, it should be re-checked in filter node, so if it would non-trivial filter (push-down does not know if is it) it would lead to incorrect selection result.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make scan filter push-down idempotent [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #20003:
URL: https://github.com/apache/datafusion/pull/20003#discussion_r2734092517
##########
datafusion/sqllogictest/test_files/tpch/plans/q22.slt.part:
##########
@@ -64,7 +64,7 @@ logical_plan
06)----------Inner Join: Filter: CAST(customer.c_acctbal AS Decimal128(19, 6)) > __scalar_sq_2.avg(customer.c_acctbal)
07)------------Projection: customer.c_phone, customer.c_acctbal
08)--------------LeftAnti Join: customer.c_custkey = __correlated_sq_1.o_custkey
-09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")])
+09)----------------Filter: substr(customer.c_phone, Int64(1), Int64(2)) IN ([Utf8View("13"), Utf8View("31"), Utf8View("23"), Utf8View("29"), Utf8View("30"), Utf8View("18"), Utf8View("17")]) AND Boolean(true)
Review Comment:
Why does this now have "AND true"? That seems unecessary 🤔
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make scan filter push-down idempotent [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20003:
URL: https://github.com/apache/datafusion/pull/20003#issuecomment-4417083088
Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org