feat(database): iii-database worker (sqlite/postgres/mysql) + e2e harness#64
Merged
andersonleal merged 18 commits intomainfrom May 4, 2026
Merged
feat(database): iii-database worker (sqlite/postgres/mysql) + e2e harness#64andersonleal merged 18 commits intomainfrom
andersonleal merged 18 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (59)
📝 WalkthroughWalkthroughAdds a new iii-database worker (library + binary) implementing multi-dialect pools/drivers (Postgres/MySQL/SQLite), TLS, RPC handlers, triggers, prepared-handle registry, cursor persistence, config parsing, extensive tests and an E2E harness; plus CI workflows, gitignore updates, and release trigger additions. Changes
Sequence Diagram(s)sequenceDiagram
participant Engine as Engine/Client
participant Handler as RPC Handler
participant App as AppState (pools, handles)
participant Pool as Connection Pool
participant Driver as DB Driver
participant DB as Database
Engine->>Handler: RPC (db, sql, params)
Handler->>App: resolve pool by name
Handler->>Handler: validate SQL / convert params
Handler->>Pool: acquire connection
Pool->>DB: checkout / provide connection
Handler->>Driver: execute/query/transaction with params
Driver->>DB: bind params, run SQL
DB-->>Driver: rows / result
Driver-->>Handler: Query/Execute/Tx result or DbError
alt success
Handler->>Engine: return JSON response (rows / affected_rows / last_insert_id / tx results)
else failure
Handler->>Engine: return serialized DbError
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 52 minutes and 6 seconds.Comment |
Comment on lines
+131
to
+161
| fn register_function<F>( | ||
| iii: &iii_sdk::III, | ||
| state: &AppState, | ||
| name: &str, | ||
| description: &str, | ||
| handler: F, | ||
| ) where | ||
| F: for<'a> Fn(&'a AppState, serde_json::Value) -> HandlerFuture<'a> | ||
| + Send | ||
| + Sync | ||
| + Copy | ||
| + 'static, | ||
| { | ||
| let id = format!("iii-database::{name}"); | ||
| let state = state.clone(); | ||
| let id_for_msg = id.clone(); | ||
| let _ = iii.register_function_with( | ||
| RegisterFunctionMessage { | ||
| id: id.clone(), | ||
| description: Some(description.to_string()), | ||
| request_format: None, | ||
| response_format: None, | ||
| metadata: None, | ||
| invocation: None, | ||
| }, | ||
| move |payload: serde_json::Value| { | ||
| let state = state.clone(); | ||
| let id_for_msg = id_for_msg.clone(); | ||
| Box::pin(async move { | ||
| handler(&state, payload).await.map_err(|s| { | ||
| tracing::warn!(function = %id_for_msg, body = %s, "handler returned error"); |
Contributor
There was a problem hiding this comment.
we need to change this, we HAVE to use the register function with the type, otherwise we lose the API Reference
https://iii.dev/docs/api-reference/sdk-rust#register_function
use iii_sdk::{register_worker, InitOptions, RegisterFunction};
use serde::Deserialize;
use schemars::JsonSchema;
#[derive(Deserialize, JsonSchema)]
struct Input { name: String }
fn greet(input: Input) -> Result<String, String> {
Ok(format!("Hello, {}!", input.name))
}
let iii = register_worker("ws://localhost:49134", InitOptions::default());
iii.register_function(RegisterFunction::new("greet", greet));
Collaborator
Author
There was a problem hiding this comment.
Done in 3efe383. All 5 RPC functions now register via RegisterFunction::new_async with typed Req/Resp structs deriving schemars::JsonSchema, so request_format/response_format are auto-populated and the API Reference works.
- Wire format unchanged (verified: 126/126 unit + integration tests, including a new
tx_resp_skips_none_fields_on_wireinvariant test). - Added regression test
registered_functions_carry_request_and_response_schemasintests/integration.rsso the typed shape is locked in. - One behavioral diff: malformed payloads now surface raw serde "missing field …" errors via
IIIError::Handlerinstead of the customINVALID_PARAMenvelope — that mirrors every other typed Rust worker (the SDK does the deserialization now). E2e harness doesn't depend on the old envelope.
andersonleal
added a commit
that referenced
this pull request
May 1, 2026
…w_async Switch all 5 RPC handlers (query, execute, prepareStatement, runStatement, transaction) from the untyped register_function_with(RegisterFunctionMessage, |Value|) path to RegisterFunction::new_async(id, fn) with typed Req/Resp structs deriving serde::Deserialize/Serialize + schemars::JsonSchema. This makes the SDK auto-generate request_format/response_format JSON Schemas for every function, restoring the API Reference at iii.dev/docs that the untyped path was leaving empty. Wire format is preserved: function IDs, response key sets, and the optional results/failed_index/error fields on transaction responses are tested with a dedicated unit test (tx_resp_skips_none_fields_on_wire). Handler-boundary deserialization moves from each handler body into the SDK; the explicit INVALID_PARAM envelope for malformed payloads is replaced by IIIError::Handler(serde_error) consistent with every other typed Rust worker. Inline tests keep the missing-field contract by exercising serde_json::from_value::<QueryReq> directly. Adds a regression test in tests/integration.rs asserting every Req/Resp type carries both request_format and response_format so a future refactor back to the untyped API would fail at test time, not in production docs. Closes the only review comment on PR #64.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (2)
.gitignore (1)
12-12: ⚡ Quick winRecheck the lockfile unignore exception against repo-wide policy.
Line 12 explicitly re-allows tracking a worker lockfile, which reintroduces the merge-conflict/churn risk the repo policy was avoiding. If this exception is intentional, please document the policy change; otherwise remove the unignore.
Suggested change
-!iii-database/tests/e2e/workers/harness/package-lock.jsonBased on learnings: In the iii-hq/workers repository, it is intentional repo policy to NOT commit Node package manager lockfiles because they cause cross-worker merge conflicts and rotting pins without deploy-path reproducibility value.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 12, The .gitignore currently contains an explicit unignore for "iii-database/tests/e2e/workers/harness/package-lock.json" which reintroduces lockfile churn against repo policy; either remove that unignore entry from .gitignore or, if intentional, add a clear documented exception (e.g., in the repository README or a CONTRIBUTING.md section) describing why this specific package-lock.json must be committed and how it won't cause cross-worker merge conflicts; update references to the unignored path so the change is discoverable and consistent with the existing "no lockfiles" policy.iii-database/src/handlers/query.rs (1)
72-87: 💤 Low valueConsider logging when row values exceed column count.
The
if let Some(col)guard silently drops row values that exceed the column count. While this is defensive, it could mask bugs in driver implementations where column metadata doesn't match the actual row data.for (i, v) in row.0.into_iter().enumerate() { if let Some(col) = columns.get(i) { obj.insert(col.name.clone(), v.into_json()); + } else { + tracing::warn!(row_index = i, "row value index exceeds column count"); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@iii-database/src/handlers/query.rs` around lines 72 - 87, The rows_to_objects function currently silently drops extra values when a Row has more entries than the columns slice; add a runtime check at the start of iterating each row in rows_to_objects to detect when row.0.len() > columns.len() and emit a warning (using the project's logging crate, e.g., log::warn! or tracing::warn!) that includes the counts and some identifying context (e.g., ColumnMeta names or the Row debug/partial contents) so driver metadata/row length mismatches are visible; keep the existing defensive behavior of using columns.get(i) so behavior is unchanged except for the new warning when extra values are present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/iii-database-e2e.yml:
- Around line 46-50: The CI step titled "Install iii engine (latest from main)"
is nondeterministic because it pipes the installer from .../iii/main/install.sh
directly to sh; change the invocation so the installer is pinned to a specific
immutable release by passing the version argument to the installer (use the curl
... | sh -s -- vX.Y.Z pattern), keeping the same retry flags and the echo
"$HOME/.local/bin" >> "$GITHUB_PATH" line; replace vX.Y.Z with the chosen
release tag.
In `@iii-database/config.yaml.example`:
- Around line 1-10: The example YAML places databases under workers[].config but
the WorkerConfig root schema expects databases at the top level, so
deserialization will yield an empty databases set; move the "databases:" block
out of the "workers:" / "config:" nesting to the root of the example (preserving
the primary.url and pool keys and their values), or provide a separate
iii-config.yaml example that keeps the workers[] wrapper; ensure the top-level
key name is exactly "databases" to match WorkerConfig so startup sees the
configured DBs.
In `@iii-database/README.md`:
- Around line 116-130: The README claims the iii-database::row-change trigger
creates publications/replication slots at startup, but the actual code registers
the trigger as UNSUPPORTED (see
iii-database/tests/e2e/workers/harness/src/cases-row-change.ts); update the
iii-database::row-change section and the v1.0.0 NOTE to mark the trigger as
unsupported (not "setup-only") and remove or change any statements that assert
the worker currently creates iii_slot_<sanitized>_<8hex> or
iii_pub_<sanitized>_<8hex> (or that operators must drop those objects); instead
document that logical replication/dispatch is planned and that no side effects
are currently performed, and mention how operators will be able to override
slot_name/publication_name when the feature is implemented.
In `@iii-database/src/driver/mysql.rs`:
- Around line 21-39: The timeout only wraps conn.exec_iter(...) but not
result.collect(), so collection can hang; change the code to wrap the entire
operation (exec_iter plus column extraction and result.collect()) in a single
future passed to tokio::time::timeout (e.g., build an async block that calls
conn.exec_iter(sql, bound), maps errors with map_err, computes columns from
result.columns_ref(), then calls result.collect().await and returns both cols
and raw_rows) and then .await the timeout on that combined future instead of
timing just the exec_iter future so timeout_ms applies to row collection as
well.
In `@iii-database/src/driver/postgres.rs`:
- Around line 9-10: The code is decoding PostgreSQL "timestamp without time
zone" as chrono::DateTime<Utc>, which causes runtime type errors; change the
imports to use chrono::NaiveDateTime (replace DateTime, Utc with NaiveDateTime)
and update the decoding/FromSql logic (e.g., in the functions referenced around
the use line and the block around lines 183-186) to accept/return NaiveDateTime;
if you need a UTC DateTime elsewhere, explicitly convert the NaiveDateTime to
DateTime<Utc> using DateTime::<Utc>::from_utc(naive, Utc) at the call site.
- Around line 191-198: The NUMERIC arm in the match (T::NUMERIC) is trying to
read a String directly and will fail; update decoding to either request a
text-cast in the query or read into a Decimal type that implements FromSql:
modify the code that handles T::NUMERIC (the match arm labeled T::NUMERIC in
postgres.rs) to one of two options—(A) ensure the SQL that produced this row
casts the column to text (e.g., column::text) so you can safely call
row.try_get::<_, Option<String>>(idx) and return RowValue::Decimal(s), or (B)
change the runtime decoding to use a supported numeric type by calling
row.try_get::<_, Option<rust_decimal::Decimal>>(idx) (or bigdecimal::BigDecimal)
and return RowValue::Decimal from that Decimal; implement the chosen path
consistently with map_err and RowValue variants.
In `@iii-database/src/driver/sqlite.rs`:
- Line 140: The current raw-text checks for row-producing SQL (e.g.,
has_returning = !returning.is_empty() || sql.to_ascii_uppercase().contains("
RETURNING ")) are too brittle; replace them with a shared helper (e.g.,
statement_returns_rows(sql: &str, returning: &[..], conn: &Connection) -> bool)
used by execute() and transaction(), that returns true if the prepared statement
produces result columns (for example by preparing the SQL with the SQLite API
and checking the column count / result metadata, or by honoring an explicit
returning list), and then use that helper instead of starts_with("SELECT") /
contains(" RETURNING ") in the places that set has_returning (and the logic at
the other locations around the 324-326 logic) so both execute() and
transaction() consistently route row-producing statements to the
read/row-handling path.
- Around line 319-323: The transaction() implementation must enforce the same
single-statement guard as execute(): before binding/executing each TxStatement
(refer to TxStatement.sql and the loop in transaction()), validate the SQL
string does not contain semicolon-separated statements using the same helper
used by execute() (or replicate its check) and return an error that includes the
offending step index if the guard fails; do this check once per statement before
calling json_param_to_sql and before executing to ensure multi-statement SQL
like "INSERT ...; DELETE ..." is rejected up front and attributed to the correct
step.
In `@iii-database/src/handlers/prepare.rs`:
- Around line 113-127: The test prepare_clamps_ttl_to_max currently only checks
resp.handle.id and can miss a missing TTL clamp; update the test to assert that
resp.handle.expires_at is approximately Utc::now() + MAX_TTL_SECONDS (not the
provided 999_999), e.g. compute now = Utc::now(), expected_max = now +
Duration::seconds(MAX_TTL_SECONDS as i64) and then assert expires_at <=
expected_max + small_slack (few seconds) and expires_at >= now +
minimal_expected (or simply > now) so the test fails if TTL was ignored;
reference resp.handle.expires_at, MAX_TTL_SECONDS, prepare_clamps_ttl_to_max and
use chrono::Utc()/Duration to compute the bounds.
In `@iii-database/src/handlers/transaction.rs`:
- Around line 91-104: The current Err(e) branch coerces missing failed_index
into 0 which incorrectly signals statement index 0; instead preserve None for
non-step failures. Change the local failed_index to an Option<usize> by matching
crate::error::DbError::DriverError { failed_index, .. } => failed_index.clone(),
and for the _ case return None; then pass that Option directly into
TxResp.failed_index (rather than wrapping a numeric 0 into Some(0)). Ensure
types line up with TxResp.failed_index being Option<usize>.
In `@iii-database/src/main.rs`:
- Around line 141-146: Current shutdown logic only awaits
tokio::signal::ctrl_c() so SIGTERM never triggers cleanup; modify the shutdown
path to listen for both SIGINT and SIGTERM (e.g., using tokio::signal::ctrl_c()
and tokio::signal::unix::signal(SignalKind::terminate()) combined with
tokio::select! or futures::future::select) and call iii.shutdown_async() when
either signal is received, preserving the tracing::info log messages for startup
and shutdown; update code that references tokio::signal::ctrl_c(),
tracing::info!(... waiting for invocations), and iii.shutdown_async()
accordingly.
- Around line 43-45: The code currently logs the full engine URL from cli.url in
the tracing::info! call (with Cli::parse() and iii_database::worker_name());
parse cli.url with a URL parser (e.g., url::Url) and produce a redacted string
that strips userinfo and sensitive query params (or formats only host:port) and
then pass that redacted value to tracing::info! instead of %cli.url so secrets
are not emitted to logs.
In `@iii-database/src/pool/sqlite.rs`:
- Around line 74-79: The current match on res maps all r2d2 errors to
DbError::PoolTimeout; change the Err arm to inspect the error (e) returned by
Pool::get_timeout()/get(), e.g., if e.source().is_none() then return
DbError::PoolTimeout { db: db_name, waited_ms: timeout.as_millis() as u64 } else
return DbError::DriverError with driver = "sqlite", message = format!("pool
acquire error: {e}"), code = None and failed_index = None; update the Err(_) arm
in the match that constructs SqliteConn to use Err(e) and branch on e.source()
accordingly.
In `@iii-database/src/triggers/handler.rs`:
- Around line 146-152: The spawn happens before the new task is visible in the
registry so unregister_trigger can miss a just-spawned poller; make the
spawn+registration atomic from unregister_trigger's POV by ensuring you either
(a) acquire the same lock used to protect tasks and by_trigger_id, insert a
placeholder/registration for the instance into tasks/by_trigger_id, then spawn
the tokio task and replace the placeholder with the actual JoinHandle, or (b)
record/mark the instance as “registered” in the registry (tasks/by_trigger_id)
under the lock before calling tokio::spawn and only spawn after that mark is
persisted; apply the same change to the other spawn block that calls
query_poll::run_loop (the block around run_loop and the similar block at lines
165-180) so unregister_trigger always sees the new poller.
In `@iii-database/src/triggers/row_change.rs`:
- Around line 153-178: RowChangeConfig::validate currently accepts an empty
tables list which later produces invalid SQL; update validate() to check if
self.tables.is_empty() and return Err(DbError::ConfigError { message: "<clear
message>" }) when empty (e.g., "row-change tables: list must not be empty") so
callers like ensure_publication_and_slot() get a clear config error; keep this
check before the loop that validates each table and use the same cfg_err helper
for the returned error message.
In `@iii-database/tests/e2e/workers/harness/src/worker.ts`:
- Around line 10-18: Move all startup initialization that can throw
(registerWorker, new Runner, and iii.registerFunction) into the existing guarded
IIFE with the try/catch that writes the HARNESS_DONE sentinel so startup
failures are reported the same as runtime failures; specifically,
create/register the worker (registerWorker(URL)), instantiate Runner, and call
iii.registerFunction('harness::on_outbox_row', ...) inside the IIFE’s try block
and let the catch write the HARNESS_DONE sentinel and log the error, ensuring no
throwing initialization occurs before the guarded block that run-tests.sh
depends on.
In `@iii-database/tests/integration.rs`:
- Around line 137-180: The test
registered_functions_carry_request_and_response_schemas currently only validates
schema derivation via RegisterFunction::new_async stubs; update it to assert the
real worker registrations instead by either (a) calling the actual registration
helper used in production (the code path that uses register_function_with or the
central registration function) and then iterating the produced registration list
to check request_format()/response_format(), or (b) refactor the production
registration code into a helper that returns the Vec/registry of
RegisterFunction entries that both the worker bootstrap and this test can call;
replace the assert_schemas invocations with checks against that real
registration list so the test fails if production wiring switches back to
register_function_with or omits schemas.
---
Nitpick comments:
In @.gitignore:
- Line 12: The .gitignore currently contains an explicit unignore for
"iii-database/tests/e2e/workers/harness/package-lock.json" which reintroduces
lockfile churn against repo policy; either remove that unignore entry from
.gitignore or, if intentional, add a clear documented exception (e.g., in the
repository README or a CONTRIBUTING.md section) describing why this specific
package-lock.json must be committed and how it won't cause cross-worker merge
conflicts; update references to the unignored path so the change is discoverable
and consistent with the existing "no lockfiles" policy.
In `@iii-database/src/handlers/query.rs`:
- Around line 72-87: The rows_to_objects function currently silently drops extra
values when a Row has more entries than the columns slice; add a runtime check
at the start of iterating each row in rows_to_objects to detect when row.0.len()
> columns.len() and emit a warning (using the project's logging crate, e.g.,
log::warn! or tracing::warn!) that includes the counts and some identifying
context (e.g., ColumnMeta names or the Row debug/partial contents) so driver
metadata/row length mismatches are visible; keep the existing defensive behavior
of using columns.get(i) so behavior is unchanged except for the new warning when
extra values are present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c98b9b6b-b332-4cee-b6f1-2e92d02c686d
⛔ Files ignored due to path filters (2)
iii-database/Cargo.lockis excluded by!**/*.lockiii-database/tests/e2e/workers/harness/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (59)
.github/workflows/create-tag.yml.github/workflows/iii-database-e2e.yml.github/workflows/release.yml.gitignoreiii-database/.gitignoreiii-database/Cargo.tomliii-database/README.mdiii-database/config.yaml.exampleiii-database/iii.worker.yamliii-database/src/config.rsiii-database/src/cursor.rsiii-database/src/driver/mod.rsiii-database/src/driver/mysql.rsiii-database/src/driver/postgres.rsiii-database/src/driver/sqlite.rsiii-database/src/error.rsiii-database/src/handle.rsiii-database/src/handlers/execute.rsiii-database/src/handlers/mod.rsiii-database/src/handlers/prepare.rsiii-database/src/handlers/query.rsiii-database/src/handlers/run_statement.rsiii-database/src/handlers/transaction.rsiii-database/src/lib.rsiii-database/src/main.rsiii-database/src/pool/mod.rsiii-database/src/pool/mysql.rsiii-database/src/pool/postgres.rsiii-database/src/pool/sqlite.rsiii-database/src/pool/tls.rsiii-database/src/triggers/handler.rsiii-database/src/triggers/mod.rsiii-database/src/triggers/query_poll.rsiii-database/src/triggers/row_change.rsiii-database/src/value.rsiii-database/tests/e2e/.gitignoreiii-database/tests/e2e/README.mdiii-database/tests/e2e/config.yamliii-database/tests/e2e/data/.gitkeepiii-database/tests/e2e/docker-compose.ymliii-database/tests/e2e/reports/.gitkeepiii-database/tests/e2e/run-tests.shiii-database/tests/e2e/workers/harness/iii.worker.yamliii-database/tests/e2e/workers/harness/package.jsoniii-database/tests/e2e/workers/harness/src/cases-boundary.tsiii-database/tests/e2e/workers/harness/src/cases-concurrency.tsiii-database/tests/e2e/workers/harness/src/cases-protocol.tsiii-database/tests/e2e/workers/harness/src/cases-row-change.tsiii-database/tests/e2e/workers/harness/src/cases-transaction.tsiii-database/tests/e2e/workers/harness/src/cases-trigger.tsiii-database/tests/e2e/workers/harness/src/cases.tsiii-database/tests/e2e/workers/harness/src/dialect.test.tsiii-database/tests/e2e/workers/harness/src/dialect.tsiii-database/tests/e2e/workers/harness/src/runner.tsiii-database/tests/e2e/workers/harness/src/test.tsiii-database/tests/e2e/workers/harness/src/worker.tsiii-database/tests/e2e/workers/harness/tsconfig.jsoniii-database/tests/integration.rsiii-database/tests/value_coercion.rs
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@iii-database/src/driver/postgres.rs`:
- Around line 201-211: The current NUMERIC match uses
get!(rust_decimal::Decimal) which can error for out-of-range PostgreSQL NUMERICs
and lets decode errors propagate; wrap that decode attempt to catch/handle
errors: try decoding to rust_decimal via get!(rust_decimal::Decimal) and on
success return RowValue::Decimal(d.to_string()), but if the decode returns Err
or None then attempt to retrieve the value as a String (e.g., get!(String)) and
return RowValue::Decimal(the_string) to preserve the wire representation; if the
String retrieval also fails or is None, fall back to RowValue::Null. Update the
T::NUMERIC arm (the get!(rust_decimal::Decimal) usage and resulting
RowValue::Decimal/Null) accordingly to implement this safe fallback.
In `@iii-database/src/driver/sqlite.rs`:
- Around line 324-330: The loop loses the step index context when a failure
occurs inside row_value_at; update the inner error handling so any error
returned by row_value_at(i) is converted with step_err(idx, e) (or otherwise
sets failed_index = Some(idx)) before bubbling up. Concretely, in the block that
builds vals (the for loop using row_value_at and pushing into vals) wrap/map the
result of row_value_at(row, i) to call step_err(idx, e) on Err so the propagated
error preserves the failed_index like the outer rows.next().map_err(...). Ensure
row_value_at, step_err, rows.next(), vals and rows_out.push(Row(vals)) are the
locations you change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9c161507-8226-4f0d-95b1-81171fb7277b
⛔ Files ignored due to path filters (1)
iii-database/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
iii-database/Cargo.tomliii-database/src/driver/mysql.rsiii-database/src/driver/postgres.rsiii-database/src/driver/sqlite.rsiii-database/tests/e2e/workers/harness/src/cases-boundary.tsiii-database/tests/e2e/workers/harness/src/cases-transaction.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- iii-database/tests/e2e/workers/harness/src/cases-transaction.ts
- iii-database/Cargo.toml
Binary worker connecting to PostgreSQL, MySQL, and SQLite. Exposes
five RPC functions (query, execute, prepareStatement, runStatement,
transaction) and two trigger types (query-poll, row-change). Full
contract in database/README.md.
Driver layer
- Three impls under src/driver/ sharing dispatch types in driver/mod.rs.
- Per-driver pools: deadpool-postgres, mysql_async, r2d2-rusqlite.
- TLS for postgres + mysql via rustls 0.23 with libpq-aligned modes
(disable | require | verify-full); ca_cert replaces system trust;
CryptoProvider installed at startup so mysql TLS errors fast-fail
instead of hanging the engine for 30s.
Triggers
- query-poll: cursor persisted in __iii_cursors, single-pass cursor
max (i64 hot path is alloc-free), ack-gated commit. ensure_table
cached per (driver, table) to avoid per-tick DDL round-trips.
- row-change: idempotent slot+publication setup. Names are sanitize+
FNV-1a-32(trigger_id) so distinct trigger_ids never share a slot
even when their sanitized form collides. Streaming decode stubbed
pending tokio-postgres replication API release.
Safety
- Pool acquire errors redacted so connection-URL credentials cannot
leak into JSON IIIError bodies.
- Strict SQL-identifier validation on every operator-controlled name
that flows into format!()'d SQL (cursor table/column, schema, slot,
publication, table list).
- User values always bind through prepared-statement parameters.
Performance
- RowValue::into_json moves String/Vec/Json payloads instead of
cloning; rows_to_objects consumes Vec<Row> end-to-end.
- SQLite run_prepared uses an Option<SqliteConn> slot to skip the
throwaway-pool round-trip on prepared-handle execution.
- query-poll cursor max is single-pass with zero per-row allocations
on the integer hot path; falls back to Cow<&str> lex max for
non-integer cursors.
Errors
- Stable code envelope: POOL_TIMEOUT, QUERY_TIMEOUT, STATEMENT_NOT_FOUND,
UNKNOWN_DB, INVALID_PARAM, DRIVER_ERROR, REPLICATION_SLOT_EXISTS,
UNSUPPORTED, CONFIG_ERROR, with structured failed_index on tx errors.
Public surface kept minimal: only iii_database::pool::{Pool, build}
and the handler/trigger entry points are pub. Per-driver pool types
and pool::tls helpers are pub(crate).
Validated by 119 unit tests + 5 integration tests in the crate, plus
a 90-test external harness at tmp/iii-database-tests covering all
three drivers (boundary values, NULL/UTF-8, transactions, prepared
statements, query-poll dispatch, row-change slot derivation, JSONB
round-trip, pool exhaustion, concurrency).
The harness previously lived outside the workers repo at an absolute path.
Vendoring it under database/tests/e2e/ lets CI exercise the worker against
real Postgres 16, MySQL 8.4, and SQLite on every PR that touches database/**.
Layout (database/tests/e2e/):
run-tests.sh # orchestrator, env-overridable paths
docker-compose.yml # postgres (wal_level=logical) + mysql
config.yaml # engine config (iii-database + harness)
workers/harness/ # tsx smoke worker (~90 assertions)
Workflow (.github/workflows/database-e2e.yml):
- triggers on database/** and the workflow file itself
- installs the engine via curl-pipe-sh from install.iii.dev/iii/main
so CI always runs against the latest engine without a version pin
- cargo builds the worker, brings up the docker stack, runs run-tests.sh
- uploads reports/ as an artifact on failure
run-tests.sh paths are env-overridable (WORKER_SRC, III_BIN,
WORKER_BIN_TARGET, WORKER_BIN_LINK) with defaults that resolve relative to
the script's own location, so the script works locally and in CI without
patching.
GHA services blocks can't pass -c wal_level=logical to postgres, which
row-change tests require, so the workflow shells out to docker compose for
dev/CI parity.
Side fixes:
- database/.gitignore: anchor 'data/' to '/data/' so it no longer
shadows tests/e2e/data/.gitkeep
- .gitignore: re-include database/tests/e2e/workers/harness/package-lock.json
so npm cache keying works in CI
Three small fixes from the eng review of the e2e harness: 1. Use `docker compose up -d --wait` instead of polling `compose ps --format json` and grep'ing for `"Health":"..."`. compose v2's --wait blocks until healthchecks pass and exits non-zero on failure — kills 19 lines of fragile JSON regex. 2. Probe TCP 127.0.0.1:49134 directly instead of grep'ing the engine log for "Engine listening on address: 0.0.0.0:49134". The log-grep silently broke us as a 30s timeout the next time the engine team touches that line; TCP is the actual signal. 3. `curl --retry 3 --retry-connrefused --retry-delay 5` on the engine install step so a transient install.iii.dev hiccup doesn't redden CI. Verified with a full local harness run: 90/90 PASS in 36s.
Adds `database` to the worker choice list in create-tag.yml and the `database/v*` tag pattern to release.yml so `database/v1.0.0` triggers the standard binary-build pipeline (matching image-resize, conductor, etc). Without this, `iii worker add iii-database@1.0.0` from the spec README has no published artifact to resolve. The release pipeline is fully data-driven from database/iii.worker.yaml (language=rust, deploy=binary, manifest=Cargo.toml, bin=iii-database), so no other plumbing is needed.
…tions
The 5 inline test sites in src/{driver,pool}/{postgres,mysql}.rs and
src/triggers/row_change.rs early-return when TEST_POSTGRES_URL or
TEST_MYSQL_URL is unset. They've been skipping silently in CI because
ci.yml's `cargo test` runs without a docker stack.
Activate them in the e2e workflow:
- new `--with-cargo-test` flag in run-tests.sh runs `cargo test
--all-features` between compose-healthy and the harness step, with
TEST_*_URL set to the docker stack's postgres + mysql
- database-e2e.yml passes the flag
Activating the tests immediately surfaced two over-specific assertions
in src/driver/mysql.rs that asserted RowValue::Int|BigInt(42) for
`SELECT ? + ?` — but MySQL types parameter-only arithmetic as
MYSQL_TYPE_DOUBLE, so the decoder correctly returns RowValue::Float(42.0).
Loosened the assertions to accept any numeric variant equal to 42 (the
test asserts "positional params bind correctly", not "integer type
preservation"). Decoder is unchanged; the tests were the bug.
Verified: full chain now runs 119 unit/integration tests + 90 e2e
assertions in 30.3s locally.
Comments that restate what the code already says, reference task history
("post-fix", "previously this was..."), or just label sequential steps
add noise without payoff. Removed 44 such lines across 12 files.
Kept everything load-bearing: sticky last_insert_id rationale, TLS
redaction reasoning, the compute_cursor_max two-pass doc-string with
its explicit "lexicographic max" gotcha, the row-change v1.0 stub
note pointing at the upstream tokio-postgres dep, etc.
Verified: cargo test (119) + e2e harness (90) PASS.
Refactored multi-line expressions into single-line equivalents to enhance readability and maintain consistency. Updated function signatures, match cases, and error handling in multiple files, including `tls.rs`, `cursor.rs`, `mysql.rs`, and others.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@iii-database/src/driver/sqlite.rs`:
- Around line 12-17: Add the same single-statement guard used by execute() and
transaction() to both query() and run_prepared(): before calling prepare() (and
before feeding SQL into any sqlite prepare/prepare_v2 path), validate the SQL
contains only a single statement using the existing helper (or replicate that
check) and return the same DbError (or multi-statement rejection) used by
execute()/transaction() when multiple statements are detected; ensure you
perform this check in the query() function and in run_prepared() so prepare()
never receives multi-statement SQL that would silently truncate.
In `@iii-database/src/pool/sqlite.rs`:
- Around line 37-45: Detect when the code chooses
SqliteConnectionManager::memory() (i.e., when path == ":memory:" or
path.starts_with(":memory:")) and prevent multi-connection pooling by validating
pool_cfg.max: if pool_cfg.max != 1 then return an error (or set pool_cfg.max =
1) before calling R2Pool::builder().max_size(...); alternatively, for true
shared memory behavior replace the path with the shared-URI
"file::memory:?cache=shared" and ensure the SqliteConnectionManager is created
in URI mode, then allow >1 connections—update the branch where manager is
selected and the subsequent R2Pool::builder() call to enforce one of these fixes
and surface a clear error message referencing path,
SqliteConnectionManager::memory(), and pool_cfg.max.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac90a183-b084-4e6d-b61e-28668a9ac6fa
📒 Files selected for processing (7)
iii-database/src/driver/postgres.rsiii-database/src/driver/sqlite.rsiii-database/src/handlers/transaction.rsiii-database/src/main.rsiii-database/src/pool/sqlite.rsiii-database/src/triggers/handler.rsiii-database/tests/e2e/workers/harness/src/cases-boundary.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- iii-database/src/main.rs
- iii-database/src/handlers/transaction.rs
- iii-database/src/triggers/handler.rs
- iii-database/tests/e2e/workers/harness/src/cases-boundary.ts
- iii-database/src/driver/postgres.rs
The folder rename in f4774b6 left behind 8 references to the old path that would have broken CI on this PR: - release.yml: tag pattern database/v* -> iii-database/v* - create-tag.yml: choice option database -> iii-database - database-e2e.yml renamed to iii-database-e2e.yml + 6 internal refs (paths filter, concurrency group, Swatinem cache key, npm cache path, working-directory, artifact name + path) - .gitignore: re-include exception for the harness package-lock.json - README + run-tests.sh: stale path references Also restores iii-database/tests/e2e/workers/harness/package-lock.json which the rename + the old .gitignore exception combined to drop from tracking (the exception no longer matched after the move, so the global `package-lock.json` ignore rule started catching it). Verified: full e2e harness PASS (119 cargo tests + 90 e2e assertions) from the new path in 49s.
…w_async Switch all 5 RPC handlers (query, execute, prepareStatement, runStatement, transaction) from the untyped register_function_with(RegisterFunctionMessage, |Value|) path to RegisterFunction::new_async(id, fn) with typed Req/Resp structs deriving serde::Deserialize/Serialize + schemars::JsonSchema. This makes the SDK auto-generate request_format/response_format JSON Schemas for every function, restoring the API Reference at iii.dev/docs that the untyped path was leaving empty. Wire format is preserved: function IDs, response key sets, and the optional results/failed_index/error fields on transaction responses are tested with a dedicated unit test (tx_resp_skips_none_fields_on_wire). Handler-boundary deserialization moves from each handler body into the SDK; the explicit INVALID_PARAM envelope for malformed payloads is replaced by IIIError::Handler(serde_error) consistent with every other typed Rust worker. Inline tests keep the missing-field contract by exercising serde_json::from_value::<QueryReq> directly. Adds a regression test in tests/integration.rs asserting every Req/Resp type carries both request_format and response_format so a future refactor back to the untyped API would fail at test time, not in production docs. Closes the only review comment on PR #64.
…t exec_iter mysql_async's `conn.exec_iter(sql, bound).await` resolves once the server returns column metadata; the row payload is streamed lazily by the subsequent `result.collect().await`. The previous code wrapped only `exec_iter` in `tokio::time::timeout`, so a query whose planner is fast but whose row stream is slow (per-row SLEEP, slow scan, large result set on a saturated link) silently bypassed `timeout_ms` for the entire row read — the call could hang for minutes despite a multi-second timeout. Wrap the entire prepare→column-read→collect pipeline in a single future so `timeout_ms` covers the whole operation. Postgres is unaffected (`client.query` materializes Vec<Row> before resolving) and SQLite is unaffected (sync work in spawn_blocking). Adds a gated unit regression test (`my_query_timeout_applies_to_row_streaming`) that issues `SELECT SLEEP(2), n FROM (1,2,3)` with `timeout_ms=500` and asserts both the QUERY_TIMEOUT error and that elapsed < 3s — the buggy code waited ~6s and returned rows. Adds the same shape as an e2e case in cases-boundary.ts gated to mysql_db. run_prepared and transaction have the same lazy-collect shape but accept no timeout argument today; that's a separate pre-existing gap.
…me<Utc>
postgres-types' chrono FromSql impls bind by exact Postgres OID (see
postgres-types-0.2/src/chrono_04.rs):
impl FromSql for DateTime<Utc> { accepts!(TIMESTAMPTZ); }
impl FromSql for NaiveDateTime { accepts!(TIMESTAMP); }
The driver's row-to-cell mapper at pg_cell_to_row_value collapsed both
TIMESTAMP and TIMESTAMPTZ into the same `get!(DateTime<Utc>)` arm, so any
SELECT touching a TIMESTAMP WITHOUT TIME ZONE column (very common with
unannotated CREATE TABLE columns) failed at runtime with WrongType — every
query/transaction/run_prepared/execute-RETURNING path that landed on a
TIMESTAMP column rejected the call.
Split the arm: TIMESTAMPTZ keeps the DateTime<Utc> decode; TIMESTAMP now
decodes as NaiveDateTime and is folded into RowValue::Timestamp by treating
the naive value as UTC via Utc.from_utc_datetime(&naive). The wire format
is unchanged — both column types serialize as the same RFC 3339 UTC string,
so existing clients see no diff aside from queries that previously errored
now succeeding.
Adds a gated unit regression test (pg_query_decodes_timestamp_without_tz_and_with_tz)
that selects both column shapes in one query and asserts both round-trip,
plus an e2e case in cases-boundary.ts gated to pg_db.
`String: FromSql::accepts` in postgres-types-0.2 (lib.rs:729) is gated to
VARCHAR/TEXT/BPCHAR/NAME/UNKNOWN/citext-family OIDs — it rejects NUMERIC.
The previous arm `try_get::<_, Option<String>>(idx)` for a NUMERIC column
failed at runtime with WrongType, so every SELECT/RETURNING/transaction/
runStatement path that touched a NUMERIC column rejected the whole call.
The accompanying code comment ("read as String") was wrong about how
postgres-types FromSql resolution works.
Add `rust_decimal = { version = "1", features = ["db-tokio-postgres"] }`
which registers FromSql/ToSql for NUMERIC, and decode the column as
`rust_decimal::Decimal`, then stringify into RowValue::Decimal so the
wire format remains a precision-preserving decimal string.
Trade-off: rust_decimal is 96-bit (~28 significant digits). NUMERIC
values exceeding that precision now fail loud at decode rather than
silently truncating — strictly better than the pre-fix state where every
NUMERIC value failed regardless of size.
Adds a gated unit regression test (pg_query_decodes_numeric_to_string)
exercising positive/negative/fractional/zero values and an e2e case in
cases-boundary.ts gated to pg_db.
The sqlite driver's transaction step router used brittle prefix/substring
matching to decide whether each statement should go to the row-capture
path or `c.execute()`:
let is_returning = upper.contains(" RETURNING ");
let is_select = upper.trim_start().starts_with("SELECT");
if is_select || is_returning { /* capture rows */ } else { /* execute */ }
This mis-classified every legitimate row-producing statement that didn't
start with `SELECT` or contain ` RETURNING ` literally:
- `WITH cte AS (...) SELECT ...` — starts with WITH, not SELECT
- `VALUES (1), (2), (3)` — neither SELECT nor RETURNING
- `EXPLAIN QUERY PLAN ...` — produces rows
- `PRAGMA foreign_keys` — produces rows
- `INSERT ... \n RETURNING \n id, n` — RETURNING with no flanking spaces
All of these fell through to `c.execute(...)` which errors with
ExecuteReturnedResults; the transaction's `step_err` then aborted the
whole transaction with a misleading driver error.
Replace the heuristic with a shared helper:
fn statement_returns_rows(stmt: &Statement, returning: &[String]) -> bool {
!returning.is_empty() || stmt.column_count() > 0
}
`Statement::column_count()` is SQLite's planner-level source of truth: it
returns > 0 for every row-producing statement shape regardless of
keyword/casing/whitespace, and 0 for pure side-effect statements.
Apply the helper in both call sites:
- run_tx_steps(): always prepare first, route via the helper. Bug fix.
- execute(): prepare first, route via the helper. Removes the brittle
` RETURNING ` text check and the ExecuteReturnedResults fallback path
the old code needed for SELECT-via-execute. Behavior change:
SELECT-via-execute now surfaces the rows on `returned_rows` instead of
silently dropping them — strictly more useful, and preserves the
cross-driver invariant that `execute(SELECT)` does not throw.
Tests:
- transaction_handles_cte_select_values_and_multiline_returning exercises
CTE-prefixed SELECT, VALUES, and multi-line INSERT...RETURNING in a
single transaction — all four step types previously aborted the tx.
- execute_with_select_does_not_throw_and_surfaces_rows updates the
pre-existing pin to assert the new "rows are surfaced" behavior.
- E2e regression case in cases-transaction.ts gated to sqlite_db.
Total: 130 unit + integration tests PASS, clippy + fmt clean.
…edaction, error classification, race)
Six independently-confirmed real bugs. Each is fixed with a regression
test (where unit-testable) and inline documentation.
1) sqlite::transaction lacked execute()'s single-statement guard. Each
TxStatement.sql ran through rusqlite's prepare_v2 which silently parses
only the first statement, so `INSERT ...; DELETE ...` would commit a
partial transaction (just the INSERT) without diagnostic. Now rejects
up-front with code=MULTI_STATEMENT and failed_index=<step idx>.
Test: transaction_rejects_multi_statement_in_step.
2) handlers::transaction's failed_index extraction coerced
DbError::DriverError{failed_index: None} and all non-DriverError
variants (PoolTimeout, UnknownDb, ConfigError, …) into Some(0),
falsely attributing every connection-level failure to "statement 0".
Extracted to failed_index_of() which preserves None for non-step
errors. TxResp.failed_index stays None on the wire when absent
(skip_serializing_if already set).
Test: failed_index_extraction_preserves_none_for_non_step_errors.
3) main.rs logged the raw engine URL via `url = %cli.url`. Operators
passing `wss://user:secret@host` would leak credentials to the
structured log. Added redact_url() that strips userinfo via the
url crate; falls back to the original string on parse failure so
logging never panics.
Test: redact_url_strips_userinfo_only.
4) Shutdown only awaited tokio::signal::ctrl_c() (SIGINT). Docker
`docker stop` / k8s `kubectl delete` send SIGTERM, which bypassed
iii.shutdown_async() entirely — the engine connection would dangle
until the process was killed. Added wait_for_shutdown_signal()
that selects on ctrl_c() and SignalKind::terminate() on Unix
(no-op on non-Unix). Manual verification only — tokio::signal
primitives don't mock cleanly.
5) pool/sqlite.rs collapsed every r2d2 acquire failure into
DbError::PoolTimeout, even when r2d2 returned a wrapped
"unable to open database file" or similar from the connection
manager. Operators staring at PoolTimeout would scale the pool
forever while the actual fix was a path/perms issue. Note: the
reviewer's suggested e.source().is_none() check is a no-op against
r2d2-0.8.10 (verified at lib.rs:567 — Error doesn't implement
source()). Distinguish via r2d2's Display string instead: pure
timeout writes "timed out waiting for connection", connection-
error case writes "timed out waiting for connection: <inner>".
classify_acquire_error() takes the formatted message so it's
unit-testable without constructing private r2d2::Error values.
Tests: classify_acquire_error_with_inner_reason_returns_driver_error,
classify_acquire_error_pure_timeout_returns_pool_timeout.
6) triggers::handler::QueryPollTrigger spawned the polling task before
acquiring the registry locks, with the comment claiming "the task
body never touches self.tasks". That's true, but a concurrent
unregister_trigger(config.id) could fire between tokio::spawn and
the lock acquisition, find tasks empty, return Ok, and the newly-
spawned poller would leak running forever (no abort handle, no
index entry). Moved tokio::spawn inside the locked section so
"task spawned" and "task in registry" are atomic from the unregister
path's POV. tokio::spawn returns synchronously so the lock window
only grows by microseconds. Race-test infrastructure would require
mocking iii_sdk::III; relying on inline documentation + code review.
Total: 135 unit + integration tests PASS, clippy + fmt clean.
…w decode
Two independently-confirmed real bugs.
1) postgres.rs NUMERIC arm propagated rust_decimal decode errors. rust_decimal
(1.41.0/src/postgres/driver.rs:91, 109) returns Err for special signs
(NaN, +Infinity, -Infinity) and for values exceeding its 96-bit range
(~28 sig digits). The previous arm did:
match get!(rust_decimal::Decimal) { Some -> Decimal, None -> Null }
where get! propagates the inner error via `?`, so a single edge-case
value in a result set failed the entire query.
Layered fix:
a) Try rust_decimal::Decimal — fast path, handles 99% of values.
b) On Err: fall back to a custom binary parser (PgNumericText newtype)
that decodes the documented Postgres NUMERIC binary format. Handles
NaN, ±Infinity, and arbitrary precision.
c) On second-stage Err: log warn, surface RowValue::Null. Strict
improvement over "fail the whole query".
The reviewer's literal suggestion of `get!(String)` as the fallback
doesn't work — `String: FromSql::accepts` is gated to TEXT-family OIDs
(postgres-types-0.2/src/lib.rs:729) and rejects NUMERIC at runtime.
Verified earlier in this branch. Custom parser sidesteps that by
accepting NUMERIC directly.
Tests: 8 unit tests for stringify_pg_numeric_binary covering zero,
zero-with-scale, simple fraction, sub-unit fraction, negative,
beyond-rust_decimal magnitude (10^30), all three special signs,
and truncated-buffer rejection. Drives the parser with crafted
bytes so no live postgres needed.
2) sqlite.rs run_tx_steps lost failed_index when row_value_at errored.
row_value_at constructs DbError via map_err which sets
failed_index: None — it has no step context. The line
vals.push(row_value_at(row, i)?);
propagated that None verbatim, so any cell-decode failure during a
tx step surfaced as "tx failed" with no step attribution on the wire.
Added with_failed_index helper that stamps the current step idx onto
a None failed_index while preserving any existing index (an inner
step's attribution wins). Applied at the row_value_at call site.
Tests: 3 unit tests for with_failed_index covering: stamp-when-None,
preserve-existing, pass-through-non-DriverError.
Total: 146 unit + integration tests PASS (was 135, +11 new), clippy +
fmt clean.
Adds the missing live-postgres regression test for the layered NUMERIC decode fix in 4b72f0e. The previous commit had unit tests for the binary parser (driven with crafted bytes — proves correctness) and for with_failed_index (proves stamping logic), but no test that exercised the production wiring `try_get<rust_decimal> → Err → try_get<PgNumericText>` end-to-end against a real postgres connection. Two new tests: - pg_query_falls_back_to_binary_parser_for_numeric_edge_cases (Rust integration test, gated on TEST_POSTGRES_URL): SELECTs NaN, +Inf, -Inf, and 10^29 — all values rust_decimal::Decimal::from_sql returns Err for. Pre-fix, this query failed entirely. Post-fix, each value surfaces as RowValue::Decimal(<correct string>) via the PgNumericText fallback path. - 'NUMERIC edge cases route through binary fallback (postgres)' in cases-boundary.ts (e2e harness, applies: ['pg_db']): same shape, exercised through the worker's wire protocol so the postgres CI job catches any regression in the layered routing. with_failed_index already had 3 unit tests in the prior commit (stamp-when-None, preserve-existing, pass-through-non-DriverError). A live sqlite test that triggers a row_value_at error mid-tx is not practical — rusqlite handles all five storage classes cleanly and get_ref doesn't fail on data we successfully stored. The unit tests on the helper are the practical coverage for that path. Total: 147 unit + integration tests PASS, clippy + fmt clean.
andersonleal
force-pushed
the
feat/database-worker
branch
from
128ffca to
3739d89
Compare
sergiofilhowz
approved these changes
May 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces the
iii-databaseworker — a Rust binary that exposes SQL (Postgres / MySQL / SQLite) over the iii bus.query,execute,prepareStatement,runStatement,transaction(withread_committed/repeatable_read/serializableisolation)query-poll(cursor-durable via__iii_cursorstable, survives restart),row-change(slot/publication name derivation done; pgoutput streaming stubbed pending upstreamtokio-postgresreplication API — registers cleanly withUNSUPPORTED)sqlx(deadpool-postgres + tokio-postgres, mysql_async, r2d2-rusqlite)disable | require | insecuremodes, chain validation defaultdatabase/tests/e2e/, wired to a new GHA workflow ondatabase/**PRscreate-tag+releaseworkflows for the 4 advertised targetsTest plan
cargo test --all-features— 119 unit + integration tests, all PASS--with-cargo-test) — 0 regressions, surfaced + fixed 2 over-specific assertions indriver/mysql.rscargo fmt --checkcleancargo clippy --all-targets --all-features -- -D warningscleanci.yml+database-e2e.yml)Spec coverage
Implemented against the v1.0 spec at
database-worker.md. Coverage: 100% of functions, drivers, error codes, config shape. One stub:row-changestreaming decode (waits on upstream Rust crate; explicitly documented). One distribution gap closed in this PR:iii-databaseregistered in release workflows.Summary by CodeRabbit
New Features
Documentation
Tests
Chores