From 6a008d04902ab4bf7346071789d9bca21c68d395 Mon Sep 17 00:00:00 2001 From: Graham Hukill Date: Tue, 28 Jul 2026 07:41:13 -0400 Subject: [PATCH 1/3] glob append deltas instead of virtual filename column Why these changes are being introduced: Work on another application revealed a bug in TDA, that might arguably be a bug in DuckDB itself. Wwhen reading parquet you can rename the virtual `"filename"` column to something else. We did so, renaming to `"append_delta_filename"`, because our append delta parquet files had a real `"filename"` column already. When selecting columns, this renaming works as expected. But when applying a `WHERE` clause, specifically against the `"filename"` column, DuckDB does not honor the renaming and the `WHERE` acts on the wrong column. The append deltas are part of the main `metadata.records` view, seamlessly including records in the append deltas with records in the static DuckDB database file. Filtering by `where filename = ...` had unexpected results due to this DuckDB irregularity. How this addresses that need: Until that is fixed upsteam in DuckDB, or in the event it is not fixed, the changes here avoid this renaming altogether. We use `glob()` to identify append delta parquet files for counting and merging, which we had already been using for counting. Side effects of this change: * There is no longer a `"append_delta_filename"` column in `metadata.records`, because we have removed the virtual `"filename"` renaming for the append delta parquet files we project over. Thankfully, this was not used for ETL purposes anywhere, it was only work on a new metadata analysis application that revealed this. Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/TIMX-646 --- tests/test_metadata.py | 25 ++++++++++++++++++++++++- timdex_dataset_api/metadata.py | 22 ++++++---------------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/tests/test_metadata.py b/tests/test_metadata.py index a27bc51..bc93912 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -262,7 +262,6 @@ def test_tdm_append_deltas_view_empty_structure(timdex_metadata): "run_record_offset", "run_timestamp", "filename", - "append_delta_filename", } assert set(append_deltas_df.columns) == expected_columns assert len(append_deltas_df) == 0 @@ -323,6 +322,30 @@ def test_tdm_append_deltas_view_has_data(timdex_metadata_with_deltas): assert append_deltas_count > 0 +def test_tdm_filename_filter_matches_append_delta_rows(timdex_metadata_with_deltas): + conn = timdex_metadata_with_deltas.timdex_dataset.conn + filename = conn.execute( + "select filename from metadata.records_append_deltas limit 1" + ).fetchone()[0] + + delta_count = conn.execute( + "select count(*) from metadata.records_append_deltas where filename = ?", + [filename], + ).fetchone()[0] + records_count = conn.execute( + "select count(*) from metadata.records where filename = ?", [filename] + ).fetchone()[0] + shadow_count = conn.execute(""" + select count(*) + from metadata.records_append_deltas + where filename like '%append_delta%' + """).fetchone()[0] + + assert delta_count > 0 + assert records_count >= delta_count + assert shadow_count == 0 + + def test_tdm_records_includes_deltas(timdex_metadata_with_deltas): static_count = timdex_metadata_with_deltas.timdex_dataset.conn.query( """select count(*) from static_db.records;""" diff --git a/timdex_dataset_api/metadata.py b/timdex_dataset_api/metadata.py index 5b1d5d6..5e93533 100644 --- a/timdex_dataset_api/metadata.py +++ b/timdex_dataset_api/metadata.py @@ -347,10 +347,7 @@ def _create_append_deltas_view( conn.execute(f""" create or replace view metadata.{view_name} as ( select * - from read_parquet( - '{deltas_path}/*.parquet', - filename = 'append_delta_filename' - ) + from read_parquet('{deltas_path}/*.parquet') ); """) return @@ -365,8 +362,7 @@ def _create_append_deltas_view( if table_exists: conn.execute(f""" create or replace view metadata.{view_name} as ( - select *, - null::varchar as append_delta_filename + select * from {static_table} where 1 = 0 ); @@ -578,22 +574,16 @@ def merge_append_deltas(self) -> None: all_delta_filenames: dict[str, list[str]] = {} has_any_deltas = False for data_type_class in self.data_type_classes: - deltas_view = f"{data_type_class.NAME}_append_deltas" + deltas_glob = f"{self.append_deltas_path_for(data_type_class)}/*.parquet" try: filenames = ( self.timdex_dataset.conn.query(f""" - select distinct(append_delta_filename) - from metadata.{deltas_view} + select file from glob('{deltas_glob}') """) - .to_df()["append_delta_filename"] + .to_df()["file"] .to_list() ) - except ( - DuckDBIOException, - DuckDBCatalogException, - DuckDBBinderException, - KeyError, - ): + except (DuckDBIOException, KeyError): filenames = [] all_delta_filenames[data_type_class.NAME] = filenames if filenames: From 4919951a06d8947e21868cd9b9f3a4c85d9e8538 Mon Sep 17 00:00:00 2001 From: Graham Hukill Date: Tue, 28 Jul 2026 08:21:43 -0400 Subject: [PATCH 2/3] Bump version to 5.2 --- pyproject.toml | 2 +- uv.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 2d37c04..e7cf310 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "timdex_dataset_api" -version = "5.1.0" +version = "5.2.0" description = "Python library for interacting with a TIMDEX parquet dataset" readme = "README.md" requires-python = ">=3.12" diff --git a/uv.lock b/uv.lock index 07913a1..57682a3 100644 --- a/uv.lock +++ b/uv.lock @@ -1612,7 +1612,7 @@ wheels = [ [[package]] name = "timdex-dataset-api" -version = "5.1.0" +version = "5.2.0" source = { editable = "." } dependencies = [ { name = "attrs" }, From 959db1fdf1108670b76a709057dfc288fc3f05d6 Mon Sep 17 00:00:00 2001 From: Graham Hukill Date: Tue, 28 Jul 2026 09:50:40 -0400 Subject: [PATCH 3/3] Gracefully handle missing append deltas Why these changes are being introduced: It is quite common for there to be zero append deltas for a given data type, this should be gracefully handled. How this addresses that need: * Catch DuckDB errors when attempting to count append deltas Side effects of this change: * None Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/TIMX-646 --- tests/test_metadata.py | 10 ++++++++++ timdex_dataset_api/metadata.py | 9 ++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/test_metadata.py b/tests/test_metadata.py index bc93912..6d933bd 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -289,6 +289,10 @@ def test_tdm_append_deltas_count_property_empty(timdex_metadata): assert timdex_metadata.append_deltas_count == 0 +def test_tdm_append_deltas_count_for_missing_view(timdex_metadata_empty): + assert timdex_metadata_empty.append_deltas_count_for(TIMDEXRecords) == 0 + + def test_tdm_records_equals_static_without_deltas(timdex_metadata): static_count = timdex_metadata.timdex_dataset.conn.query( """select count(*) from static_db.records;""" @@ -457,6 +461,12 @@ def test_tdm_merge_append_deltas_deletes_append_deltas( assert not os.listdir(records_deltas_path_after) +def test_tdm_append_deltas_count_for_after_merge(timdex_metadata_with_deltas): + timdex_metadata_with_deltas.merge_append_deltas() + + assert timdex_metadata_with_deltas.append_deltas_count_for(TIMDEXRecords) == 0 + + def test_tdm_embeddings_metadata_view_structure(tmp_path): td = TIMDEXDataset(str(tmp_path / "embeddings_metadata_structure")) diff --git a/timdex_dataset_api/metadata.py b/timdex_dataset_api/metadata.py index 5e93533..94376dc 100644 --- a/timdex_dataset_api/metadata.py +++ b/timdex_dataset_api/metadata.py @@ -129,9 +129,12 @@ def current_records_count(self) -> int: def append_deltas_count_for(self, data_type_class: type["TIMDEXDataType"]) -> int: """Count append deltas rows for a single data type.""" view_name = f"{data_type_class.NAME}_append_deltas" - return self.timdex_dataset.conn.query(f""" - select count(*) from metadata.{view_name}; - """).fetchone()[0] # type: ignore[index] + try: + return self.timdex_dataset.conn.query(f""" + select count(*) from metadata.{view_name}; + """).fetchone()[0] # type: ignore[index] + except (DuckDBCatalogException, DuckDBIOException): + return 0 @property def append_deltas_count(self) -> int: