Skip to content

Commit 060455c

Browse files
committed
sub(feat): include primary key in change msg
- It's the PK that the user really needs and that might not be the rowid - If the table schema includes WITHOUT ROWID, the first/only column making up the PK is returned. That's hokey. So now we return None for rowid in that case - Needed to add some introspection logic to find which column(s) make up the PK and whether the table was created using WITHOUT ROWID
1 parent b126ff1 commit 060455c

9 files changed

Lines changed: 375 additions & 30 deletions

File tree

Cargo.lock

Lines changed: 4 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/sqlx-sqlite-observer/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ tokio = { version = "1.49.0", features = ["sync"] }
2323
thiserror = "2.0.17"
2424
tracing = { version = "0.1.44", default-features = false, features = ["std", "release_max_level_off"] }
2525
parking_lot = "0.12.3"
26+
regex = "1.12.3"
27+
sqlx = { version = "0.8.6", features = ["sqlite", "runtime-tokio"], default-features = false }
2628
# Required for preupdate_hook - SQLite must be compiled with SQLITE_ENABLE_PREUPDATE_HOOK
2729
libsqlite3-sys = { version = "0.30", features = ["preupdate_hook"] }
2830

crates/sqlx-sqlite-observer/README.md

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,65 @@ This ensures subscribers **only receive notifications for committed changes**.
116116
* **`ObservableSqliteDatabase`**: Wrapper for `SqliteDatabase` with observation
117117
* **`ObservableWriteGuard`**: Write guard with hooks registered
118118

119+
### `TableInfo`
120+
121+
Schema information for observed tables (used internally, also exported).
122+
123+
* `pk_columns: Vec<usize>` - Column indices forming the primary key
124+
* `without_rowid: bool` - Whether the table uses WITHOUT ROWID
125+
126+
## Primary Key Extraction
127+
128+
The `primary_key` field on `TableChange` always contains the actual primary key
129+
value(s) for the affected row:
130+
131+
```rust
132+
let change = rx.recv().await?;
133+
134+
// Single-column PK (e.g., INTEGER PRIMARY KEY)
135+
if let Some(ColumnValue::Integer(id)) = change.primary_key.first() {
136+
println!("Changed row id: {}", id);
137+
}
138+
139+
// Composite PK - values are in declaration order
140+
for (i, pk_value) in change.primary_key.iter().enumerate() {
141+
println!("PK column {}: {:?}", i, pk_value);
142+
}
143+
```
144+
145+
**Why `primary_key` instead of just `rowid`?**
146+
147+
SQLite's internal `rowid` works well for tables with `INTEGER PRIMARY KEY`, but
148+
has limitations:
149+
150+
* **Text or UUID primary keys**: The `rowid` is an internal integer, not your
151+
actual key
152+
* **Composite primary keys**: The `rowid` doesn't represent your multi-column
153+
key
154+
* **WITHOUT ROWID tables**: The `rowid` from the preupdate hook is unreliable
155+
156+
The `primary_key` field extracts the actual primary key values from the captured
157+
column data, giving you meaningful identifiers regardless of table structure.
158+
159+
### WITHOUT ROWID Tables
160+
161+
For tables created with `WITHOUT ROWID`, the `rowid` field in `TableChange` will
162+
be `None`:
163+
164+
```rust
165+
let change = rx.recv().await?;
166+
167+
if change.rowid.is_none() {
168+
// This is a WITHOUT ROWID table
169+
// Use primary_key instead
170+
println!("PK: {:?}", change.primary_key);
171+
}
172+
```
173+
174+
This is because SQLite's preupdate hook provides the first PRIMARY KEY column
175+
(coerced to i64) as the "rowid" for WITHOUT ROWID tables, which may not be
176+
meaningful/correct for non-integer or composite primary keys.
177+
119178
## Examples
120179

121180
> **Coming in Phase 2** - Full working examples will be added in a subsequent PR.

crates/sqlx-sqlite-observer/src/broker.rs

Lines changed: 92 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,18 @@
3030
//! ```
3131
//!
3232
//! Changes captured by the preupdate hook are buffered until the transaction
33-
//! completes. On commit, buffered changes are published to subscribers. On
34-
//! rollback (or implicit rollback due to an error), they are discarded without
35-
//! notification.
33+
//! (explicit or implicit) completes. On commit, buffered changes are published
34+
//! to subscribers. On rollback, they are discarded without notification.
3635
37-
use std::collections::HashSet;
36+
use std::collections::{HashMap, HashSet};
3837
use std::sync::Arc;
3938
use std::time::Instant;
4039

4140
use parking_lot::{Mutex, RwLock};
4241
use tokio::sync::broadcast;
43-
use tracing::{debug, trace};
42+
use tracing::{debug, error, trace};
4443

45-
use crate::change::{ChangeOperation, TableChange};
44+
use crate::change::{ChangeOperation, ColumnValue, TableChange, TableInfo};
4645
use crate::hooks::{PreUpdateEvent, SqliteValue};
4746

4847
/// Transaction-aware observation broker.
@@ -54,6 +53,7 @@ pub struct ObservationBroker {
5453
buffer: Mutex<Vec<PreUpdateEvent>>,
5554
change_tx: broadcast::Sender<TableChange>,
5655
observed_tables: RwLock<HashSet<String>>,
56+
table_info: RwLock<HashMap<String, TableInfo>>,
5757
capture_values: bool,
5858
}
5959

@@ -65,6 +65,7 @@ impl ObservationBroker {
6565
buffer: Mutex::new(Vec::new()),
6666
change_tx,
6767
observed_tables: RwLock::new(HashSet::new()),
68+
table_info: RwLock::new(HashMap::new()),
6869
capture_values,
6970
})
7071
}
@@ -74,20 +75,25 @@ impl ObservationBroker {
7475
self.observed_tables.read().contains(table)
7576
}
7677

77-
/// Registers tables for observation.
78+
/// Registers a table for observation with its schema information.
7879
///
7980
/// Only changes to observed tables will be buffered and published.
80-
pub fn observe_tables<I, S>(&self, tables: I)
81-
where
82-
I: IntoIterator<Item = S>,
83-
S: AsRef<str>,
84-
{
85-
let mut observed = self.observed_tables.write();
86-
for table in tables {
87-
let table_name = table.as_ref().to_string();
88-
trace!(table = %table_name, "Observing table");
89-
observed.insert(table_name);
90-
}
81+
/// The `TableInfo` is required to correctly extract primary key values
82+
/// and determine whether the rowid is meaningful for the table.
83+
pub fn observe_table(&self, table: &str, info: TableInfo) {
84+
trace!(
85+
table = %table,
86+
pk_columns = ?info.pk_columns,
87+
without_rowid = info.without_rowid,
88+
"Observing table with schema info"
89+
);
90+
self.observed_tables.write().insert(table.to_string());
91+
self.table_info.write().insert(table.to_string(), info);
92+
}
93+
94+
/// Gets the schema information for an observed table.
95+
pub fn get_table_info(&self, table: &str) -> Option<TableInfo> {
96+
self.table_info.read().get(table).cloned()
9197
}
9298

9399
/// Returns a list of all observed tables.
@@ -125,8 +131,14 @@ impl ObservationBroker {
125131
debug!(count = events.len(), "Flushing buffered changes on commit");
126132

127133
for event in events {
128-
let table_change = self.event_to_change(event);
129-
let _ = self.change_tx.send(table_change);
134+
match self.event_to_change(event) {
135+
Ok(table_change) => {
136+
let _ = self.change_tx.send(table_change);
137+
}
138+
Err(e) => {
139+
error!(error = %e, "Failed to convert event to change");
140+
}
141+
}
130142
}
131143
}
132144

@@ -155,13 +167,22 @@ impl ObservationBroker {
155167
}
156168

157169
/// Converts a PreUpdateEvent to a TableChange for broadcast.
158-
fn event_to_change(&self, event: PreUpdateEvent) -> TableChange {
159-
let rowid = match event.operation {
160-
ChangeOperation::Insert => Some(event.new_rowid),
161-
ChangeOperation::Delete => Some(event.old_rowid),
162-
ChangeOperation::Update => Some(event.new_rowid),
170+
fn event_to_change(&self, event: PreUpdateEvent) -> crate::Result<TableChange> {
171+
let table_info = self.table_info.read().get(&event.table).cloned();
172+
173+
// For WITHOUT ROWID tables, the rowid from preupdate hook is not meaningful
174+
let rowid = match &table_info {
175+
Some(info) if info.without_rowid => None,
176+
_ => match event.operation {
177+
ChangeOperation::Insert => Some(event.new_rowid),
178+
ChangeOperation::Delete => Some(event.old_rowid),
179+
ChangeOperation::Update => Some(event.new_rowid),
180+
},
163181
};
164182

183+
// Extract primary key values from the appropriate column values
184+
let primary_key = self.extract_primary_key(&event, table_info.as_ref())?;
185+
165186
let (old_values, new_values) = if self.capture_values {
166187
(
167188
event.old_values.map(Self::values_to_vec),
@@ -171,14 +192,59 @@ impl ObservationBroker {
171192
(None, None)
172193
};
173194

174-
TableChange {
195+
Ok(TableChange {
175196
table: event.table,
176197
operation: Some(event.operation),
177198
rowid,
199+
primary_key,
178200
old_values,
179201
new_values,
180202
timestamp: Instant::now(),
203+
})
204+
}
205+
206+
/// Extracts primary key values from the event based on table schema.
207+
///
208+
/// Returns an error if the schema has drifted (e.g., table was altered)
209+
/// and PK column indices are out of bounds.
210+
fn extract_primary_key(
211+
&self,
212+
event: &PreUpdateEvent,
213+
table_info: Option<&TableInfo>,
214+
) -> crate::Result<Vec<ColumnValue>> {
215+
let Some(info) = table_info else {
216+
return Ok(Vec::new());
217+
};
218+
219+
if info.pk_columns.is_empty() {
220+
return Ok(Vec::new());
221+
}
222+
223+
// For DELETE, use old values; for INSERT/UPDATE, use new values
224+
let values = match event.operation {
225+
ChangeOperation::Delete => event.old_values.as_ref(),
226+
ChangeOperation::Insert | ChangeOperation::Update => event.new_values.as_ref(),
227+
};
228+
229+
let Some(values) = values else {
230+
return Ok(Vec::new());
231+
};
232+
233+
// Extract values at the PK column indices, erroring if any index is out of bounds
234+
let mut pk_values = Vec::with_capacity(info.pk_columns.len());
235+
for &idx in &info.pk_columns {
236+
match values.get(idx) {
237+
Some(v) => pk_values.push(v.clone().into()),
238+
None => {
239+
return Err(crate::Error::SchemaMismatch {
240+
table: event.table.clone(),
241+
expected: info.pk_columns.len(),
242+
actual: values.len(),
243+
});
244+
}
245+
}
181246
}
247+
Ok(pk_values)
182248
}
183249

184250
/// Converts SqliteValue vec to ColumnValue vec for TableChange.

crates/sqlx-sqlite-observer/src/change.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,32 @@ use std::time::Instant;
22

33
use crate::hooks::SqliteValue;
44

5+
/// Schema information for an observed table.
6+
///
7+
/// Used to extract primary key values and determine if rowid is meaningful.
8+
#[derive(Debug, Clone, Default)]
9+
pub struct TableInfo {
10+
/// Column indices that form the primary key, in declaration order.
11+
/// For `INTEGER PRIMARY KEY` tables, this contains the single PK column index.
12+
/// For composite PKs, indices are ordered as declared in the schema.
13+
pub pk_columns: Vec<usize>,
14+
/// True if the table was created with `WITHOUT ROWID`.
15+
/// For such tables, the preupdate hook's rowid parameter contains the first column
16+
/// of the PRIMARY KEY (coerced to i64), which may not be meaningful/correct for
17+
/// non-integer or composite primary keys.
18+
pub without_rowid: bool,
19+
}
20+
21+
impl TableInfo {
22+
/// Creates a new TableInfo with the given PK column indices.
23+
pub fn new(pk_columns: Vec<usize>, without_rowid: bool) -> Self {
24+
Self {
25+
pk_columns,
26+
without_rowid,
27+
}
28+
}
29+
}
30+
531
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
632
pub enum ChangeOperation {
733
Insert,
@@ -83,7 +109,14 @@ impl ColumnValue {
83109
pub struct TableChange {
84110
pub table: String,
85111
pub operation: Option<ChangeOperation>,
112+
/// The SQLite internal rowid. This is `None` for WITHOUT ROWID tables
113+
/// since the preupdate hook's rowid parameter is not meaningful for them.
86114
pub rowid: Option<i64>,
115+
/// The primary key value(s) for the affected row.
116+
/// For composite primary keys, values are ordered by their declaration order.
117+
/// For DELETE operations, this contains the old PK values.
118+
/// For INSERT/UPDATE operations, this contains the new PK values.
119+
pub primary_key: Vec<ColumnValue>,
87120
/// Column values before the change (for UPDATE and DELETE).
88121
/// Values are ordered by column index as defined in the table schema.
89122
pub old_values: Option<Vec<ColumnValue>>,

crates/sqlx-sqlite-observer/src/error.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,18 @@ pub enum Error {
66
/// Failed to register SQLite hooks.
77
#[error("Hook registration failed: {0}")]
88
HookRegistration(String),
9+
10+
/// SQLx database error.
11+
#[error("Database error: {0}")]
12+
Sqlx(#[from] sqlx::Error),
13+
14+
/// Schema mismatch - table schema changed while observing.
15+
#[error(
16+
"Schema mismatch for table '{table}': expected {expected} PK columns, but only {actual} values available"
17+
)]
18+
SchemaMismatch {
19+
table: String,
20+
expected: usize,
21+
actual: usize,
22+
},
923
}

crates/sqlx-sqlite-observer/src/hooks.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ struct HookContext {
106106
///
107107
/// # Example
108108
///
109-
/// ```rust
109+
/// ```no_run
110110
/// use sqlx_sqlite_observer::is_preupdate_hook_enabled;
111111
///
112112
/// if !is_preupdate_hook_enabled() {

crates/sqlx-sqlite-observer/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,10 @@ pub mod broker;
2727
pub mod change;
2828
pub mod error;
2929
pub mod hooks;
30+
pub mod schema;
3031

3132
pub use broker::ObservationBroker;
32-
pub use change::{ChangeOperation, ColumnValue, TableChange};
33+
pub use change::{ChangeOperation, ColumnValue, TableChange, TableInfo};
3334
pub use error::Error;
3435
pub use hooks::{SqliteValue, is_preupdate_hook_enabled};
3536

0 commit comments

Comments
 (0)