JunRuiLee commented on code in PR #496:
URL: https://github.com/apache/paimon-rust/pull/496#discussion_r3564357083
##########
crates/integrations/datafusion/src/filter_pushdown.rs:
##########
@@ -81,18 +89,29 @@ 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
Review Comment:
True that every current call site passes `true` — but the `case_sensitive`
parameter is intentional, not dead code. It is the explicit carrier of the "SQL
path is always case-sensitive" contract: `scan` and the variant-extraction
planner both pass `true` on purpose, and the end-to-end negative test (`SELECT
name` vs a `Name` field fails at planning) pins that. The `false` path keeps
`FilterTranslator`/`classify` consistent with the rest of the
case-insensitivity machinery and is exercised by unit tests. I would rather
keep the contract visible in the signature than hardcode `true` internally and
lose that. Happy to revisit if you feel strongly.
--
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]
