From 50579bbbcb9272bba97d598dcef7e83919e8f140 Mon Sep 17 00:00:00 2001 From: Ion Koutsouris <15728914+ion-elgreco@users.noreply.github.com> Date: Sun, 6 Oct 2024 16:44:42 +0200 Subject: [PATCH 1/3] fix: don't create new metadata struct --- crates/core/src/operations/write.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/core/src/operations/write.rs b/crates/core/src/operations/write.rs index 36dcec5b70..c27a4f205b 100644 --- a/crates/core/src/operations/write.rs +++ b/crates/core/src/operations/write.rs @@ -950,7 +950,7 @@ impl std::future::IntoFuture for WriteBuilder { if let Some(snapshot) = &this.snapshot { let schema_struct: StructType = schema.clone().try_into()?; let current_protocol = snapshot.protocol(); - let configuration = snapshot.metadata().configuration.clone(); + let mut metadata = snapshot.metadata().clone(); let maybe_new_protocol = if PROTOCOL .contains_timestampntz(schema_struct.fields()) && !current_protocol @@ -965,19 +965,18 @@ impl std::future::IntoFuture for WriteBuilder { if !(current_protocol.min_reader_version == 3 && current_protocol.min_writer_version == 7) { - Some(new_protocol.move_table_properties_into_features(&configuration)) + Some(new_protocol.move_table_properties_into_features( + &metadata.configuration.clone(), + )) } else { Some(new_protocol) } } else { None }; - let schema_action = Action::Metadata(Metadata::try_new( - schema_struct, - partition_columns.clone(), - configuration, - )?); - actions.push(schema_action); + metadata.schema_string = serde_json::to_string(&schema_struct)?; + metadata.partition_columns = partition_columns.clone(); + actions.push(metadata.into()); if let Some(new_protocol) = maybe_new_protocol { actions.push(new_protocol.into()) } From c8b47a0e4fe1c15ae185038f6f315917ae66fb2c Mon Sep 17 00:00:00 2001 From: "R. Tyler Croy" Date: Mon, 7 Oct 2024 14:23:23 +0000 Subject: [PATCH 2/3] Revert "fix: don't create new metadata struct" This reverts commit 1f6cd6f87c4cfbeaca63d7e9659ab9a9fb4ebe3c. --- crates/core/src/operations/write.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/core/src/operations/write.rs b/crates/core/src/operations/write.rs index c27a4f205b..36dcec5b70 100644 --- a/crates/core/src/operations/write.rs +++ b/crates/core/src/operations/write.rs @@ -950,7 +950,7 @@ impl std::future::IntoFuture for WriteBuilder { if let Some(snapshot) = &this.snapshot { let schema_struct: StructType = schema.clone().try_into()?; let current_protocol = snapshot.protocol(); - let mut metadata = snapshot.metadata().clone(); + let configuration = snapshot.metadata().configuration.clone(); let maybe_new_protocol = if PROTOCOL .contains_timestampntz(schema_struct.fields()) && !current_protocol @@ -965,18 +965,19 @@ impl std::future::IntoFuture for WriteBuilder { if !(current_protocol.min_reader_version == 3 && current_protocol.min_writer_version == 7) { - Some(new_protocol.move_table_properties_into_features( - &metadata.configuration.clone(), - )) + Some(new_protocol.move_table_properties_into_features(&configuration)) } else { Some(new_protocol) } } else { None }; - metadata.schema_string = serde_json::to_string(&schema_struct)?; - metadata.partition_columns = partition_columns.clone(); - actions.push(metadata.into()); + let schema_action = Action::Metadata(Metadata::try_new( + schema_struct, + partition_columns.clone(), + configuration, + )?); + actions.push(schema_action); if let Some(new_protocol) = maybe_new_protocol { actions.push(new_protocol.into()) } From 32a3945a808d1a1d47f8537bc4e4b9fee2d51004 Mon Sep 17 00:00:00 2001 From: "R. Tyler Croy" Date: Mon, 7 Oct 2024 14:31:28 +0000 Subject: [PATCH 3/3] fix: ensure metaData.createdTime is set to the action creation time Fixes #2925 --- crates/core/src/kernel/models/actions.rs | 2 +- crates/core/src/operations/write.rs | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/core/src/kernel/models/actions.rs b/crates/core/src/kernel/models/actions.rs index 6ec8fc11fb..bfcd9ec757 100644 --- a/crates/core/src/kernel/models/actions.rs +++ b/crates/core/src/kernel/models/actions.rs @@ -86,7 +86,7 @@ impl Metadata { configuration, name: None, description: None, - created_time: None, + created_time: Some(chrono::Utc::now().timestamp_millis()), }) } diff --git a/crates/core/src/operations/write.rs b/crates/core/src/operations/write.rs index 36dcec5b70..5deaaa1393 100644 --- a/crates/core/src/operations/write.rs +++ b/crates/core/src/operations/write.rs @@ -1588,6 +1588,15 @@ mod tests { let names = fields.map(|f| f.name()).collect::>(); assert_eq!(names, vec!["id", "value", "modified", "inserted_by"]); + // + let metadata = table + .metadata() + .expect("Failed to retrieve updated metadata"); + assert_ne!( + None, metadata.created_time, + "Created time should be the milliseconds since epoch of when the action was created" + ); + let write_metrics: WriteMetrics = get_write_metrics(table.clone()).await; assert_common_write_metrics(write_metrics); }