diff --git a/PARITY.md b/PARITY.md index c9eea144b..da83b0d85 100644 --- a/PARITY.md +++ b/PARITY.md @@ -11,6 +11,16 @@ API surface comparison across the three language SDKs. | Row event | `RowEvent` (class, frozen attrs; `file_path` on all variants) | `RowEvent` (enum: Insert/Update/Delete/Error; `file_path` on all variants) | `RowEvent` (plain object with action string; `filePath` on all variants) | | Row type | `dict[str, Any]` | `HashMap` | `Record` | +**Where the row-event flattening lives.** Both bindings publish the same flat +`{table, action, row, old_row, error, file_path}` record, and the decisions +behind it — which variant carries an old row, which one may have no table, the +lowercase `action` names — are language-neutral. Since +[#1131](https://github.com/thekevinscott/dirsql/issues/1131) they live once in +`dirsql::flatten_row_event` (`#[doc(hidden)]`, not a public Rust API); each +binding keeps only the marshaling of the borrowed rows into host values +(`value_row_to_py_dict` / `value_row_to_js`) and its own event struct. No +SDK-visible shape changed — this restores parity rather than introducing drift. + The `on_file` callback receives a single argument: the absolute filesystem path of the matched file. `dirsql` does not read file contents — a callback that needs the file body reads it itself. This is consistent across all three diff --git a/packages/python/changelog.d/2026-09-18-flatten-row-event-in-core.md b/packages/python/changelog.d/2026-09-18-flatten-row-event-in-core.md new file mode 100644 index 000000000..ff4558941 --- /dev/null +++ b/packages/python/changelog.d/2026-09-18-flatten-row-event-in-core.md @@ -0,0 +1,5 @@ +**Changed** + +The extension module now flattens row events through the core's +`flatten_row_event` instead of its own copy of that match. No user-visible +change: `RowEvent` fields and `action` values are identical. diff --git a/packages/python/e2e-attestations/claude-1131-flatten-row-event.json b/packages/python/e2e-attestations/claude-1131-flatten-row-event.json new file mode 100644 index 000000000..d5e741b1f --- /dev/null +++ b/packages/python/e2e-attestations/claude-1131-flatten-row-event.json @@ -0,0 +1,7 @@ +{ + "command": "just test-e2e", + "ran_at": 1789748553, + "exit_code": 0, + "commit": "d106fd4917b9dc93978646f6cb6d8f94374af617", + "branch": "claude/1131-flatten-row-event" +} diff --git a/packages/python/src/lib.rs b/packages/python/src/lib.rs index af496bdab..918f42a25 100644 --- a/packages/python/src/lib.rs +++ b/packages/python/src/lib.rs @@ -13,7 +13,7 @@ #[cfg(feature = "extension-module")] mod python { use ::dirsql::extension_resolution::{self, ConfigSource, PlanEntry}; - use ::dirsql::{DirSQL, Extension, Row, RowEvent, Table, Value}; + use ::dirsql::{DirSQL, Extension, Row, RowEvent, Table, Value, flatten_row_event}; use pyo3::exceptions::{PyOverflowError, PyRuntimeError, PyValueError}; use pyo3::prelude::*; use pyo3::types::{PyBool, PyByteArray, PyBytes, PyDict, PyInt, PyList}; @@ -279,90 +279,18 @@ mod python { PyRuntimeError::new_err(e.to_string()) } - /// Pure, GIL-free intermediate for a row event. [`row_event_to_plain`] - /// builds it from a core [`RowEvent`] (unit-testable without a Python - /// interpreter); [`row_event_to_py`] then marshals it into the - /// Python-facing [`PyRowEvent`] (the GIL step). - struct PlainRowEvent { - table: Option, - action: &'static str, - row: Option, - old_row: Option, - error: Option, - file_path: String, - } - - fn row_event_to_plain(event: &RowEvent) -> PlainRowEvent { - match event { - RowEvent::Insert { - table, - row, - file_path, - } => PlainRowEvent { - table: Some(table.clone()), - action: "insert", - row: Some(row.clone()), - old_row: None, - error: None, - file_path: file_path.clone(), - }, - RowEvent::Update { - table, - old_row, - new_row, - file_path, - } => PlainRowEvent { - table: Some(table.clone()), - action: "update", - row: Some(new_row.clone()), - old_row: Some(old_row.clone()), - error: None, - file_path: file_path.clone(), - }, - RowEvent::Delete { - table, - row, - file_path, - } => PlainRowEvent { - table: Some(table.clone()), - action: "delete", - row: Some(row.clone()), - old_row: None, - error: None, - file_path: file_path.clone(), - }, - RowEvent::Error { - table, - file_path, - error, - } => PlainRowEvent { - table: table.clone(), - action: "error", - row: None, - old_row: None, - error: Some(error.clone()), - file_path: file_path.to_string_lossy().to_string(), - }, - } - } - fn row_event_to_py(py: Python<'_>, event: &RowEvent) -> PyResult { - let plain = row_event_to_plain(event); + let flat = flatten_row_event(event); Ok(PyRowEvent { - table: plain.table, - action: plain.action.to_string(), - row: plain - .row - .as_ref() - .map(|r| value_row_to_py_dict(py, r)) - .transpose()?, - old_row: plain + table: flat.table, + action: flat.action.to_string(), + row: flat.row.map(|r| value_row_to_py_dict(py, r)).transpose()?, + old_row: flat .old_row - .as_ref() .map(|r| value_row_to_py_dict(py, r)) .transpose()?, - error: plain.error, - file_path: Some(plain.file_path), + error: flat.error, + file_path: Some(flat.file_path), }) } @@ -609,76 +537,6 @@ mod python { mod tests { use super::*; - fn one_row() -> HashMap { - HashMap::from([("k".to_string(), Value::Integer(7))]) - } - - #[test] - fn plain_insert_maps_row_and_action() { - let p = row_event_to_plain(&RowEvent::Insert { - table: "t".into(), - row: one_row(), - file_path: "/f".into(), - }); - assert_eq!(p.action, "insert"); - assert_eq!(p.table.as_deref(), Some("t")); - assert_eq!( - p.row.as_ref().and_then(|r| r.get("k")), - Some(&Value::Integer(7)) - ); - assert!(p.old_row.is_none()); - assert!(p.error.is_none()); - assert_eq!(p.file_path, "/f"); - } - - #[test] - fn plain_update_carries_old_and_new() { - let mut new = one_row(); - new.insert("k".to_string(), Value::Integer(9)); - let p = row_event_to_plain(&RowEvent::Update { - table: "t".into(), - old_row: one_row(), - new_row: new, - file_path: "/f".into(), - }); - assert_eq!(p.action, "update"); - assert_eq!( - p.row.as_ref().and_then(|r| r.get("k")), - Some(&Value::Integer(9)) - ); - assert_eq!( - p.old_row.as_ref().and_then(|r| r.get("k")), - Some(&Value::Integer(7)) - ); - } - - #[test] - fn plain_delete_has_row_no_old() { - let p = row_event_to_plain(&RowEvent::Delete { - table: "t".into(), - row: one_row(), - file_path: "/f".into(), - }); - assert_eq!(p.action, "delete"); - assert!(p.row.is_some()); - assert!(p.old_row.is_none()); - } - - #[test] - fn plain_error_has_no_row_and_optional_table() { - let p = row_event_to_plain(&RowEvent::Error { - table: None, - file_path: std::path::PathBuf::from("/f"), - error: "boom".into(), - }); - assert_eq!(p.action, "error"); - assert!(p.table.is_none()); - assert!(p.row.is_none()); - assert!(p.old_row.is_none()); - assert_eq!(p.error.as_deref(), Some("boom")); - assert_eq!(p.file_path, "/f"); - } - #[test] fn on_file_error_displays_inner() { assert_eq!(OnFileError("bad".to_string()).to_string(), "bad"); diff --git a/packages/rust/changelog.d/2026-09-18-flatten-row-event-in-core.md b/packages/rust/changelog.d/2026-09-18-flatten-row-event-in-core.md new file mode 100644 index 000000000..28dd4e57b --- /dev/null +++ b/packages/rust/changelog.d/2026-09-18-flatten-row-event-in-core.md @@ -0,0 +1,6 @@ +**Changed** + +`RowEvent` flattening now lives once in the core as the `#[doc(hidden)]` +`dirsql::flatten_row_event`, instead of being reimplemented per binding. No +observable behavior change: field values, the four `action` strings, and both +error paths are identical. diff --git a/packages/rust/src/lib.rs b/packages/rust/src/lib.rs index 30994baea..d171938f4 100644 --- a/packages/rust/src/lib.rs +++ b/packages/rust/src/lib.rs @@ -36,6 +36,8 @@ pub mod persist; #[doc(hidden)] pub mod progress; #[doc(hidden)] +pub mod row_event_flat; +#[doc(hidden)] pub mod scanner; #[doc(hidden)] pub mod sql_literal; @@ -78,6 +80,8 @@ pub use crate::config::ExtensionSpec as Extension; pub use crate::db::{DbError, Value}; pub use crate::differ::RowEvent; #[doc(hidden)] +pub use crate::row_event_flat::{FlatRowEvent, flatten_row_event}; +#[doc(hidden)] pub use crate::watcher::FileEvent as RawFileEvent; pub type Row = HashMap; diff --git a/packages/rust/src/row_event_flat.rs b/packages/rust/src/row_event_flat.rs new file mode 100644 index 000000000..b8eb03b36 --- /dev/null +++ b/packages/rust/src/row_event_flat.rs @@ -0,0 +1,170 @@ +//! Flattening a [`RowEvent`] into the six-field shape both bindings expose. +//! +//! Every binding publishes the same `{table, action, row, old_row, error, +//! file_path}` record, and none of the decisions behind it are host-specific: +//! which variant carries an old row, which one may have no table, and the +//! lowercase action names all belong to the core enum. Only the marshaling of +//! the borrowed rows into host values is per-binding. +//! +//! Rows are borrowed so the clone stays at the binding boundary, where it is +//! already paid. + +use crate::Row; +#[cfg(test)] +use crate::db::Value; +use crate::differ::RowEvent; + +/// A [`RowEvent`] reduced to the flat record the bindings publish. +#[derive(Debug, Clone, PartialEq)] +pub struct FlatRowEvent<'a> { + pub table: Option, + pub action: &'static str, + pub row: Option<&'a Row>, + pub old_row: Option<&'a Row>, + pub error: Option, + pub file_path: String, +} + +pub fn flatten_row_event(event: &RowEvent) -> FlatRowEvent<'_> { + match event { + RowEvent::Insert { + table, + row, + file_path, + } => FlatRowEvent { + table: Some(table.clone()), + action: "insert", + row: Some(row), + old_row: None, + error: None, + file_path: file_path.clone(), + }, + RowEvent::Update { + table, + old_row, + new_row, + file_path, + } => FlatRowEvent { + table: Some(table.clone()), + action: "update", + row: Some(new_row), + old_row: Some(old_row), + error: None, + file_path: file_path.clone(), + }, + RowEvent::Delete { + table, + row, + file_path, + } => FlatRowEvent { + table: Some(table.clone()), + action: "delete", + row: Some(row), + old_row: None, + error: None, + file_path: file_path.clone(), + }, + RowEvent::Error { + table, + file_path, + error, + } => FlatRowEvent { + table: table.clone(), + action: "error", + row: None, + old_row: None, + error: Some(error.clone()), + file_path: file_path.to_string_lossy().to_string(), + }, + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use super::*; + + fn one_row(v: i64) -> Row { + Row::from([("k".to_string(), Value::Integer(v))]) + } + + #[test] + fn insert_maps_row_and_action() { + let event = RowEvent::Insert { + table: "t".into(), + row: one_row(7), + file_path: "/f".into(), + }; + let flat = flatten_row_event(&event); + assert_eq!(flat.action, "insert"); + assert_eq!(flat.table.as_deref(), Some("t")); + assert_eq!(flat.row.and_then(|r| r.get("k")), Some(&Value::Integer(7))); + assert!(flat.old_row.is_none()); + assert!(flat.error.is_none()); + assert_eq!(flat.file_path, "/f"); + } + + #[test] + fn update_carries_new_row_as_row_and_old_row_beside_it() { + let event = RowEvent::Update { + table: "t".into(), + old_row: one_row(7), + new_row: one_row(9), + file_path: "/f".into(), + }; + let flat = flatten_row_event(&event); + assert_eq!(flat.action, "update"); + assert_eq!(flat.table.as_deref(), Some("t")); + assert_eq!(flat.row.and_then(|r| r.get("k")), Some(&Value::Integer(9))); + assert_eq!( + flat.old_row.and_then(|r| r.get("k")), + Some(&Value::Integer(7)) + ); + assert!(flat.error.is_none()); + assert_eq!(flat.file_path, "/f"); + } + + #[test] + fn delete_has_a_row_but_no_old_row() { + let event = RowEvent::Delete { + table: "t".into(), + row: one_row(7), + file_path: "/f".into(), + }; + let flat = flatten_row_event(&event); + assert_eq!(flat.action, "delete"); + assert_eq!(flat.table.as_deref(), Some("t")); + assert_eq!(flat.row.and_then(|r| r.get("k")), Some(&Value::Integer(7))); + assert!(flat.old_row.is_none()); + assert!(flat.error.is_none()); + assert_eq!(flat.file_path, "/f"); + } + + #[test] + fn error_has_no_rows_and_stringifies_its_path() { + let event = RowEvent::Error { + table: None, + file_path: PathBuf::from("/f"), + error: "boom".into(), + }; + let flat = flatten_row_event(&event); + assert_eq!(flat.action, "error"); + assert!(flat.table.is_none()); + assert!(flat.row.is_none()); + assert!(flat.old_row.is_none()); + assert_eq!(flat.error.as_deref(), Some("boom")); + assert_eq!(flat.file_path, "/f"); + } + + #[test] + fn error_keeps_the_table_when_the_variant_names_one() { + let event = RowEvent::Error { + table: Some("t".into()), + file_path: PathBuf::from("/f"), + error: "boom".into(), + }; + let flat = flatten_row_event(&event); + assert_eq!(flat.table.as_deref(), Some("t")); + } +} diff --git a/packages/ts/changelog.d/2026-09-18-flatten-row-event-in-core.md b/packages/ts/changelog.d/2026-09-18-flatten-row-event-in-core.md new file mode 100644 index 000000000..be82759c9 --- /dev/null +++ b/packages/ts/changelog.d/2026-09-18-flatten-row-event-in-core.md @@ -0,0 +1,5 @@ +**Changed** + +The native addon now flattens row events through the core's +`flatten_row_event` instead of its own copy of that match. No user-visible +change: `RowEvent` fields and `action` values are identical. diff --git a/packages/ts/e2e-attestations/claude-1131-flatten-row-event.json b/packages/ts/e2e-attestations/claude-1131-flatten-row-event.json new file mode 100644 index 000000000..9a825f7c2 --- /dev/null +++ b/packages/ts/e2e-attestations/claude-1131-flatten-row-event.json @@ -0,0 +1,7 @@ +{ + "command": "pnpm test:e2e", + "ran_at": 1789748596, + "exit_code": 0, + "commit": "6821ef4c95e1c652bdba657af6d55d8ecfd547dd", + "branch": "claude/1131-flatten-row-event" +} diff --git a/packages/ts/napi/src/lib.rs b/packages/ts/napi/src/lib.rs index a9f0a296c..58e9f9e4a 100644 --- a/packages/ts/napi/src/lib.rs +++ b/packages/ts/napi/src/lib.rs @@ -21,7 +21,7 @@ use dirsql::extension_resolution::{self, ConfigSource as CoreConfigSource, PlanEntry}; use dirsql::{ DirSQL as CoreDirSQL, Extension, PreparedBuild, RawFileEvent, Row, RowEvent as CoreRowEvent, - Table, Value, + Table, Value, flatten_row_event, }; use napi::Task; use napi::bindgen_prelude::*; @@ -821,56 +821,14 @@ fn value_row_to_js(row: &HashMap) -> Result Result { - Ok(match event { - CoreRowEvent::Insert { - table, - row, - file_path, - } => RowEvent { - table: Some(table.clone()), - action: "insert".to_string(), - row: Some(value_row_to_js(row)?), - old_row: None, - error: None, - file_path: Some(file_path.clone()), - }, - CoreRowEvent::Update { - table, - old_row, - new_row, - file_path, - } => RowEvent { - table: Some(table.clone()), - action: "update".to_string(), - row: Some(value_row_to_js(new_row)?), - old_row: Some(value_row_to_js(old_row)?), - error: None, - file_path: Some(file_path.clone()), - }, - CoreRowEvent::Delete { - table, - row, - file_path, - } => RowEvent { - table: Some(table.clone()), - action: "delete".to_string(), - row: Some(value_row_to_js(row)?), - old_row: None, - error: None, - file_path: Some(file_path.clone()), - }, - CoreRowEvent::Error { - table, - file_path, - error, - } => RowEvent { - table: table.clone(), - action: "error".to_string(), - row: None, - old_row: None, - error: Some(error.clone()), - file_path: Some(file_path.to_string_lossy().to_string()), - }, + let flat = flatten_row_event(event); + Ok(RowEvent { + table: flat.table, + action: flat.action.to_string(), + row: flat.row.map(value_row_to_js).transpose()?, + old_row: flat.old_row.map(value_row_to_js).transpose()?, + error: flat.error, + file_path: Some(flat.file_path), }) }