You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by gi...@apache.org on 2026/10/11 13:53:27 UTC
(iceberg-rust) 01/01: fix(scan): reject ambiguous case-insensitive fields (#3070)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-3070-e42840bc98cbf91a67c014162ed68036a8452d7a
in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git
commit d63e8610f2152158882f8773f3745f3f32752912
Author: Manu Zhang <Ow...@gmail.com>
AuthorDate: Sun Oct 11 13:53:01 2026 +0000
fix(scan): reject ambiguous case-insensitive fields (#3070)
* fix(scan): reject ambiguous case-insensitive fields
Co-authored-by: Codex <co...@openai.com>
* fix(schema): clarify ambiguous case-insensitive lookup
Expose the checked lookup, document collision semantics, and cover the public fallback. Simplify the binding and scan callers.
Co-authored-by: Codex <co...@openai.com>
* fix(schema): improve case-insensitive collision diagnostics
List sorted colliding field names with invalid_data!, simplify optional lookup, and loosen error assertions while checking the reported colliders.
Co-authored-by: Codex <co...@openai.com>
---------
Co-authored-by: Codex <co...@openai.com>
---
crates/iceberg/public-api.txt | 1 +
crates/iceberg/src/expr/term.rs | 19 +++++++-
crates/iceberg/src/scan/mod.rs | 68 ++++++++++++++++++++++++---
crates/iceberg/src/spec/schema/mod.rs | 86 ++++++++++++++++++++++++++++++++---
4 files changed, 160 insertions(+), 14 deletions(-)
diff --git a/crates/iceberg/public-api.txt b/crates/iceberg/public-api.txt
index 8a34bbf7b..466614757 100644
--- a/crates/iceberg/public-api.txt
+++ b/crates/iceberg/public-api.txt
@@ -2474,6 +2474,7 @@ pub fn iceberg::spec::Schema::field_by_alias(&self, alias: &str) -> core::option
pub fn iceberg::spec::Schema::field_by_id(&self, field_id: i32) -> core::option::Option<&iceberg::spec::NestedFieldRef>
pub fn iceberg::spec::Schema::field_by_name(&self, field_name: &str) -> core::option::Option<&iceberg::spec::NestedFieldRef>
pub fn iceberg::spec::Schema::field_by_name_case_insensitive(&self, field_name: &str) -> core::option::Option<&iceberg::spec::NestedFieldRef>
+pub fn iceberg::spec::Schema::field_by_name_case_insensitive_checked(&self, field_name: &str) -> iceberg::Result<core::option::Option<&iceberg::spec::NestedFieldRef>>
pub fn iceberg::spec::Schema::field_id_by_name(&self, name: &str) -> core::option::Option<i32>
pub fn iceberg::spec::Schema::field_id_to_fields(&self) -> &std::collections::hash::map::HashMap<i32, iceberg::spec::NestedFieldRef>
pub fn iceberg::spec::Schema::field_id_to_name_map(&self) -> &std::collections::hash::map::HashMap<i32, alloc::string::String>
diff --git a/crates/iceberg/src/expr/term.rs b/crates/iceberg/src/expr/term.rs
index 4a5aedf74..7c5cc0891 100644
--- a/crates/iceberg/src/expr/term.rs
+++ b/crates/iceberg/src/expr/term.rs
@@ -313,7 +313,7 @@ impl Bind for Reference {
let field = if case_sensitive {
schema.field_by_name(&self.name)
} else {
- schema.field_by_name_case_insensitive(&self.name)
+ schema.field_by_name_case_insensitive_checked(&self.name)?
};
let field =
@@ -428,6 +428,23 @@ mod tests {
assert_eq!(expected_ref, reference);
}
+ #[test]
+ fn test_bind_reference_case_insensitive_ambiguous() {
+ let schema = Arc::new(
+ Schema::builder()
+ .with_fields(vec![
+ NestedField::optional(1, "id", PrimitiveType::Int.into()).into(),
+ NestedField::optional(2, "ID", PrimitiveType::Int.into()).into(),
+ ])
+ .build()
+ .unwrap(),
+ );
+
+ let error = Reference::new("Id").bind(schema, false).unwrap_err();
+ assert_eq!(error.kind(), crate::ErrorKind::DataInvalid);
+ assert!(error.message().contains("case-insensitively"), "{error}");
+ }
+
#[test]
fn test_bind_reference_failure() {
let schema = table_schema_simple();
diff --git a/crates/iceberg/src/scan/mod.rs b/crates/iceberg/src/scan/mod.rs
index 0c241125d..7e81900a5 100644
--- a/crates/iceberg/src/scan/mod.rs
+++ b/crates/iceberg/src/scan/mod.rs
@@ -53,13 +53,16 @@ use crate::{Error, ErrorKind, Result};
pub type ArrowRecordBatchStream = BoxStream<'static, Result<RecordBatch>>;
/// Resolves a column name to its field ID, honouring the scan's case sensitivity.
-fn resolve_field_id(schema: &Schema, column_name: &str, case_sensitive: bool) -> Option<i32> {
+fn resolve_field_id(
+ schema: &Schema,
+ column_name: &str,
+ case_sensitive: bool,
+) -> Result<Option<i32>> {
if case_sensitive {
- schema.field_id_by_name(column_name)
+ Ok(schema.field_id_by_name(column_name))
} else {
- schema
- .field_by_name_case_insensitive(column_name)
- .map(|field| field.id)
+ let field = schema.field_by_name_case_insensitive_checked(column_name)?;
+ Ok(field.map(|field| field.id))
}
}
@@ -79,7 +82,7 @@ fn collect_scan_field_ids(
return get_metadata_field_id(column_name);
}
- let field_id = resolve_field_id(schema, column_name, case_sensitive).ok_or_else(|| {
+ let field_id = resolve_field_id(schema, column_name, case_sensitive)?.ok_or_else(|| {
invalid_data!("Column {column_name} not found in table. Schema: {schema}")
})?;
@@ -692,6 +695,7 @@ mod tests {
use futures::{TryStreamExt, stream};
use uuid::Uuid;
+ use super::resolve_field_id;
use crate::arrow::ArrowReaderBuilder;
use crate::expr::{BoundPredicate, Reference};
use crate::io::FileIO;
@@ -761,6 +765,58 @@ mod tests {
);
}
+ #[test]
+ fn test_resolve_field_id_case_insensitive_rejects_ambiguous_name() {
+ let schema = Schema::builder()
+ .with_fields(vec![
+ NestedField::optional(1, "id", PrimitiveType::Int.into()).into(),
+ NestedField::optional(2, "ID", PrimitiveType::Int.into()).into(),
+ NestedField::optional(3, "data", PrimitiveType::Int.into()).into(),
+ ])
+ .build()
+ .unwrap();
+
+ assert_eq!(resolve_field_id(&schema, "id", true).unwrap(), Some(1));
+ assert_eq!(resolve_field_id(&schema, "DATA", false).unwrap(), Some(3));
+ let error = resolve_field_id(&schema, "Id", false).unwrap_err();
+ assert_eq!(error.kind(), ErrorKind::DataInvalid);
+ assert!(error.message().contains("case-insensitively"), "{error}");
+ }
+
+ #[test]
+ fn test_case_insensitive_scan_with_ambiguous_schema() {
+ let fixture = TableTestFixture::new();
+ let mut metadata = fixture.table.metadata().clone();
+ let schema = Schema::builder()
+ .with_schema_id(metadata.current_schema_id())
+ .with_fields(vec![
+ NestedField::optional(1, "id", PrimitiveType::Int.into()).into(),
+ NestedField::optional(2, "ID", PrimitiveType::Int.into()).into(),
+ NestedField::optional(3, "data", PrimitiveType::Int.into()).into(),
+ ])
+ .build()
+ .unwrap();
+ metadata
+ .schemas
+ .insert(metadata.current_schema_id(), Arc::new(schema));
+ let table = fixture.table.with_metadata(Arc::new(metadata));
+
+ table
+ .scan()
+ .with_case_sensitive(false)
+ .select(["DATA"])
+ .build()
+ .unwrap();
+ let error = table
+ .scan()
+ .with_case_sensitive(false)
+ .select(["Id"])
+ .build()
+ .unwrap_err();
+ assert_eq!(error.kind(), ErrorKind::DataInvalid);
+ assert!(error.message().contains("case-insensitively"), "{error}");
+ }
+
#[tokio::test]
async fn test_case_insensitive_scan_resolves_mismatched_column_case() {
let mut fixture = TableTestFixture::new();
diff --git a/crates/iceberg/src/spec/schema/mod.rs b/crates/iceberg/src/spec/schema/mod.rs
index c6dcf0707..67924dfe8 100644
--- a/crates/iceberg/src/spec/schema/mod.rs
+++ b/crates/iceberg/src/spec/schema/mod.rs
@@ -71,7 +71,8 @@ pub struct Schema {
id_to_field: HashMap<i32, NestedFieldRef>,
name_to_id: HashMap<String, i32>,
- lowercase_name_to_id: HashMap<String, i32>,
+ // `None` marks a lower-cased name that resolves to more than one field.
+ lowercase_name_to_id: HashMap<String, Option<i32>>,
id_to_name: HashMap<i32, String>,
field_id_to_accessor: HashMap<i32, Arc<StructAccessor>>,
@@ -150,10 +151,13 @@ impl SchemaBuilder {
index.indexes()
};
- let lowercase_name_to_id = name_to_id
- .iter()
- .map(|(k, v)| (k.to_lowercase(), *v))
- .collect();
+ let mut lowercase_name_to_id = HashMap::new();
+ for (name, field_id) in &name_to_id {
+ lowercase_name_to_id
+ .entry(name.to_lowercase())
+ .and_modify(|existing| *existing = None)
+ .or_insert(Some(*field_id));
+ }
let highest_field_id = id_to_field.keys().max().cloned().unwrap_or(0);
@@ -342,13 +346,47 @@ impl Schema {
.and_then(|id| self.field_by_id(*id))
}
- /// Get field by field name, but in case-insensitive way.
+ /// Get field by field name in a case-insensitive way.
///
/// Both full name and short name could work here.
+ /// Returns `None` if the name is missing or matches multiple fields case-insensitively.
+ /// Use [`Self::field_by_name_case_insensitive_checked`] to distinguish those cases.
pub fn field_by_name_case_insensitive(&self, field_name: &str) -> Option<&NestedFieldRef> {
self.lowercase_name_to_id
.get(&field_name.to_lowercase())
- .and_then(|id| self.field_by_id(*id))
+ .copied()
+ .flatten()
+ .and_then(|id| self.field_by_id(id))
+ }
+
+ /// Get field by field name in a case-insensitive way, reporting ambiguous names.
+ ///
+ /// Both full name and short name could work here. Returns `Ok(None)` if the name is
+ /// missing, or an error if it matches multiple fields case-insensitively. A collision
+ /// on another name does not prevent this name from resolving. This differs from Iceberg
+ /// Java, which rejects all case-insensitive lookups after a collision, and PyIceberg,
+ /// which silently keeps one of the colliding fields.
+ pub fn field_by_name_case_insensitive_checked(
+ &self,
+ field_name: &str,
+ ) -> Result<Option<&NestedFieldRef>> {
+ let lowercase_name = field_name.to_lowercase();
+ match self.lowercase_name_to_id.get(&lowercase_name) {
+ Some(Some(id)) => Ok(self.field_by_id(*id)),
+ Some(None) => {
+ let mut colliders: Vec<&str> = self
+ .name_to_id
+ .keys()
+ .filter(|name| name.to_lowercase() == lowercase_name)
+ .map(String::as_str)
+ .collect();
+ colliders.sort_unstable();
+ Err(invalid_data!(
+ "Multiple fields match {field_name} case-insensitively: {colliders:?}"
+ ))
+ }
+ None => Ok(None),
+ }
}
/// Get field by alias.
@@ -973,6 +1011,40 @@ table {
}
}
+ #[test]
+ fn test_schema_field_by_name_case_insensitive_checked() {
+ let schema = Schema::builder()
+ .with_fields(vec![
+ NestedField::optional(1, "id", Primitive(PrimitiveType::Int)).into(),
+ NestedField::optional(2, "ID", Primitive(PrimitiveType::Int)).into(),
+ NestedField::optional(3, "data", Primitive(PrimitiveType::String)).into(),
+ ])
+ .build()
+ .expect("case-insensitive name ambiguity is valid in a schema");
+
+ assert_eq!(
+ schema
+ .field_by_name_case_insensitive_checked("DATA")
+ .unwrap()
+ .map(|field| field.id),
+ Some(3)
+ );
+ assert!(
+ schema
+ .field_by_name_case_insensitive_checked("missing")
+ .unwrap()
+ .is_none()
+ );
+
+ let error = schema
+ .field_by_name_case_insensitive_checked("Id")
+ .unwrap_err();
+ assert_eq!(error.kind(), crate::ErrorKind::DataInvalid);
+ assert!(error.message().contains("case-insensitively"), "{error}");
+ assert!(error.message().contains("[\"ID\", \"id\"]"), "{error}");
+ assert!(schema.field_by_name_case_insensitive("Id").is_none());
+ }
+
#[test]
fn test_schema_find_column_name() {
let expected_column_name = HashMap::from([