blackmwk commented on code in PR #3131:
URL: https://github.com/apache/iceberg-rust/pull/3131#discussion_r3902500277
##########
crates/iceberg/src/scan/task.rs:
##########
@@ -267,73 +267,32 @@ impl FileScanTask {
}
fn validate(&self) -> Result<()> {
- let Some(partition) = self.partition.as_ref() else {
- return Ok(());
- };
- let Some(partition_spec) = self.partition_spec.as_deref() else {
- return if partition.fields().is_empty() {
- Ok(())
- } else {
- Err(Error::new(
- ErrorKind::DataInvalid,
- "Non-empty FileScanTask partition requires a partition
spec",
- ))
- };
- };
-
- if partition.fields().len() != partition_spec.fields().len() {
- return Err(Error::new(
+ 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,
- format!(
- "FileScanTask partition has {} fields but partition spec
has {} fields",
- partition.fields().len(),
- partition_spec.fields().len()
- ),
- ));
- }
-
- if partition_spec
- .fields()
- .iter()
- .any(|field| self.schema.field_by_id(field.source_id).is_none())
- {
- // A historical partition spec may legally reference a source
column that has
- // since been dropped. Without its type information, the partition
values cannot
- // be validated, but they are still valid advisory metadata for
the scan task.
- return Ok(());
- }
Review Comment:
No, this partition_spec vs schema check should still exists
--
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]
