JunRuiLee commented on code in PR #496:
URL: https://github.com/apache/paimon-rust/pull/496#discussion_r3564356482
##########
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:
Fixed. Both `resolve_projected_fields` and `validate_projection_possible`
now build the name index once (O(fields)) and then look up each projected name
in it, restoring O(fields + projections): an exact-name `HashMap` for the
case-sensitive path, and a folded-name `HashMap` (folding a collision to
`None`) for the case-insensitive path, which keeps the ambiguity check. Dropped
the now-unused `find_projection_field`. (2e8c912)
##########
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:
Fixed the doc to match. An obvious typo — a name that matches no field under
any case sensitivity — is rejected by this call (via
`validate_projection_possible`); only case-dependent outcomes (a name matching
only case-insensitively, or a case-fold duplicate/ambiguity) are deferred to
`paimon_read_builder_new_read`, which uses the case sensitivity effective then.
(2e8c912)
--
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]
