Skip to content

Commit e832955

Browse files
authored
Merge pull request #302 from benfleis/move-metadata-writes-to-append-phase
move metadata writes from Commit to Append phase
2 parents 776be7a + 6aeda65 commit e832955

3 files changed

Lines changed: 20 additions & 18 deletions

File tree

duckdb

Submodule duckdb updated 394 files

src/storage/delta_transaction.cpp

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,8 @@ struct WriteMetaData {
278278

279279
ffi::ArrowFFIData ffi_data;
280280
unordered_map<idx_t, const shared_ptr<ArrowTypeExtensionData>> extension_types;
281-
ClientProperties props("UTC", ArrowOffsetSize::REGULAR, false, false, false, ArrowFormatVersion::V1_0, context);
281+
ClientProperties props("UTC", ArrowOffsetSize::REGULAR, false, false, false, ArrowFormatVersion::V1_0,
282+
optional_ptr<ClientContext>(&context));
282283
ArrowConverter::ToArrowArray(*buffer, (ArrowArray *)(&ffi_data.array), props, extension_types);
283284
ArrowConverter::ToArrowSchema((ArrowSchema *)(&ffi_data.schema), buffer_types, GetNames(), props);
284285

@@ -411,21 +412,6 @@ void DeltaTransaction::Commit(ClientContext &context) {
411412
transaction_state = DeltaTransactionState::TRANSACTION_FINISHED;
412413

413414
if (!outstanding_appends.empty()) {
414-
auto write_context = ffi::get_write_context(kernel_transaction.get());
415-
416-
// Create metadata from the current outstanding appends
417-
WriteMetaData write_metadata(*table_entry->snapshot, outstanding_appends);
418-
// Convert write metadata to ArrowFFI
419-
auto write_metadata_ffi = write_metadata.ToArrow(context);
420-
421-
// Convert to Delta Kernel EngineData
422-
KernelEngineData write_metadata_engine_data =
423-
table_entry->snapshot->TryUnpackKernelResult(ffi::get_engine_data(
424-
write_metadata_ffi.array, &write_metadata_ffi.schema, DuckDBEngineError::AllocateError));
425-
426-
// Add the write data to the commit
427-
ffi::add_files(kernel_transaction.get(), write_metadata_engine_data.release());
428-
429415
// Finally we add the registered transaction versions
430416
for (const auto &app_version : app_versions) {
431417
auto app_id = app_version.first;
@@ -567,6 +553,22 @@ void DeltaTransaction::Append(ClientContext &context, const vector<DeltaDataFile
567553
auto f = fs.OpenFile(file.file_name, FileOpenFlags::FILE_FLAGS_READ);
568554
file.last_modified_time = f->file_system.GetLastModifiedTime(*f);
569555
}
556+
557+
if (!append_files.empty()) {
558+
// Build and add write metadata for new files per append; we do so here instead of in ::Commit
559+
// within Commit we no longer have an active transaction, which is required to build the arrow schema. We could
560+
// alternatively extend the Arrow API to support pre-build/cache the schema, but writing per append here is
561+
// simple.
562+
vector<DeltaDataFile> new_files(outstanding_appends.begin() + NumericCast<ptrdiff_t>(start),
563+
outstanding_appends.end());
564+
WriteMetaData write_metadata(*table_entry->snapshot, new_files);
565+
auto write_metadata_ffi = write_metadata.ToArrow(context);
566+
567+
KernelEngineData write_metadata_engine_data = table_entry->snapshot->TryUnpackKernelResult(ffi::get_engine_data(
568+
write_metadata_ffi.array, &write_metadata_ffi.schema, DuckDBEngineError::AllocateError));
569+
570+
ffi::add_files(kernel_transaction.get(), write_metadata_engine_data.release());
571+
}
570572
}
571573

572574
void DeltaTransaction::SetTransactionVersion(const string &app_id_p, idx_t new_version_p, Value expected_version_p) {

0 commit comments

Comments
 (0)