laskoviymishka commented on code in PR #3432:
URL: https://github.com/apache/iceberg-rust/pull/3432#discussion_r4394088767
##########
crates/iceberg/src/transaction/snapshot.rs:
##########
@@ -658,6 +664,78 @@ mod tests {
assert_eq!(properties.get("removed-files-size").unwrap(), "100");
}
+ #[test]
+ fn test_summary_keeps_decimal_scale_of_dropped_partition_source() {
+ let table = make_v2_minimal_table();
+ let without_d = table.metadata().current_schema().as_ref().clone();
+ let d = NestedField::required(
+ 4,
+ "d",
+ Type::Primitive(PrimitiveType::Decimal {
+ precision: 9,
+ scale: 2,
+ }),
+ );
+ let x = NestedField::required(1, "x",
Type::Primitive(PrimitiveType::Int));
Review Comment:
Could this build the history in the order a real table would? `x` is `long`
in the minimal table, so `with_d` declaring it `int` is a long→int demotion
Iceberg doesn't allow, and the test only exercises `(Int, Long)`. The realistic
case is a file written before the promotion: `{d, x: int}` → promote `x` to
long → drop `d`, with `Literal::int(5)` for `x`, which hits `(Long, Int)`. The
same `d=1.23/x=5` assertion should hold.
##########
crates/iceberg/src/transaction/snapshot.rs:
##########
@@ -431,11 +431,17 @@ impl<'a> SnapshotProducer<'a> {
.ok_or_else(|| {
invalid_data!("Unknown partition spec id {}",
data_file.partition_spec_id)
})?;
- summary_collector.remove_file(
- data_file,
- table_metadata.current_schema().clone(),
- partition_spec.clone(),
- );
+ // A dropped source column keeps its type, such as a decimal's
scale, only in an
+ // older schema.
+ let schema = table_metadata
+ .schemas_iter()
Review Comment:
nit: this only depends on the spec, so it could be resolved once per
`partition_spec_id` instead of per removed file. A large delete rescans every
schema for every file.
--
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]
