Skip to content

Clean DataFolder and extend tests#382

Merged
CGodiksen merged 49 commits intomainfrom
dev/clean-data-folder
Mar 24, 2026
Merged

Clean DataFolder and extend tests#382
CGodiksen merged 49 commits intomainfrom
dev/clean-data-folder

Conversation

@CGodiksen
Copy link
Copy Markdown
Collaborator

This PR closes #357 by cleaning up the API of DataFolder and extending the tests to cover all functionality.

The main changes in the PR are:

  • Combining functionality for handling metadata tables and normal tables
  • Adding a new write method that can be used for both normal tables and time series tables
  • Cleaning up the public API of DataFolder
  • Reordering methods and tests to better match the rest of the repository
  • Adding tests for all public methods in DataFolder
  • Adding tests for all public methods in DeltaTableWriter

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors DataFolder to unify handling of normal vs metadata tables, introduces a shared write path, and expands test coverage across DataFolder and DeltaTableWriter, aligning with issue #357.

Changes:

  • Unifies metadata-table querying by removing MetadataTable and reusing NormalTable with optional DataSink (INSERT disabled for metadata tables).
  • Introduces a standalone DeltaTableWriter module and updates DataFolder to expose a single write_record_batches() API for both normal and time series tables.
  • Expands and reorganizes tests to cover public APIs and rollback/commit behavior for DeltaTableWriter.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
crates/modelardb_test/src/table.rs Adds shared NoOpDataSink test utility (currently panics via unimplemented!()).
crates/modelardb_test/Cargo.toml Adds async-trait and datafusion dependencies to support test sink.
crates/modelardb_storage/src/query/normal_table.rs Extends NormalTable to support metadata tables by making DataSink optional and rejecting INSERT when absent.
crates/modelardb_storage/src/query/mod.rs Removes metadata_table module from the query layer.
crates/modelardb_storage/src/query/metadata_table.rs Deletes dedicated metadata table provider implementation.
crates/modelardb_storage/src/optimizer/model_simple_aggregates.rs Switches tests to use shared NoOpDataSink from modelardb_test.
crates/modelardb_storage/src/lib.rs Updates normal table registration to pass Some(data_sink) to NormalTable.
crates/modelardb_storage/src/data_folder/mod.rs Cleans up constructors, adds table_writer() + unified write_record_batches(), and significantly expands tests.
crates/modelardb_storage/src/data_folder/delta_table_writer.rs New module for transactional Delta writes, including commit/rollback tests.
crates/modelardb_server/src/storage/data_transfer.rs Simplifies remote writes to use write_record_batches() regardless of table type.
crates/modelardb_server/src/storage/compressed_data_manager.rs Switches both normal and time-series writes to write_record_batches().
crates/modelardb_server/src/context.rs Updates tests to use unified write API.
crates/modelardb_embedded/src/operations/data_folder.rs Updates embedded operations to use unified write API.
crates/modelardb_bulkloader/src/main.rs Updates DeltaTableWriter import path to new module location.
Cargo.lock Records new transitive dependencies for modelardb_test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@CGodiksen CGodiksen requested a review from chrthomsen March 20, 2026 17:09
@CGodiksen CGodiksen merged commit 1f9c0e5 into main Mar 24, 2026
5 checks passed
@CGodiksen CGodiksen deleted the dev/clean-data-folder branch March 24, 2026 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clean DataFolder and extend its tests

4 participants