feat: SQLite + PostgreSQL + MariaDB + ODBC behind a pluggable backend-ops registry

[Admnwk]

Summary

This folds the optional SQL table-driver backends — SQLite, PostgreSQL, MariaDB and a generic ODBC driver — behind the ACE ABI into a single buildable line, routed through a small pluggable backend-ops registry so the per-backend dispatch no longer multiplies across the ~17 navigation / field ABI functions.

It resolves the pairwise merge conflict between the separate backend PRs (#22 / #23 / #24): each adds its own if (get_X_table(h)) { … } block to the same 17 Ads* functions, so combining N backends produces ~17×N conflicting hunks in ace_exports.cpp. The registry confines each backend to one ops struct + one registration line; the 17 ABI functions become backend-agnostic, and the native / tcp:// remote path stays the unchanged fall-through.

Mechanism

A BackendTableOps vtable (17 function pointers mirroring the table ops) is registered per HandleKind. Each ABI function does one backend_table_ops_for(h) lookup and dispatches if non-null, else runs the unchanged native/remote body. Each backend lifts its dispatch into <backend>_<op>() functions exposed via <backend>_table_ops() and registered once in register_builtin_backends() (the only place with per-backend #if).

What's here (stacked)

This branch is stacked on the not-yet-merged base PRs, so the diff includes them:

• SQLite table driver (feat: optional SQLite-backed table driver behind the ACE ABI #18) + SQL passthrough (feat: SQL passthrough — run SQL directly against the SQLite backend (builds on #18) #21)
• ABI remote-write fixes (fix(abi): AdsSetDouble/AdsSetLogical honor remote tables (tcp:// navigational writes) #28)
• the backend-ops registry
• PostgreSQL, MariaDB and generic ODBC drivers, each lifted onto the registry

Verification (MSVC x64, all backends ON together)

• Existing unit suite: every prior case green — behavior-preserving refactor, no test edits.
• PostgreSQL e2e: 41/41 assertions vs a live server.
• MariaDB e2e: 45/45 assertions vs a live server.
• ODBC e2e: 59/59 assertions vs a throwaway Microsoft Access fixture.
• All four backends link into one DLL; the native local DBF/ADT and native client/server paths do not register ops and run unchanged.

Adding a further backend (Firebird, or MSSQL/Oracle via ODBC) is one ops struct + one registration line.

[gemini-code-assist]

Code Review

This pull request refactors the table dispatch mechanism by introducing a pluggable backend-ops registry, moving inline SQL backend dispatch blocks (SQLite, PostgreSQL, MariaDB, and ODBC) into dedicated, backend-specific helper functions. It also introduces openmonitor, a Rust-based terminal and web monitor for the openads_serverd server. The review feedback highlights several critical issues: potential null pointer dereferences in the lifted SQLite helper functions, an unexpanded CMake variable for PostgreSQL include directories, terminal restoration bypass on TUI errors, blocking calls on the Tokio thread pool in the web backend, and potential TUI freezes from blocking TCP connection attempts.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

The refactored sqlite_goto_top function (and other lifted functions like sqlite_goto_bottom, sqlite_skip, etc.) dereferences st via st->conn without checking if st is nullptr. If get_sqlite_table returns nullptr (e.g., due to an invalid or closed handle), this will cause an immediate segmentation fault / crash. A null check must be added before accessing any members of st.

UNSIGNED32 sqlite_goto_top(ADSHANDLE hTable) {
    auto* st = get_sqlite_table(hTable);
    if (st == nullptr) return fail(openads::AE_TABLE_NOT_OPEN, "table not open");
    if (st->conn == nullptr) return fail(openads::AE_INVALID_CONNECTION_HANDLE, "");
    auto r = st->conn->goto_top(st);
    if (!r) return fail(r.error());
    return ok();
}

[gemini-code-assist]

The variable PostgreSQL_INCLUDE_DIRS is passed as a raw string literal to target_include_directories instead of being expanded. This will cause CMake to look for a literal directory named PostgreSQL_INCLUDE_DIRS rather than the actual PostgreSQL include path, leading to compilation failures when find_package(PostgreSQL) is used.

        target_include_directories(openads_libpq INTERFACE "${PostgreSQL_INCLUDE_DIRS}")

[gemini-code-assist]

The sqlite_field_index helper function dereferences st immediately via !st->fields_cached without verifying if st is nullptr. Since get_sqlite_table can return nullptr, this helper must guard against null pointers to prevent crashes in metadata-related ABI functions.

std::size_t sqlite_field_index(openads::sql_backend::SqliteTable* st,
                               UNSIGNED8* pucField) {
    if (st == nullptr) {
        return std::numeric_limits<std::size_t>::max();
    }
    if (!st->fields_cached) {

[gemini-code-assist]

If an error occurs inside the main TUI loop (e.g., during term.draw or event::poll), run_tui will return early via the ? operator. This bypasses the terminal restoration calls (disable_raw_mode, LeaveAlternateScreen, and ratatui::restore), leaving the user's terminal in a broken/unusable state. Wrapping the loop in a closure or block ensures that terminal cleanup is always executed before returning.

    let res = (|| -> Result<()> {
        let tick = Duration::from_secs(cfg.interval_secs);
        let mut needs_redraw = true;

        loop {
            if needs_redraw || app.last_refresh.elapsed() >= tick {
                app.refresh(cfg);
                needs_redraw = true;
            }

            if needs_redraw {
                term.draw(|f| ui(f, app, cfg, args))?;
                needs_redraw = false;
            }

            if event::poll(Duration::from_millis(200))? {
                if let Event::Key(key) = event::read()? {
                    if key.kind != KeyEventKind::Press {
                        continue;
                    }
                    match key.code {
                        KeyCode::Char('q') | KeyCode::Esc => break,
                        KeyCode::Char('r') => {
                            app.refresh(cfg);
                            app.status_msg = "refreshed".into();
                            needs_redraw = true;
                        }
                        KeyCode::Up => {
                            app.selected_session = app.selected_session.saturating_sub(1);
                            needs_redraw = true;
                        }
                        KeyCode::Down => {
                            if !app.inner.http.sessions.is_empty() {
                                app.selected_session =
                                    (app.selected_session + 1).min(app.inner.http.sessions.len() - 1);
                            }
                            needs_redraw = true;
                        }
                        KeyCode::Char('g') => {
                            app.show_graphs = !app.show_graphs;
                            app.status_msg = if app.show_graphs {
                                "graphs on".into()
                            } else {
                                "graphs off".into()
                            };
                            needs_redraw = true;
                        }
                        KeyCode::Char('k') => {
                            if let (Some(base), Some(sess)) = 
                                (&cfg.http, app.inner.http.sessions.get(app.selected_session))
                            { 
                                let client = HttpClient::new(base);
                                match client.kill_session(sess.id) {
                                    Ok(()) => {
                                        app.status_msg = format!("killed session {}", sess.id);
                                        app.refresh(cfg);
                                    }
                                    Err(e) => app.status_msg = format!("kill failed: {}", e),
                                }
                                needs_redraw = true;
                            } else {
                                app.status_msg = "kill needs --http and a selected session".into();
                                needs_redraw = true;
                            }
                        }
                        _ => {}
                    }
                }
            }
        }
        Ok(())
    })();

    disable_raw_mode()?;
    stdout().execute(LeaveAlternateScreen)?;
    ratatui::restore();
    res

[gemini-code-assist]

Calling the blocking HttpClient::kill_session directly inside the asynchronous api_kill_session handler blocks the Tokio worker thread. This can degrade the performance of the entire web server. It should be wrapped in tokio::task::spawn_blocking to run on a dedicated thread pool.

    let base_clone = base.clone();
    let kill_res = tokio::task::spawn_blocking(move || {
        HttpClient::new(&base_clone).kill_session(id)
    })
    .await;

    match kill_res {
        Ok(Ok(())) => {
            if let Ok(mut state) = st.poll.write() {
                state.refresh(&st.cfg);
                if let Ok(mut out) = st.snap.write() {
                    *out = state.to_snapshot(&st.cfg);
                }
            }
            StatusCode::OK.into_response()
        }
        Ok(Err(e)) => (StatusCode::BAD_GATEWAY, e.to_string()).into_response(),
        Err(e) => (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response(),
    }

[gemini-code-assist]

TcpStream::connect blocks indefinitely (or up to the OS default timeout of 20+ seconds) if the host is unreachable or firewalled. Since this is a TUI application that runs on a single thread, this will freeze the interface. Using TcpStream::connect_timeout with a short timeout (e.g., 3 seconds) prevents long freezes.

Suggested change

	pub fn fetch_mg_snapshot(host: &str, port: u16) -> Result<MgSnapshot> {
	let addr = format!("{host}:{port}");
	let mut stream = TcpStream::connect(&addr).with_context(|| format!("connect {addr}"))?;
	stream.set_read_timeout(Some(Duration::from_secs(5)))?;
	pub fn fetch_mg_snapshot(host: &str, port: u16) -> Result<MgSnapshot> {
	use std::net::ToSocketAddrs;
	let addr = format!("{host}:{port}");
	let socket_addr = addr
	.to_socket_addrs()
	.context("resolve address")?
	.next()
	.ok_or_else(|| anyhow!("failed to resolve address"))?;
	let mut stream = TcpStream::connect_timeout(&socket_addr, Duration::from_secs(3))
	.with_context(|| format!("connect {addr}"))?;
	stream.set_read_timeout(Some(Duration::from_secs(5)))?;

[Admnwk]

Thanks for the review. Going through the findings:

CMakeLists.txt:221 (high) — fixed in 248d2a1: the include-dirs variable is now expanded (${PostgreSQL_INCLUDE_DIRS}). It only affected the find_package(PostgreSQL) fallback; the explicit OPENADS_LIBPQ_* path used by the e2e build was unaffected, which is why it built green.

ace_exports.cpp:674 / :321 (sqlite_goto_top, sqlite_field_index, etc.) — not a reachable crash. These lifted helpers are reached only through the backend-ops registry: an ABI function calls ops->X(h) only after backend_table_ops_for(h) resolved the handle's HandleKind to SqliteTable. get_sqlite_table(h) then looks the same handle up under that exact kind, so it cannot return null on this path — the old if (auto* st = get_sqlite_table(h)) guard was doing the kind-filter that the registry now performs up front. A null here would mean the registry returned ops for a kind whose object pointer is null (an internal invariant break), not a reachable input. Glad to add a defensive guard if you'd prefer belt-and-suspenders.

openmonitor/*.rs (main.rs:238, web.rs:108, wire.rs:195) — these are in the standalone openmonitor Rust tool, not the backend-ops registry change this PR is about; it rode in via the branch base. Valid points, but they belong to that tool's own track rather than this refactor.