Skip to content

Commit b4d831f

Browse files
fix(codegen): provenance view uses ROW_NUMBER for latest-per-entity (#82)
* fix(codegen): temporal current-version UNIQUE; valid_to CHECK Closes #41. `verisimdb_temporal_versions` had a non-unique partial index on `(entity_id, table_name) WHERE valid_to IS NULL`. Two concurrent writers could both successfully insert a row with `valid_to=NULL` for the same entity, leaving two "current" versions and breaking the point-in-time query semantics. There was also no constraint that `valid_to` (when set) couldn't precede `valid_from`. - Promote the partial index to `CREATE UNIQUE INDEX` so the storage layer enforces "at most one current version per (entity, table)". - Add `CHECK (valid_to IS NULL OR valid_to >= valid_from)` so a version interval can't be backwards. Test `test_temporal_table_has_unique_partial_index_and_valid_to_check` asserts both clauses appear in the emitted DDL with temporal enabled. `cargo clippy --all-targets -- -D warnings` clean; 37 unit tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(codegen): provenance view uses ROW_NUMBER for latest-per-entity Closes #40. `generate_provenance_view` emitted a correlated MAX(timestamp) subquery whose outer reference `verisimdb_provenance_log.entity_id` was ambiguous: the same table appears in multiple nested scopes, and depending on planner choices the correlation either bound to the wrong scope (returning all rows for the entity) or worked accidentally. The view did not reliably return exactly one "latest" row per entity. Replace with a window-function pattern: ROW_NUMBER() OVER (PARTITION BY entity_id ORDER BY timestamp DESC) picked out as `_rn = 1`. No correlation, no scoping ambiguity, one row per entity by construction. Supported on PostgreSQL (always) and SQLite ≥ 3.25. Test `test_provenance_view_uses_window_function_for_latest_per_entity` asserts the new pattern is present and the old `MAX(p2.timestamp)` pattern is gone. `cargo clippy --all-targets -- -D warnings` clean; 38 unit tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent a4dc26e commit b4d831f

1 file changed

Lines changed: 43 additions & 9 deletions

File tree

src/codegen/query.rs

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,13 @@ fn generate_provenance_view(
180180
table_name
181181
);
182182

183-
// Use a subquery to get the latest provenance entry per entity.
183+
// Select the latest provenance row per entity via ROW_NUMBER() over a
184+
// PARTITION-by-entity / ORDER-by-timestamp-DESC window. The previous
185+
// correlated MAX(timestamp) subquery referenced the outer
186+
// `verisimdb_provenance_log` table by base-name, which is ambiguous
187+
// when the same table appears in nested scopes and produced wrong
188+
// (or no) "latest" rows depending on planner choices. Closes #40.
189+
// ROW_NUMBER is supported on PostgreSQL (always) and SQLite ≥ 3.25.
184190
format!(
185191
"{comment}\
186192
CREATE VIEW IF NOT EXISTS verisimdb_{table_name}_with_provenance AS\n\
@@ -193,14 +199,13 @@ fn generate_provenance_view(
193199
FROM {table_name}\n\
194200
LEFT JOIN (\n\
195201
\x20 SELECT entity_id, operation, actor, timestamp, hash\n\
196-
\x20 FROM verisimdb_provenance_log\n\
197-
\x20 WHERE table_name = '{table_name}'\n\
198-
\x20 AND timestamp = (\n\
199-
\x20 SELECT MAX(p2.timestamp)\n\
200-
\x20 FROM verisimdb_provenance_log p2\n\
201-
\x20 WHERE p2.entity_id = verisimdb_provenance_log.entity_id\n\
202-
\x20 AND p2.table_name = '{table_name}'\n\
203-
\x20 )\n\
202+
\x20 FROM (\n\
203+
\x20 SELECT entity_id, operation, actor, timestamp, hash,\n\
204+
\x20 ROW_NUMBER() OVER (PARTITION BY entity_id ORDER BY timestamp DESC) AS _rn\n\
205+
\x20 FROM verisimdb_provenance_log\n\
206+
\x20 WHERE table_name = '{table_name}'\n\
207+
\x20 ) ranked\n\
208+
\x20 WHERE _rn = 1\n\
204209
) prov ON prov.entity_id = ({entity_id_expr});\n\n",
205210
columns = column_list.join(",\n"),
206211
)
@@ -398,6 +403,35 @@ mod tests {
398403
assert!(interceptor.access_filter.is_none());
399404
}
400405

406+
/// The provenance view must use a window-function `ROW_NUMBER()` pattern
407+
/// instead of the old (broken) correlated MAX(timestamp) subquery.
408+
/// Closes #40.
409+
#[test]
410+
fn test_provenance_view_uses_window_function_for_latest_per_entity() {
411+
let schema = test_schema();
412+
let octad = OctadConfig {
413+
enable_provenance: true,
414+
enable_lineage: false,
415+
enable_temporal: false,
416+
enable_access_control: false,
417+
enable_constraints: false,
418+
enable_simulation: false,
419+
};
420+
let interceptors = generate_interceptors(&schema, &octad, DatabaseBackend::SQLite);
421+
let view = interceptors[0]
422+
.provenance_view
423+
.as_ref()
424+
.expect("provenance view present when enabled");
425+
assert!(
426+
view.contains("ROW_NUMBER() OVER (PARTITION BY entity_id ORDER BY timestamp DESC)"),
427+
"provenance view should use window-function latest-per-entity"
428+
);
429+
assert!(
430+
!view.contains("MAX(p2.timestamp)"),
431+
"old correlated MAX(timestamp) pattern must be gone"
432+
);
433+
}
434+
401435
#[test]
402436
fn test_provenance_view_references_table() {
403437
let schema = test_schema();

0 commit comments

Comments
 (0)