QuakeWang commented on code in PR #496:
URL: https://github.com/apache/paimon-rust/pull/496#discussion_r3564189062
##########
crates/paimon/src/table/read_builder.rs:
##########
@@ -376,27 +459,91 @@ impl<'a> PaimonReadBuilder<'a> {
self.table.identifier().full_name(),
self.table.schema.fields(),
projection_names,
+ self.case_sensitive,
)
}
}
+/// Look up a schema field by name under the given case sensitivity.
+///
+/// Case-sensitive: exact match. Case-insensitive: match by ASCII
+/// case-folding, returning the unique match, `None` if absent, or
+/// `Error::ConfigInvalid` when two or more fields collide under folding
+/// (ambiguous, mirroring Spark's `AMBIGUOUS` behavior).
+fn find_projection_field<'a>(
+ fields: &'a [DataField],
+ name: &str,
+ case_sensitive: bool,
+ full_name: &str,
+) -> Result<Option<&'a DataField>> {
+ if case_sensitive {
+ return Ok(fields.iter().find(|f| f.name() == name));
+ }
+ let mut matches = fields
+ .iter()
+ .filter(|f| f.name().eq_ignore_ascii_case(name));
+ let first = matches.next();
+ if first.is_some() && matches.next().is_some() {
+ return Err(Error::ConfigInvalid {
+ message: format!(
+ "Ambiguous projection column '{name}' for table {full_name}:
multiple fields match case-insensitively"
+ ),
+ });
+ }
+ Ok(first)
+}
+
+/// Best-effort early validation for `with_projection`: reject only columns
that
+/// cannot match the schema under *any* case sensitivity, so an obvious typo
+/// (e.g. `foo`) fails fast at call time. Case-dependent outcomes (a name that
+/// matches only case-insensitively, or a case-fold ambiguity) are left to the
+/// final resolution in `new_scan`/`new_read`, which uses the effective
+/// `case_sensitive` — keeping `with_projection` and `with_case_sensitive`
+/// order-independent.
+pub(super) fn validate_projection_possible(
+ full_name: String,
+ fields: &[DataField],
+ projection_names: &[String],
+) -> Result<()> {
+ for name in projection_names {
+ if name == crate::spec::ROW_ID_FIELD_NAME {
+ continue;
+ }
+ let matches_any = fields.iter().any(|f|
f.name().eq_ignore_ascii_case(name));
Review Comment:
This scans the full schema once per projected name, and the same lookup runs
again in `new_scan` and `new_read`. The default case-sensitive path regresses
from the previous `O(fields + projections)` lookup to `O(fields ×
projections)`, which is significant for wide tables.
##########
bindings/c/src/table.rs:
##########
@@ -111,8 +112,11 @@ pub unsafe extern "C" fn paimon_read_builder_free(rb: *mut
paimon_read_builder)
/// Set column projection for a ReadBuilder.
///
/// The `columns` parameter is a null-terminated array of null-terminated C
strings.
-/// Output order follows the caller-specified order. Unknown or duplicate names
-/// are validated immediately; an empty list is a valid zero-column projection.
+/// Output order follows the caller-specified order. An empty list is a valid
+/// zero-column projection. Column-name resolution is deferred to
+/// `paimon_read_builder_new_read` (order-independent with
+/// `paimon_read_builder_with_case_sensitive`), so unknown, duplicate, or
Review Comment:
Unknown columns are not always deferred to `paimon_read_builder_new_read`.
The `validate_projection_possible` call below still rejects names that cannot
match under either case mode in this function.
##########
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:
`FilterTranslator` is private to the DataFusion integration, so direct
`ReadBuilder` callers never reach this case-insensitive branch. Every non-test
call site passes `true`, leaving the `false` path test-only.
--
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]
