JunRuiLee commented on code in PR #496:
URL: https://github.com/apache/paimon-rust/pull/496#discussion_r3563889062
##########
crates/integrations/datafusion/src/filter_pushdown.rs:
##########
@@ -81,22 +89,34 @@ pub(crate) fn analyze_filters(filters: &[Expr], fields:
&[DataField]) -> FilterP
#[cfg(test)]
pub(crate) fn build_pushed_predicate(filters: &[Expr], fields: &[DataField])
-> Option<Predicate> {
- analyze_filters(filters, fields).pushed_predicate
+ analyze_filters(filters, fields, true).pushed_predicate
}
pub(crate) fn classify_filter_pushdown<F>(
filter: &Expr,
fields: &[DataField],
+ case_sensitive: bool,
is_exact_filter_pushdown: F,
) -> TableProviderFilterPushDown
where
F: Fn(&Predicate) -> bool,
{
- let translator = FilterTranslator::new(fields);
+ // `FilterTranslator` still supports case-insensitive column resolution for
+ // direct ReadBuilder API callers (and its own unit tests), but the
DataFusion
+ // TableProvider/SQL path always passes `case_sensitive = true`: the
planner
+ // resolves columns against the schema before `scan`, so SQL reads are
+ // case-sensitive. Reporting `Exact` tells DataFusion to drop its residual
+ // filter, so it must only be returned when column resolution is
unambiguous.
+ // The `Inexact` cap for schemas with ASCII case-folding collisions (e.g.
+ // `Name`/`name`) is a conservative guard: if a reference were ever
resolved
+ // case-insensitively, `Exact` would have dropped the residual while the
+ // translation pushed nothing — filtering neither side. Keep the residual.
+ let allow_exact = !has_ascii_case_collision(fields);
Review Comment:
Good catch. Since the DataFusion path always resolves case-sensitively, a
reference matches exactly one field and an unrelated ASCII case-folding
collision (e.g. `Name`/`name`) can never make a resolved filter ambiguous — the
`Inexact` cap was only over-downgrading. Dropped the `has_ascii_case_collision`
guard so classification depends solely on `requires_residual` /
`is_exact_filter_pushdown`, and retargeted the test to assert a filter on `dt`
stays `Exact` in a schema that also has a `Name`/`name` pair. (e205628)
##########
bindings/c/src/table.rs:
##########
@@ -157,6 +167,26 @@ pub unsafe extern "C" fn
paimon_read_builder_with_projection(
std::ptr::null_mut()
}
+/// Set whether column-name matching (projection and predicate resolution) is
Review Comment:
Correct — a predicate is resolved when it is constructed, so this setting
only affects projection matching. Rather than have the read builder
retroactively rewrite an already-resolved predicate (which would break the
independent predicate-constructor ABI), I fixed the doc:
`paimon_read_builder_with_case_sensitive` now documents that it controls
projection only, and that predicate case sensitivity is chosen by the
constructor variant (`paimon_predicate_*` vs
`paimon_predicate_*_with_case_sensitive`), independent of this setting.
(e205628)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
