Observer re-queries schema for nonexistent observed tables on every writer acquisition

[jjhafer]

ObservationBroker's lazy TableInfo fill never converges for a table name that
isn't in the schema. query_table_info() returns Ok(None), and the Ok(None)
arm of ensure_table_info only logs — set_table_info() is never called
(crates/sqlx-sqlite-observer/src/conn_mgr.rs:188-200,
crates/sqlx-sqlite-observer/src/schema.rs:30-32). The name therefore stays in
the "needs querying" set for the broker's lifetime and costs 2 SQL round trips
on every acquire_writer().

This is amplified by two things:

• ensure_table_info runs on every writer acquisition
  (conn_mgr.rs:161), and it runs while holding the max-1 write connection
  (crates/sqlx-sqlite-conn-mgr/src/database.rs:192-193, checked out at :286),
  so every writer in the app serializes behind it — not just the observing caller.
• Nothing removes an individual table from the observed set; it is cleared only on
  full teardown (close/remove/disable_observation/last unobserve()).

Measured on release build, warm, steady state per write: 42.9µs with one real
table, 1.48ms at 100 nonexistent tables (~34x), 13.7ms at 1000 (~320x) — linear
in the accumulated set. The cost is per acquire_writer(), not per statement, so
a caller batching via execute_transaction pays once; the plugin's execute IPC
command pays per call.

It does not require hostile input: one typo'd table name, or a table dropped by a
later migration, is enough. observe() caps a single call at 100 tables, but
subscribe() has no table-count cap at all and also merges its tables in via
subscribe_stream → observe_tables, so a cap on observe() alone would not
close it.

Pre-existing. PR #55 touches neither conn_mgr.rs nor schema.rs. What #55
changed is that enable_observation() no longer tears the broker down, and that
teardown was the only incremental reset of the observed set — it was also
precisely the #54 bug. So #55 removed an accidental mitigation of this separate
defect; it did not introduce it.

Proposed fix

1. Add tables_pending_info: RwLock<HashSet<String>> to ObservationBroker,
   populated by observe_table/observe_tables and drained by set_table_info.
   Have ensure_table_info scan that set instead of get_observed_tables().
   Steady state becomes one RwLock read plus an is_empty() early return,
   rather than cloning every observed table name and taking one lock read per
   table per acquisition. This is a win even with zero nonexistent tables.
2. Gate the pending scan on PRAGMA schema_version, cached on the broker: when
   the pending set is non-empty, spend 1 statement and skip the scan entirely if
   the version is unchanged. Any DDL bumps it, so the legitimate
   "observe, then create the table" ordering resolves on the next write after the
   DDL with no need to re-observe(). Worst case becomes 1 statement per write
   instead of 2N.

Explicitly rejected alternatives

• Validating table existence in observe()/subscribe() and erroring.
  Observing before migrations run is legitimate.
• Caching "absent" with re-attempt only on the next explicit
  observe()/subscribe() naming that table. Unsafe: hooks.rs:238-239 gates
  event delivery on is_table_observed alone, so a table with no resolved
  TableInfo still delivers change events — but with an empty primary_key, and
  a meaningless rowid for WITHOUT ROWID tables. Caching absence would make
  that silent correctness degradation permanent for a table created later.

Tests

In crates/sqlx-sqlite-toolkit/tests/observation_tests.rs:

• an observed table that does not exist is not re-queried while no DDL runs
• a late CREATE TABLE is picked up without re-calling observe()
• a WITHOUT ROWID table created after observe() reports rowid: None once resolved

---

Split out of the code review on #55, where it was validated as real but out of scope for that PR. See the review threads there for the supporting analysis.

Issue filed by AI model Claude