blackmwk commented on code in PR #3131:
URL: https://github.com/apache/iceberg-rust/pull/3131#discussion_r3913030881
##########
crates/iceberg/src/scan/task.rs:
##########
@@ -183,14 +231,89 @@ impl FileScanTask {
self.predicate.as_ref()
}
- /// Returns the schema of this file scan task as a reference
- pub fn schema(&self) -> &Schema {
- &self.schema
+ /// Returns the delete files that may need to be applied to the data file.
+ pub fn deletes(&self) -> &[FileScanTaskDeleteFile] {
+ &self.deletes
}
- /// Returns the schema of this file scan task as a SchemaRef
- pub fn schema_ref(&self) -> SchemaRef {
- self.schema.clone()
+ /// Returns the partition data from the manifest entry.
+ pub fn partition(&self) -> Option<&Struct> {
+ self.partition.as_ref()
+ }
+
+ /// Returns the partition spec for the data file.
+ pub fn partition_spec(&self) -> Option<&Arc<PartitionSpec>> {
+ self.partition_spec.as_ref()
+ }
+
+ /// Returns the name mapping used to resolve field ids.
+ pub fn name_mapping(&self) -> Option<&Arc<NameMapping>> {
+ self.name_mapping.as_ref()
+ }
+
+ /// Returns the unified partition type across all table partition specs.
+ pub fn unified_partition_type(&self) -> Option<&Arc<StructType>> {
+ self.unified_partition_type.as_ref()
+ }
+
+ /// Returns whether names are treated as case-sensitive.
+ pub fn case_sensitive(&self) -> bool {
+ self.case_sensitive
+ }
+
+ /// Returns the key metadata for the encrypted data file.
+ pub fn key_metadata(&self) -> Option<&[u8]> {
+ self.key_metadata.as_deref()
+ }
+
+ fn validate(&self) -> Result<()> {
+ match (self.partition.as_ref(), self.partition_spec.as_deref()) {
+ (None, None) => Ok(()),
+ (None, Some(partition_spec)) if partition_spec.is_unpartitioned()
=> Ok(()),
+ (None, Some(_)) => Err(Error::new(
+ ErrorKind::DataInvalid,
+ "FileScanTask with a partitioned spec requires partition
values",
+ )),
+ (Some(partition), None) if partition.fields().is_empty() => Ok(()),
+ (Some(_), None) => Err(Error::new(
+ ErrorKind::DataInvalid,
+ "Non-empty FileScanTask partition requires a partition spec",
+ )),
+ (Some(partition), Some(partition_spec))
+ if partition.fields().len() != partition_spec.fields().len() =>
+ {
+ Err(Error::new(
+ ErrorKind::DataInvalid,
+ format!(
+ "FileScanTask partition has {} fields but partition
spec has {} fields",
+ partition.fields().len(),
+ partition_spec.fields().len()
+ ),
+ ))
+ }
+ (Some(_), Some(partition_spec)) => {
+ if partition_spec
+ .fields()
+ .iter()
+ .any(|field|
self.schema.field_by_id(field.source_id).is_none())
+ {
+ // Historical specs may reference source columns that were
later dropped.
+ // Without those source types, the spec cannot be checked
against the
+ // current schema, but the partition metadata remains
valid for the task.
Review Comment:
Removed the duplicate source-column precheck in 2021e72e4. This branch now
contains only partition_spec.partition_type(&self.schema)? for schema/spec
validation, so one missing source no longer skips validation of the whole spec.
Updated both builder and scan coverage for the propagated missing-source error;
all 1,623 local unit tests and the full local gates pass.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
