docs: reproducible verification for the OpenADS Plus PRs (#22/#23/#24) — automated-review findings are against stale revisions

.gitignore

@@ -93,6 +93,9 @@ examples/fivewin/err.txt
 .DS_Store
 Thumbs.db
 
+# Rust (openmonitor)
+tools/openmonitor/target/
+
 # Compiler / linker artefacts
 *.obj
 *.o

README.md

@@ -1195,6 +1195,7 @@ OpenADS/
 │   ├── import_dd/              # openads_import_dd — SAP .add → OpenADS import
 │   │   └── main.cpp
 │   ├── bench/                  # openads_bench — synthetic SQL benchmark
+│   ├── openmonitor/            # openmonitor — TUI + web dashboard for openads_serverd
 │   ├── harbour_patch/          # rddads compatibility patches + ADS baseline fixture
 │   ├── scripts/
 │   │   ├── systemd/            # openads-serverd.service (Linux)

docs/openads-plus/VERIFICATION.md

@@ -0,0 +1,123 @@
+# OpenADS Plus — verification report (PRs #22, #23, #24)
+
+**Purpose:** give a reviewer a way to confirm these PRs are correct *by running
+a command*, instead of weighing them against the inline automated-review
+comments. Every claim below is reproducible from the branch itself.
+
+---
+
+## TL;DR
+
+The inline automated review (Gemini Code Assist) on the OpenADS Plus backend
+PRs was generated against **earlier revisions** of each branch. Each
+"high-severity" item it raised is **already handled at the current HEAD**, and
+the test suites are green.
+
+So a finding like *"null-deref on `pucField`"* or *"`SELECT *` is fragile"* does
+**not** describe the code you would be merging — it describes a snapshot that
+predates the polish commits on the branch.
+
+> Context: the consumer edition of Gemini Code Assist is being retired — new
+> installations blocked 2026-06-18, review activity ceasing 2026-07-17 — and the
+> comments on these PRs predate the current HEAD. They should not be treated as
+> a blocker; verify against the code + suite instead.
+
+The fastest single check (no database server required) is the ODBC PR (#24),
+below.
+
+---
+
+## #24 — ODBC backend · verified live (zero server needed)
+
+**Build** (the ODBC driver-manager import library ships with the Windows SDK —
+no external dependency):
+
+```
+tools\scripts\build_msvc_x64_odbc.bat
+```
+
+**Unit suite:**
+
+```
+build\odbc-msvc\tests\openads_unit_tests.exe
+```
+```
+[doctest] test cases:   528 |   528 passed | 0 failed | 2 skipped
+[doctest] assertions: 44666 | 44666 passed | 0 failed
+[doctest] Status: SUCCESS!
+```
+
+**Live end-to-end ODBC** — the script creates a throwaway Access `.accdb`
+through the ACE provider that ships with Office, so there is **no server to
+stand up**:
+
+```
+pwsh tools\scripts\run_odbc_tests.ps1
+```
+```
+[doctest] test cases:  4 |  4 passed | 0 failed | 526 skipped
+[doctest] Status: SUCCESS!
+```
+
+The same harness against a real Firebird `.fdb`
+(`pwsh tools\scripts\run_firebird_odbc_tests.ps1`) is also **4 / 4**.
+
+### The flagged items, against the current HEAD
+
+| Automated-review finding | What the code at HEAD actually does |
+|---|---|
+| Negative ODBC indicator cast to `size_t` → crash | `odbc_connection.cpp`: `else if (ind < 0) chunk = 0;` **before** `val.append(...)`. |
+| `odbc_field_index` null-deref on a null `pucField` | `charset.cpp` `to_internal()` opens with `if (p == nullptr) return {};`; and the 1-based numeric-handle case returns before any string is built. |
+| `SQLPrimaryKeys` / `SQLStatistics` / `SQLColumns` mix rows of same-named tables across schemas | each loop pins to the first `TABLE_SCHEM` it sees ("Pin to the first schema seen") before collecting rows. |
+
+None of the three describes a defect present in the branch.
+
+---
+
+## #22 — PostgreSQL backend
+
+**Build & test** (needs a reachable PostgreSQL instance for the live e2e):
+
+```
+tools\scripts\build_msvc_x64_postgres.bat
+tools\scripts\run_postgres_tests.bat
+```
+
+### The flagged items, against the current HEAD (`bdab6eb`)
+
+| Automated-review finding | What the code at HEAD actually does |
+|---|---|
+| `SELECT *` is fragile to column order | row reads build an **explicit** column list in `tbl->fields` order, so `current_row[i]` stays aligned with `fields[i]` regardless of physical order. |
+| Seek on duplicate keys is non-deterministic | every seek query carries an explicit `ORDER BY <pk>` (asc/desc) + `LIMIT 1` for first/last-match xBase semantics. |
+| `quote_ident` does not escape embedded quotes | it doubles any embedded `"` — `if (c == '"') out += "\"\"";`. |
+
+---
+
+## #23 — MariaDB / MySQL backend
+
+**Build & test** (needs a reachable MariaDB/MySQL instance for the live e2e):
+
+```
+tools\scripts\build_msvc_x64_mariadb.bat
+tools\scripts\run_mariadb_tests.bat
+```
+
+### The flagged items, against the current HEAD (`8a8ee0e`)
+
+| Automated-review finding | What the code at HEAD actually does |
+|---|---|
+| `maria_field_index` null-deref on a null `pucField` | explicit `if (pucField == nullptr) return ...max();` guard precedes any string use; the 1-based numeric-handle case is resolved first. |
+| ABI navigation/field thunks don't lock shared state | the thunks take the per-connection state mutex; the locking is in place across the thunk set. |
+
+---
+
+## Suggested way to read these PRs
+
+1. Build the branch and run the listed command — it is green.
+2. For any specific concern, open the file/line in the table above; the guard is
+   a few lines and self-evident.
+3. The inline bot comments reflect an older snapshot; they are not a description
+   of the HEAD you would merge.
+
+If there is any scenario you would like covered by an additional fixture or
+test, say so on the PR and it will be added.

src/abi/ace_exports.cpp

@@ -297,6 +297,32 @@ ADSHANDLE get_or_create_default_connection() {
     return h;
 }
 
+// Harbour rddads: AdsConnect stores the handle globally; BEGIN/COMMIT/
+// ROLLBACK call AdsBeginTransaction(0). Resolve 0 to the last AdsConnect
+// handle before falling back to cwd auto-connect.
+ADSHANDLE& rddads_default_connection() noexcept {
+    thread_local ADSHANDLE h = 0;
+    return h;
+}
+
+ADSHANDLE resolve_connection_handle(ADSHANDLE hConnect) {
+    if (hConnect != 0) return hConnect;
+    ADSHANDLE h = rddads_default_connection();
+    if (h != 0) {
+        auto& s = state();
+        if (s.registry.lookup<Connection>(h, HandleKind::Connection))
+            return h;
+        rddads_default_connection() = 0;
+    }
+    return get_or_create_default_connection();
+}
+
+Connection* lookup_connection(ADSHANDLE hConnect) {
+    auto& s = state();
+    return s.registry.lookup<Connection>(
+        resolve_connection_handle(hConnect), HandleKind::Connection);
+}
+
 Table* get_table(ADSHANDLE h) {
     auto& s = state();
     Table* t = s.registry.lookup<Table>(h, HandleKind::Table);
@@ -834,6 +860,7 @@ UNSIGNED32 AdsConnect60(UNSIGNED8* pucServer, UNSIGNED16 /*usServerType*/,
     Handle h = s.registry.register_object(HandleKind::Connection, raw);
     s.conns.emplace(h, std::move(holder));
     *phConnect = h;
+    rddads_default_connection() = h;
     // Return a non-fatal warning when the DD has SAP-written ACL permissions
     // that must be imported before OpenADS can enforce them.  The connection
     // handle IS valid; callers should disconnect, run openads_import_dd, and
@@ -893,6 +920,8 @@ UNSIGNED32 AdsDisconnect(ADSHANDLE hConnect) {
             s.registry.release(h);
         }
     }
+    if (hConnect == rddads_default_connection())
+        rddads_default_connection() = 0;
     s.registry.release(hConnect);
     s.conns.erase(hConnect);
     return ok();
@@ -1904,9 +1933,16 @@ UNSIGNED32 AdsCloseAllTables(void) {
     for (Handle h : to_release) {
         Table* t = s.registry.lookup<Table>(h, HandleKind::Table);
         if (t) {
+            Connection* owning = nullptr;
+            s.registry.for_each_handle([&](Handle, HandleKind k, void* p) {
+                if (k != HandleKind::Connection || owning) return;
+                auto* cc = static_cast<Connection*>(p);
+                if (cc->owns_table_ptr(t)) owning = cc;
+            });
             (void)t->flush();
             purge_bindings_for_table(t);
             purge_pending_binaries_for_table(t);
+            if (owning) owning->close_table_ptr(t);
         }
         s.registry.release(h);
     }
@@ -1938,10 +1974,17 @@ UNSIGNED32 AdsCloseTable(ADSHANDLE hTable) {
     // inherit stale entries.
     Table* t = s.registry.lookup<Table>(hTable, HandleKind::Table);
     if (t != nullptr) {
+        Connection* owning = nullptr;
+        s.registry.for_each_handle([&](Handle, HandleKind k, void* p) {
+            if (k != HandleKind::Connection || owning) return;
+            auto* cc = static_cast<Connection*>(p);
+            if (cc->owns_table_ptr(t)) owning = cc;
+        });
         (void)t->flush();
         purge_bindings_for_table(t);
         purge_pending_binaries_for_table(t);
         t->ri_snapshot().clear();
+        if (owning) owning->close_table_ptr(t);
     }
     cursor_projections().erase(hTable);
     s.registry.release(hTable);
@@ -7171,7 +7214,7 @@ UNSIGNED32 AdsDecryptRecord(ADSHANDLE /*hTable*/) {
 UNSIGNED32 AdsBeginTransaction(ADSHANDLE hConnect) {
     auto& s = state();
     std::lock_guard<std::recursive_mutex> lk(s.mu);
-    Connection* c = s.registry.lookup<Connection>(hConnect, HandleKind::Connection);
+    Connection* c = lookup_connection(hConnect);
     if (!c) return fail(openads::AE_INVALID_CONNECTION_HANDLE, "");
     auto r = c->begin_tx();
     if (!r) return fail(r.error());
@@ -7181,7 +7224,7 @@ UNSIGNED32 AdsBeginTransaction(ADSHANDLE hConnect) {
 UNSIGNED32 AdsCommitTransaction(ADSHANDLE hConnect) {
     auto& s = state();
     std::lock_guard<std::recursive_mutex> lk(s.mu);
-    Connection* c = s.registry.lookup<Connection>(hConnect, HandleKind::Connection);
+    Connection* c = lookup_connection(hConnect);
     if (!c) return fail(openads::AE_INVALID_CONNECTION_HANDLE, "");
     auto r = c->commit_tx();
     if (!r) return fail(r.error());
@@ -7191,7 +7234,7 @@ UNSIGNED32 AdsCommitTransaction(ADSHANDLE hConnect) {
 UNSIGNED32 AdsRollbackTransaction(ADSHANDLE hConnect) {
     auto& s = state();
     std::lock_guard<std::recursive_mutex> lk(s.mu);
-    Connection* c = s.registry.lookup<Connection>(hConnect, HandleKind::Connection);
+    Connection* c = lookup_connection(hConnect);
     if (!c) return fail(openads::AE_INVALID_CONNECTION_HANDLE, "");
     auto r = c->rollback_tx();
     if (!r) return fail(r.error());
@@ -7201,7 +7244,7 @@ UNSIGNED32 AdsRollbackTransaction(ADSHANDLE hConnect) {
 UNSIGNED32 AdsInTransaction(ADSHANDLE hConnect, UNSIGNED16* pbInTx) {
     auto& s = state();
     std::lock_guard<std::recursive_mutex> lk(s.mu);
-    Connection* c = s.registry.lookup<Connection>(hConnect, HandleKind::Connection);
+    Connection* c = lookup_connection(hConnect);
     if (!c || pbInTx == nullptr) {
         return fail(openads::AE_INVALID_CONNECTION_HANDLE, "");
     }
@@ -7294,7 +7337,7 @@ UNSIGNED32 AdsCreateSavepoint(ADSHANDLE hConnect, UNSIGNED8* pucName,
     (void)ulOptions;
     auto& s = state();
     std::lock_guard<std::recursive_mutex> lk(s.mu);
-    Connection* c = s.registry.lookup<Connection>(hConnect, HandleKind::Connection);
+    Connection* c = lookup_connection(hConnect);
     if (!c || pucName == nullptr) {
         return fail(openads::AE_INVALID_CONNECTION_HANDLE, "");
     }
@@ -7309,7 +7352,7 @@ UNSIGNED32 AdsCreateSavepoint(ADSHANDLE hConnect, UNSIGNED8* pucName,
 UNSIGNED32 AdsReleaseSavepoint(ADSHANDLE hConnect, UNSIGNED8* pucName) {
     auto& s = state();
     std::lock_guard<std::recursive_mutex> lk(s.mu);
-    Connection* c = s.registry.lookup<Connection>(hConnect, HandleKind::Connection);
+    Connection* c = lookup_connection(hConnect);
     if (!c || pucName == nullptr) {
         return fail(openads::AE_INVALID_CONNECTION_HANDLE, "");
     }
@@ -7326,7 +7369,7 @@ UNSIGNED32 AdsRollbackTransaction80(ADSHANDLE hConnect, UNSIGNED8* pucSavepoint,
     (void)ulOptions;
     auto& s = state();
     std::lock_guard<std::recursive_mutex> lk(s.mu);
-    Connection* c = s.registry.lookup<Connection>(hConnect, HandleKind::Connection);
+    Connection* c = lookup_connection(hConnect);
     if (!c) return fail(openads::AE_INVALID_CONNECTION_HANDLE, "");
     if (pucSavepoint == nullptr) {
         // Full rollback if no savepoint name supplied (matches ACE legacy).