Skip to content

Commit 490b3bf

Browse files
Merge branch 'main' into feat/table-features-and-write-tests
2 parents 5bf741d + ab2eb39 commit 490b3bf

7 files changed

Lines changed: 553 additions & 61 deletions

File tree

kernel/src/actions/visitors.rs

Lines changed: 124 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22
//! [`crate::engine_data::EngineData`] types.
33
44
use std::collections::HashMap;
5-
use std::sync::LazyLock;
5+
use std::sync::{Arc, LazyLock};
66

77
use delta_kernel_derive::internal_api;
88

99
use crate::engine_data::{GetData, RowVisitor, TypedGetData as _};
10-
use crate::schema::{column_name, ColumnName, ColumnNamesAndTypes, DataType, ToSchema as _};
10+
use crate::schema::{column_name, ColumnName, ColumnNamesAndTypes, DataType, Schema, StructField};
1111
use crate::utils::require;
1212
use crate::{DeltaResult, Error};
1313

@@ -600,14 +600,82 @@ pub(crate) fn visit_protocol_at<'a>(
600600
Ok(Some(protocol))
601601
}
602602

603+
/// This visitor extracts the in-commit timestamp (ICT) from a CommitInfo action in the log it is
604+
/// present. The [`EngineData`] being visited must have the schema defined in
605+
/// [`InCommitTimestampVisitor::schema`].
606+
///
607+
/// Only the a single row of the engine data is checked (the first row). This is because in-commit
608+
/// timestamps requires that the CommitInfo containing the ICT be the first action in the log.
609+
#[allow(unused)]
610+
#[derive(Default)]
611+
pub(crate) struct InCommitTimestampVisitor {
612+
pub(crate) in_commit_timestamp: Option<i64>,
613+
}
614+
615+
impl InCommitTimestampVisitor {
616+
#[allow(unused)]
617+
/// Get the schema that the visitor expects the data to have.
618+
pub(crate) fn schema() -> Arc<Schema> {
619+
static SCHEMA: LazyLock<Arc<Schema>> = LazyLock::new(|| {
620+
let ict_type = StructField::new("inCommitTimestamp", DataType::LONG, true);
621+
Arc::new(StructType::new(vec![StructField::new(
622+
COMMIT_INFO_NAME,
623+
StructType::new([ict_type]),
624+
true,
625+
)]))
626+
});
627+
SCHEMA.clone()
628+
}
629+
}
630+
impl RowVisitor for InCommitTimestampVisitor {
631+
fn selected_column_names_and_types(
632+
&self,
633+
) -> (&'static [crate::schema::ColumnName], &'static [DataType]) {
634+
static NAMES_AND_TYPES: LazyLock<ColumnNamesAndTypes> = LazyLock::new(|| {
635+
let names = vec![column_name!("commitInfo.inCommitTimestamp")];
636+
let types = vec![DataType::LONG];
637+
638+
(names, types).into()
639+
});
640+
NAMES_AND_TYPES.as_ref()
641+
}
642+
643+
fn visit<'a>(
644+
&mut self,
645+
row_count: usize,
646+
getters: &[&'a dyn crate::engine_data::GetData<'a>],
647+
) -> DeltaResult<()> {
648+
require!(
649+
getters.len() == 1,
650+
Error::InternalError(format!(
651+
"Wrong number of InCommitTimestampVisitor getters: {}",
652+
getters.len()
653+
))
654+
);
655+
656+
// If the batch is empty, return
657+
if row_count == 0 {
658+
return Ok(());
659+
}
660+
// CommitInfo must be the first action in a commit
661+
if let Some(in_commit_timestamp) = getters[0].get_long(0, "commitInfo.inCommitTimestamp")? {
662+
self.in_commit_timestamp = Some(in_commit_timestamp);
663+
}
664+
Ok(())
665+
}
666+
}
667+
603668
#[cfg(test)]
604669
mod tests {
605670
use super::*;
606671

607672
use crate::arrow::array::StringArray;
608673

674+
use crate::engine::sync::SyncEngine;
675+
use crate::expressions::{column_expr, Expression};
609676
use crate::table_features::{ReaderFeature, WriterFeature};
610677
use crate::utils::test_utils::{action_batch, parse_json_batch};
678+
use crate::Engine;
611679

612680
#[test]
613681
fn test_parse_protocol() -> DeltaResult<()> {
@@ -985,4 +1053,58 @@ mod tests {
9851053
.unwrap();
9861054
assert!(domain_metadata_visitor.domain_metadatas.is_empty());
9871055
}
1056+
1057+
/*************************************
1058+
* In-commit timestamp visitor tests *
1059+
**************************************/
1060+
1061+
fn add_action() -> &'static str {
1062+
r#"{"add":{"path":"file1","partitionValues":{"c1":"6","c2":"a"},"size":452,"modificationTime":1670892998137,"dataChange":true}}"#
1063+
}
1064+
fn commit_info_action() -> &'static str {
1065+
r#"{"commitInfo":{"inCommitTimestamp":1677811178585, "timestamp":1677811178585,"operation":"WRITE","operationParameters":{"mode":"ErrorIfExists","partitionBy":"[]"},"isolationLevel":"WriteSerializable","isBlindAppend":true,"operationMetrics":{"numFiles":"1","numOutputRows":"10","numOutputBytes":"635"},"engineInfo":"Databricks-Runtime/<unknown>","txnId":"a6a94671-55ef-450e-9546-b8465b9147de"}}"#
1066+
}
1067+
1068+
fn transform_batch(batch: Box<dyn EngineData>) -> Box<dyn EngineData> {
1069+
let engine = SyncEngine::new();
1070+
engine
1071+
.evaluation_handler()
1072+
.new_expression_evaluator(
1073+
get_log_schema().clone(),
1074+
Expression::Struct(vec![Expression::Struct(vec![column_expr!(
1075+
"commitInfo.inCommitTimestamp"
1076+
)])]),
1077+
InCommitTimestampVisitor::schema().into(),
1078+
)
1079+
.evaluate(batch.as_ref())
1080+
.unwrap()
1081+
}
1082+
1083+
// Helper function to reduce duplication in tests
1084+
fn run_timestamp_visitor_test(json_strings: Vec<&str>, expected_timestamp: Option<i64>) {
1085+
let json_strings: StringArray = json_strings.into();
1086+
let batch = parse_json_batch(json_strings);
1087+
let batch = transform_batch(batch);
1088+
let mut visitor = InCommitTimestampVisitor::default();
1089+
visitor.visit_rows_of(batch.as_ref()).unwrap();
1090+
assert_eq!(visitor.in_commit_timestamp, expected_timestamp);
1091+
}
1092+
1093+
#[test]
1094+
fn commit_info_not_first() {
1095+
run_timestamp_visitor_test(vec![add_action(), commit_info_action()], None);
1096+
}
1097+
1098+
#[test]
1099+
fn commit_info_not_present() {
1100+
run_timestamp_visitor_test(vec![add_action()], None);
1101+
}
1102+
1103+
#[test]
1104+
fn commit_info_get() {
1105+
run_timestamp_visitor_test(
1106+
vec![commit_info_action(), add_action()],
1107+
Some(1677811178585), // Retrieved ICT
1108+
);
1109+
}
9881110
}

kernel/src/history_manager/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
pub(crate) mod search;

0 commit comments

Comments
 (0)