Avoid empty results table in main DB for split raw data storage#405
Open
astafan8 wants to merge 1 commit into
Open
Avoid empty results table in main DB for split raw data storage#405astafan8 wants to merge 1 commit into
astafan8 wants to merge 1 commit into
Conversation
Previously a results table was still created (empty) in the main database for split-storage datasets, purely to satisfy code paths that assumed the table exists. This removes that table entirely so the main DB holds only metadata - cleaner and future-proof for non-sqlite raw data backends. Changes: - Do not create the results table in the main DB when raw storage is enabled (create_run_table=False, insert_into_results_table=False), mirroring how DataSetInMem records runs. - Identify split-storage runs via a dedicated 'raw_data_db_path' column in the runs table, recorded at dataset creation. This disambiguates them from DataSetInMem runs (which also have no results table) in _get_datasetprotocol_from_guid, replacing the old reliance on the empty table's presence. - Keep 'raw_data_db_path' out of the user-facing metadata dict (read/write via get/set_raw_data_db_path_for_run); this also stops it leaking into metadata comparisons and extract/export copies. - number_of_results/__len__ return 0 when the results table does not exist (e.g. a pristine, not-yet-started split dataset). - Defer subscriptions requested before the dataset is started until start time, when the raw data table exists (fixes subscriber trigger creation). - Restrict management queries (purge/cleanup) to started runs only. - Docs + tests updated; add tests for subscribe-before-start, count before start, no-main-table, and extract not carrying over raw_data_db_path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is a stacked change on top of microsoft#8219 (targets
feature/split-raw-data-sqlite, notmain) that removes the empty results table from the main database for split raw data storage.Previously, even with split raw data storage enabled, an empty results table was still created in the main database purely to satisfy code paths that assumed the table exists. That was flagged in microsoft#8219 as unclean/hacky, and becomes more confusing once raw data can be written to non-SQLite backends. This PR removes that table entirely: the main DB then holds only metadata.
What changes
No results table in the main DB
create_run_table=Falseand parameters are added withinsert_into_results_table=False. This mirrors exactly howDataSetInMem(the netcdf-backed dataset) already records runs without a results table.Identifying split-storage runs (disambiguation)
DataSetapart from aDataSetInMem(both otherwise "have no results table") in_get_datasetprotocol_from_guid.raw_data_db_pathcolumn in therunstable, recorded at dataset creation (so even an unstarted dataset is correctly identified). This is future-proof for other raw-data backends.raw_data_db_pathis now an internal storage detailrunstable but excluded from the user-facingmetadatadict (viaget_metadata_from_run_id), read/written throughget_raw_data_db_path_for_run/set_raw_data_db_path_for_run.the_same_dataset_as) and being copied into extracted/exported databases.Features that depended on the empty table — now handled explicitly
number_of_results/__len__): return0when the results table does not exist (e.g. a pristine, not-yet-started split dataset).load_by_id/load_by_guid/load_by_countercorrectly reconstruct split datasets (started or not) from theraw_data_db_pathmarker.purge_orphaned_datasets/cleanup_datasets): restricted to started runs only, since an unstarted run records an intended path but never creates the file.Verification
Validated by temporarily enabling split raw data storage globally for the whole
tests/datasetsuite (autouse fixture) and iterating until only expected/inherent failures remained:tests/datasetsuite in default mode: 1064 passed, 35 skipped — no regressions.queries.get_parameter_data(ds.conn, ...)directly, bypassing the DataSet API (already documented expected failures in feat: split raw data storage into per-dataset SQLite files microsoft/Qcodes#8219).test_db_overviewtests + 1test_guids_from_dirassert main-DB row counts / scan for DB files — these are inherent to the feature (data isn't in the main DB) and fail with the empty-table approach too; they are not regressions from removing the table.test_raw_data_storage.py: all pass, with new tests for subscribe-before-start deferral, counting before start, absence of a main-DB results table, and extract not carrying overraw_data_db_path.Note
The autouse "global enable" fixture used for validation is not included in this PR — it was only a local harness.