feat: add a documented mysqli subset over the elephc-pdo bridge

[nahime0]

Summary

Adds a documented mysqli subset as its own PHP surface over the existing elephc-pdo MySQL client. A mysqli-only program links elephc_pdo but never declares PDO / PDOStatement or throws PDOException.

The shared extern "elephc_pdo" block is injected at most once. extension_loaded('mysqli') / extension_loaded('PDO') follow the injected surface, not the archive. --with-mysqli force-injects the prelude (and links the bridge) without pulling in the PDO classes.

Surface

• mysqli / mysqli_stmt / mysqli_result / mysqli_sql_exception and the procedural mysqli_* aliases
• Connect (p: persistent, real_connect flags), escape without wrapping quotes, transactions, charset, ping
• Buffered query() / mysqli_result with independent result identity (data_seek, foreach)
• Prepared statements: bind_param, execute / execute($params), get_result, execute_query (8.2+)
• multi_query / next_result / store_result
• mysqli_report (8.0 default OFF, 8.1+ ERROR|STRICT)

Intentional divergences (documented)

• bind_param snapshots values at bind time (no cross-call reference aliasing); bind_result / fetch are not declared
• query() rejects multi-statement SQL client-side (errno 1064), including compound-body DDL — use multi_query()
• MYSQLI_USE_RESULT is still buffered; MYSQLI_CLIENT_SSL is rejected; insert_id is int; $info is empty
• Unconsumed result sets (including mid-multi_query) make later statements fail with 2014

Tests

• Offline codegen + error + extension_loaded matrix (mysqli vs PDO vs both vs --with-mysqli)
• Live MySQL fixtures (#[ignore], ELEPHC_MY_DSN) for fetch identity, statements, multi_query, reconnect, 2014, and the multi-statement scanner

[greptile-apps]

Greptile Summary

The PR adds a PHP-facing mysqli subset over the existing MySQL bridge while keeping mysqli and PDO surface detection independent.

• Adds mysqli connections, buffered results, prepared statements, multi-query handling, procedural aliases, and reporting modes.
• Extends the native MySQL bridge with mysqli-specific metadata, escaping, charset tracking, statement scanning, and result-lifecycle support.
• Adds compiler injection/linking integration, compatibility documentation, examples, and offline and live test coverage.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains from the previously reported issues.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/mysqli_prelude/connection.rs	Implements mysqli connection, query, transaction, pending-result, and DSN behavior; the previously reported DSN injection path is now rejected.
src/mysqli_prelude/statement.rs	Implements prepared-statement binding and buffered result access; the previously reported connection-busy concern is contradicted by the bridge’s retain-at-execute behavior.
crates/elephc-pdo/src/my.rs	Extends the MySQL bridge with mysqli metadata, escaping, charset, scanning, and buffered rowset behavior.
crates/elephc-pdo/src/lib.rs	Exposes the additional stable C ABI functions consumed by the mysqli prelude.
src/pipeline.rs	Integrates conditional mysqli prelude injection and independent PHP-surface reporting.
docs/php/mysqli.md	Documents the supported mysqli subset and its intentional behavioral divergences.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    PHP[PHP mysqli API] --> PRELUDE[mysqli prelude]
    PRELUDE --> ABI[elephc_pdo C ABI]
    ABI --> MYSQL[Pure-Rust MySQL client]
    DETECT[Usage detection or --with-mysqli] --> PRELUDE
    DETECT --> LINK[Link elephc_pdo]
    DETECT --> EXT[Report mysqli as loaded]
    PDO[PDO prelude] --> ABI
    PDO --> PDOEXT[Report PDO independently]

Loading

_{Reviews (14): Last reviewed commit: "test(mysqli): make the multi_query error..." | Re-trigger Greptile}

[Guikingone]

Review — mysqli subset over the elephc_pdo bridge

Really strong piece of work overall: the surface/archive split (php_extension: None + linked_php_surfaces) is the right call, the extern-block extraction is byte-for-byte lossless (I diffed all 167 signatures), the exhaustive no-wildcard detect.rs walk is the correct discipline, and docs/php/mysqli.md is unusually honest about its own divergences. The notes below are what survived an adversarial pass.

Method. Every claim marked ✅ proven was verified by executing real PHP 8.5.6 with the real mysqli extension against a purpose-built fake MySQL server that logs every COM_QUERY packet — so the "what php actually does" side is captured wire traffic, not recollection. The findings were then put to a three-model jury (GLM 5.2, Kimi K2.7, Kimi K3 via Ollama) asked to refute them: all three confirmed every item except one, where they split — the bind_param argument-error class — settled against php-src's own source. Four of the jury's own additions survived execution and are folded in below (credited inline); three did not and were dropped, noted here so nobody re-litigates them: real_escape_string under NO_BACKSLASH_ESCAPES does not need to double " (php doesn't either), real_connect()'s reconnect path does already reset the multi-query state (via multiClose()), and the --\v case is a false-positive risk, not a bypass.

---

Blockers

1. commit($flags, $name) never commits — silent data loss

connection.rs treats the $name argument of begin_transaction / commit / rollback as a savepoint name. In php-src it is a SQL comment. Captured from real php:

$db->begin_transaction(0, "sp1");   →  START TRANSACTION /*sp1*/
$db->savepoint("sp2");              →  SAVEPOINT `sp2`
$db->commit(0, "sp1");              →  COMMIT /*sp1*/
$db->rollback(0, "sp1");            →  ROLLBACK /*sp1*/
$db->release_savepoint("sp2");      →  RELEASE SAVEPOINT `sp2`

php has separate savepoint() / release_savepoint() methods (and mysqli_savepoint / mysqli_release_savepoint) precisely because $name is not a savepoint — this PR implements neither. ✅ proven

What the PR emits instead:

call	php	this PR	consequence
begin_transaction(0,"tx")	START TRANSACTION /*tx*/	BEGIN + SAVEPOINT \tx``	extra round-trip
commit(0,"tx")	COMMIT /*tx*/	RELEASE SAVEPOINT \tx``	transaction is never committed
rollback(0,"tx")	ROLLBACK /*tx*/	ROLLBACK TO \tx``	partial rollback, transaction stays open

The commit path is the dangerous one, and it fails two different ways:

• Paired with a named begin — RELEASE SAVEPOINT succeeds, commit() returns true, the transaction is still open, and close() / __destruct() (connection.rs, which rolls back an open transaction on the way out) silently discards the writes. Success return, no error state, no data.
• Paired with an unnamed begin — no such savepoint exists, MySQL raises 1305, opFailed() fires, and commit() returns false / throws under MYSQLI_REPORT_STRICT for a perfectly valid php program.

Note the live test at tests/codegen/mysqli_mysql.rs:542 pins this: begin_transaction(0,"sp1") then rollback(0,"sp1") then rollback() returns true under both the correct and the incorrect semantics, so it cannot detect the bug.

Fix: fold $name into a comment (COMMIT /*name*/, rejecting */ in the name), and add savepoint() / release_savepoint() + their procedural aliases for the real savepoint API.

2. /*! … */ executable comments bypass the multi-statement guard

queryHasMultipleStatements() skips every /* … */ as inert. MySQL treats a version-gated executable comment as live SQL: the lexer discards the /*! marker and keeps lexing the contents as ordinary tokens, so a ; inside it reaches the parser as a statement separator.

$db->query("SELECT 1/*!;DROP TABLE t*/");   // scanner: single statement → allowed

With CLIENT_MULTI_STATEMENTS enabled connection-wide by the bridge, the server sees and runs both. The scan is documented as the reason the "1; DROP TABLE …" class is contained, so this is a hole in the stated security boundary — and the client-side scan is the only guard on this path.

Worth spelling out why nothing else catches it: the bridge has its own guard in MyConn::prepare — if !self.multi_statements && sql_has_multiple_statements(...) (my.rs:2112) — but multi_statements defaults to true (my.rs:125) and the mysqli prelude never passes multi=0 in $_driverConfig (the key exists, my.rs:619), because multi_query() needs it on. So the Rust guard is dead on this path by construction. The same /*! blind spot also lives in the bridge's scan_my_comment, where it is latent for the same reason.

Fix: do not skip a comment opening with /*! — either scan its body as live SQL, or reject the statement outright.

---

Should fix before merge

3. real_escape_string() is not charset-aware — undocumented injection divergence

mysql_real_escape_string consults the connection charset; this implementation is pure byte substitution. Under an ASCII-incompatible multi-byte charset reachable through set_charset() / MYSQLI_SET_CHARSET_NAME (gbk, big5, sjis, cp932), the classic trailing-byte breakout applies. Byte-for-byte capture after $db->set_charset("gbk"), escaping 0xBF 0x27:

real php  →  5c bf 5c 27      (escapes the lead byte too — safe)
this PR   →  bf 5c 27         (0xBF5C is one valid GBK char, then a BARE unescaped quote)

✅ proven. That is a live SQL-injection breakout in the escaped output. Not in the divergence list.

Credit where due: I checked the NO_BACKSLASH_ESCAPES branch the same way and it is correct — real php also doubles only ' there and leaves " and \ alone (a''b"c\d), exactly matching your str_replace("'", "''", …). The jury flagged that branch as a bug; execution refuted it.

Fix: at minimum document it; better, reject the known-dangerous charsets in set_charset() / charsetIdentIsValid(), or route escaping through a charset-aware bridge call.

4. commit() / rollback() silently ignore $flags

Both methods accept int $flags and never read it. php composes it into the SQL — captured:

$db->commit(MYSQLI_TRANS_COR_AND_CHAIN)                                      →  COMMIT AND CHAIN
$db->rollback(MYSQLI_TRANS_COR_RELEASE)                                      →  ROLLBACK RELEASE
$db->begin_transaction(START_READ_WRITE|START_WITH_CONSISTENT_SNAPSHOT)      →  START TRANSACTION WITH CONSISTENT SNAPSHOT, READ WRITE
$db->begin_transaction(START_READ_ONLY)                                      →  START TRANSACTION READ ONLY

✅ proven. begin_transaction also splits what php sends as one statement into SET TRANSACTION … + BEGIN (two round-trips; it works, since scope-less SET TRANSACTION applies to the next transaction, but it is not the same statement). The MYSQLI_TRANS_COR_* constants aren't declared either, so a symbolic call fails loudly at compile time — but an integer literal or variable is silently dropped. (Raised independently by GLM 5.2; confirmed by capture.)

5. Connection-level affected_rows / insert_id / warning_count go stale

They are written only by query() / real_query() / multi_query(). Real php refreshes them from the OK packet of every command — captured (fake server answers INSERT with affected_rows=5, last_insert_id=42):

after INSERT: affected=5 insert_id=42
after START TRANSACTION: affected=0 insert_id=0
after COMMIT: affected=0 insert_id=0

✅ proven. Two consequences:

• $stmt->execute(); $id = $db->insert_id; — a very common idiom — reads 0 or a stale value, because mysqli_stmt::execute() updates only the statement's own copies and never touches $this->link.
• The PR's own shipped example is affected: examples/mysqli-crud/main.php:98 prints $db->affected_rows immediately after $db->commit(). php prints 0; elephc prints the DELETE count. The example demonstrates behaviour php does not have.

6. Operations on a never-connected object diverge, one of them silently

php 8 raises Error: mysqli object is not fully initialized for any operation on a mysqli_init() / argument-less new mysqli() object. ✅ proven — executed against php 8.5.6:

call on an unconnected object	php 8.5.6	this PR
real_escape_string("a'b")	Error: mysqli object is not fully initialized	returns a\'b, no error, errno stays 0
character_set_name()	Error: mysqli object is not fully initialized	returns ""
query("SELECT 1")	Error: mysqli object is not fully initialized	errno 2006 "MySQL server has gone away"
close()	true	true ✓

The real_escape_string row is the one that matters: it is the only path that silently produces a value where php hard-errors, so a program that escapes before connecting gets an answer with no signal at all. (2006 for an object that was never connected is also a slightly odd choice — "gone away" implies it was there — but at least it fails loudly.) (Raised by Kimi K2.7 for real_escape_string; executing it showed the family is wider.)

7. mysqli_sql_exception::getSqlState() is missing

It exists in php 8.1+ and is the documented way to read the SQLSTATE — docs/php/mysqli.md even says "php-src keeps it protected behind getSqlState()", then never provides the method. $e->getSqlState() fails to compile. One line:

public function getSqlState(): string { return $this->sqlstate; }

(Confirmed by reflection on php 8.5.6, alongside the $sqlstate default "00000" and protected visibility — both of which your implementation already matches.)

8. The procedural surface is materially incomplete, and silently so

docs/php/mysqli.md claims "Every public method has a mysqli_* procedural alias" — true of the methods you implemented, but the surface is much smaller than php's 106 functions, and the "Not implemented (fails loudly)" list doesn't mention the gap. Absent and unlisted:

• methods: mysqli::savepoint, mysqli::release_savepoint, mysqli_result::field_seek
• property: mysqli_stmt::$error_list
• procedural: mysqli_savepoint, mysqli_release_savepoint, mysqli_fetch_lengths, mysqli_field_seek, mysqli_field_tell, mysqli_stmt_sqlstate, mysqli_stmt_field_count, mysqli_stmt_insert_id, mysqli_stmt_free_result, mysqli_stmt_data_seek, mysqli_stmt_error_list, mysqli_stmt_result_metadata, mysqli_stmt_init, mysqli_stmt_prepare, mysqli_get_charset, mysqli_thread_safe, mysqli_execute

Several are one-liners over state you already hold (mysqli_stmt_sqlstate, mysqli_stmt_field_count, mysqli_stmt_insert_id, mysqli_fetch_lengths). The rest just need a line in "Not implemented" so the omission is a promise, not a surprise.

---

Worth addressing

9. Three SQL scanners, three sets of rules — one already exists in Rust

queryHasMultipleStatements() (PHP), __elephcCountPlaceholders() (PHP), and sql_has_multiple_statements() + scan_my_comment() (Rust, crates/elephc-pdo/src/my.rs) all scan MySQL SQL and disagree:

input	PHP queryHasMultipleStatements	Rust sql_has_multiple_statements
"; SELECT 1"	multi (rejects)	single
"SELECT 1;;"	multi (rejects)	single
-- at EOF	comment	not a comment
--\v / --\f	not a comment (rejects valid SQL)	comment
--\n	comment	not a comment

The Rust one is already written, already tested, and already handles the NO_BACKSLASH_ESCAPES and doubled-backtick cases. It just isn't reachable from the prelude because it only runs when multi_statements is off. Exposing it as elephc_pdo_sql_has_multiple_statements(int $conn, string $sql): int would collapse three scanners into one — and would mean fixing #2 in exactly one place instead of three.

10. Both PHP scanners are per-byte substr() loops on every statement

queryHasMultipleStatements() runs on every query(), __elephcCountPlaceholders() on every prepare(). Each iteration allocates a fresh one-character string; a 1 MB batch INSERT costs ~1M allocations before a byte reaches the wire. Two lines remove the scan entirely for the overwhelming majority of statements:

if (strpos($query, ";") === false) { return false; }   // queryHasMultipleStatements
if (strpos($query, "?") === false) { return 0; }       // __elephcCountPlaceholders

11. real_connect() costs two extra round-trips php doesn't pay

SELECT CONNECTION_ID() for thread_id and SELECT @@character_set_client for character_set_name(). Real php sends zero queries at connect — both values come out of the initial handshake packet. ✅ proven (the packet log shows no COM_QUERY before the first user statement, yet thread_id=1 charset=utf8mb4). Beyond the cost, @@character_set_client is a session variable, not the negotiated client charset, so the two can disagree.

12. detect.rs misses bare MYSQLI_* constants

ExprKind::ConstRef(_) => false, so a global constant is never a mysqli reference — but the prelude declares ~45 of them. A program whose only mysqli mention is a constant (a config file returning ['mode' => MYSQLI_ASSOC], a helper that just forwards MYSQLI_ASSOC) fails with "undefined constant", which is exactly the failure the module's own "soundness over precision" comment says the exhaustive match exists to prevent. PDO doesn't have this gap because its constants are class constants reached through a detected StaticReceiver.

13. mysqli_stmt::close() leaves hasPending set

close() / __destruct() set $this->stmt = -1 but never clear $this->hasPending. A post-close get_result() / store_result() therefore drives elephc_pdo_column_* / elephc_pdo_step with handle -1. The bridge is defensive so nothing crashes — it returns "", type 5 (NULL) and step -1 — but the caller gets a silently empty result, or an opFailed() carrying errno 0 and an empty message, which under MYSQLI_REPORT_STRICT throws a mysqli_sql_exception with no message. php raises "mysqli_stmt object is already closed".

14. No automated runtime coverage at all

All 17 tests in tests/codegen/mysqli_mysql.rs are #[ignore] and CI runs no MySQL service, so green CI on a 6.5k-line surface means "it compiles". Every finding above except #12 and #16 is a runtime behaviour. A GitHub Actions services: mysql job — even one that only runs the existing 17 — would have caught #1, #4 and #5. The offline matrix in tests/codegen/mysqli.rs and tests/extension_loaded_tests.rs is genuinely good; it's the live half that has no gate.

15. begin_transaction(0, "") throws after opening the transaction

The empty-$name guard sits at the end of the method, so SET TRANSACTION … and BEGIN / START TRANSACTION … have already gone to the server when the ValueError is thrown — leaving an open transaction behind on a call php rejects before sending anything. commit() and rollback() get the ordering right (they throw before any SQL); only begin_transaction() doesn't. Moving the check next to the other argument validation fixes it. (Kimi K3.)

16. pipeline.rs walks the AST twice per surface

let pdo_used = pdo_force || pdo_prelude::program_uses_pdo(&ast);
let ast = pdo_prelude::inject_if_used(ast, pdo_force);   // recomputes program_uses_pdo

Both inject_if_used functions start with if !force && !detect::program_uses_X(&program), so the detection walk runs twice per surface per compile — over an AST that, for mysqli, already contains the injected PDO prelude. Passing the precomputed boolean as force is behaviour-identical and halves it.

---

Nits

• MYSQLI_REPORT_ERROR writes to STDERR instead of raising a warning. The doc table discloses this, but it isn't in the divergence list and the consequence isn't stated: php raises a real E_WARNING, so set_error_handler(), error_get_last(), error_reporting, and log routing all see it — none of them see an fwrite(STDERR, …). The format differs too. ✅ proven, captured from php 8.5.6 under MYSQLI_REPORT_ERROR with no STRICT:

  E_WARNING: mysqli::query(): (42000/1064): You have an error in your SQL syntax
  

  vs this PR's mysqli error: You have an error in your SQL syntax — no SQLSTATE, no errno, no method name, and invisible to every php error hook. (Kimi K3.)
• __elephcCountPlaceholders diverges from its sibling. It treats any -- as a comment (no whitespace-after check) and hardcodes backslash escaping inside literals, ignoring NO_BACKSLASH_ESCAPES — both of which queryHasMultipleStatements gets right. SELECT ?--? is two placeholders in MySQL; the counter returns 1. Folded into #9 if you go that route.
• p: prefix is matched case-sensitively. str_starts_with($_host, "p:"); php uses a case-insensitive compare — new mysqli("P:127.0.0.1", …) connects fine in php ✅ proven, and would try to resolve the literal host P:127.0.0.1 here.
• --allow-multiple-definition widening. bridge_archive_count(plan) >= 2 now relaxes duplicate-symbol checking for every two-bridge Linux link, where before only forced whole-archive pairs got it. The rationale in the comment is sound, but it also silences a class of real link bugs on links that used to be strict — worth a note that the escape hatch grew.
• Argument-error classes. bind_param() records errno 2031 and returns false when the variable count doesn't match $types (including bind_param("ii") with no variables at all); php throws ArgumentCountError. data_seek(-1) returns false; php throws ValueError. Straight from php-src ext/mysqli/mysqli_api.c:

  zend_argument_count_error("The number of elements in the type definition string must match the number of bind variables");
  ...
  if (offset < 0) { zend_argument_value_error(ERROR_ARG_POS(2), "must be greater than or equal to 0"); RETURN_THROWS(); }

  Neither is in the divergence list. (The jury split on this one — resolved against php-src.)
• Checked and clean (so you don't have to re-audit): mysqli_report() accepts any int with no validation in php too ✅ proven; mysqli_sql_exception::$sqlstate really does default to "00000" and really is protected ✅ proven; the NO_BACKSLASH_ESCAPES escape branch matches php byte-for-byte ✅ proven; all 167 extern signatures moved to bridge_externs.rs are identical to the ones deleted from PDO_PRELUDE_SRC; MYSQLI_* constant values, mysqli_report flag bits, fetch-mode values and option ids all match php-src.

---

Also worth noting the PR is currently CONFLICTING with main, so it will need a rebase before it can merge.

[nahime0]

@Guikingone — thank you, this was an exceptional review: wire-captured against real PHP + a fake-server packet log, adversarial, and almost entirely correct. Every finding is addressed. I re-verified the two scariest and one soundness gap myself against php 8.4 + real MariaDB 11 before touching anything, and all three reproduced exactly as you described. Two commits: 050babde7 (blockers 1–3 + lifecycle) and b02d8cc8c (surface completion + soundness).

Blockers — all fixed and re-verified live

• 1 commit($name) data loss — $name is now a SQL comment (COMMIT /*name*/), not a savepoint; savepoint() / release_savepoint() + procedural aliases added; $flags composed into START TRANSACTION / COMMIT / ROLLBACK; MYSQLI_TRANS_COR_* declared. Wire-confirmed: named commit() commits and the row survives a reconnect (was RELEASE SAVEPOINT → silent loss). Live test test_mysqli_named_commit_persists_and_savepoints.
• 2 /*! executable comments — the fix is in the bridge (scan_my_comment no longer skips /*! … */), and query()'s guard now calls the single authoritative bridge scanner. SELECT 1/*!;DROP…*/ → errno 1064.
• 3 GBK escape — new charset-aware bridge ABI elephc_pdo_real_escape_string; 0xBF 0x27 under gbk → \ 0xBF \ ', byte-identical to php. (Confirmed your note: the NO_BACKSLASH_ESCAPES branch was already correct.)

Should fix

• 4 commit/rollback read $flags (AND [NO] CHAIN, [NO] RELEASE); begin_transaction emits one START TRANSACTION …; MYSQLI_TRANS_COR_* declared.
• 5 stmt execute refreshes the connection's affected_rows/insert_id/warning_count; transaction control resets them (post-commit affected_rows is 0). The example now reads the DELETE count before commit.
• 6 unconnected-object ops raise php 8's Error: mysqli object is not fully initialized (including the silent real_escape_string).
• 7 getSqlState() added.
• 8 procedural surface completed: savepoint/release_savepoint, stmt_init/stmt_prepare/execute, stmt_sqlstate/stmt_field_count/stmt_insert_id/stmt_error_list/stmt_free_result, fetch_lengths, field_seek/field_tell, get_charset, thread_safe; mysqli_stmt::$error_list added; the genuinely-absent members (stmt_data_seek, stmt_result_metadata) are now listed under Not-implemented.

Worth addressing

• 9 three scanners collapsed into one — the bridge scanner is exposed as elephc_pdo_sql_has_multiple_statements and the PHP queryHasMultipleStatements / __elephcCountPlaceholders are deleted (param_count now comes from elephc_pdo_mysql_param_count, exact).
• 10 strpos early-outs (no ;/? → skip the scan).
• 11 thread_id and charset come from the handshake with zero connect-time queries (elephc_pdo_mysql_thread_id; charset defaults to the negotiated utf8mb4).
• 12 detect.rs now treats a bare MYSQLI_* constant as a reference (a config-array/helper no longer fails "undefined constant").
• 13 stmt close()/__destruct clear hasPending; get_result/store_result guard on a closed handle.
• 14 — one correction: the live suite does run in CI. pdo-live.yml runs cargo test … --ignored mysql, and mysql matches mysqli_mysql by substring, so all 18 live tests ran (and passed) in the existing PDO Live Databases job. You're right the main CI has no MySQL service; the live half rides pdo-live as the plan directed. I added 3 more live tests (transactions/savepoints, stmt_init, injection) there.
• 15 begin_transaction empty-$name ValueError is raised before any SQL.
• 16 single AST walk per surface (precomputed bool passed as force).

Nits

p: prefix matched case-insensitively; docs now cover the unconnected-object Error, MYSQLI_REPORT_ERROR STDERR-vs-E_WARNING, the ArgumentCountError/ValueError argument-error divergences, charset-aware escaping, exact param_count, and the --allow-multiple-definition widening rationale (in-code on bridge_archive_count).

Verification: 125 bridge tests, 25 unit + 12 offline codegen + 7 error + 16 extension, 18/18 mysqli live and 43/43 PDO-mysql live on MariaDB (the 3 red mysql_tls_* need the TLS env only — not a regression). The three findings your model jury dropped (" under NO_BACKSLASH_ESCAPES, the reconnect multi-query reset, --\v) I left as you concluded. Re-requesting your review.

[Guikingone]

Re-review — b02d8cc8c0 (rebased, CI 130/130, live suites green)

This is a strong follow-up. Fifteen of the sixteen items landed, several of them by doing the harder and more correct thing rather than the cheap one: moving the multi-statement scanner and the escape into the bridge so there is one authoritative implementation, taking thread_id / param_count from the handshake and the prepared statement instead of round-trips, and turning /*! into live SQL in scan_my_comment so the fix lands for PDO too, not just mysqli. Same method as last time: every ✅ proven claim below is captured bytes from real PHP 8.5.6 + the real mysqli extension against a fake MySQL server, cross-checked by a jury of other agents asked to refute them.

First, a correction I owe you. My previous review said the live tests were #[ignore]d with no MySQL in CI, so "green CI means it compiles". That was wrong. .github/workflows/pdo-live.yml runs on every PR with a MySQL 8.4 service, and its --ignored mysql filter matches mysqli_mysql::test_mysqli_* (mod mysqli_mysql; is in the codegen_tests binary). "PDO core live suites (PostgreSQL 16 + MySQL 8.4)" is SUCCESS on this head — the mysqli live suite does run, and does pass. I should have checked the workflow before asserting its absence. Apologies for the noise; the new test_mysqli_named_commit_persists_and_savepoints regression test is exactly the right addition and it is genuinely gated.

---

Blocker 1 — the charset-aware escape is right only for the GBK family

my_real_escape closes the breakout for gbk, and the unit tests pin exactly the bytes I captured for it. But it applies one lead/trail band — lead >= 0x81, trail 0x40..=0xFE — to all nine charsets in charset_escape_is_dangerous, and the real ranges differ per charset. Two consequences, one of them serious.

It reintroduces the breakout for sjis / cp932 / euckr / ujis

0x5C (backslash) sits inside the assumed trail range, so any lead-looking byte followed by a backslash is swallowed as a "complete 2-byte character" and the backslash is copied out raw. That is correct for GBK, where 0xBF5C really is one character. It is wrong everywhere else: 0xBF is not a Shift-JIS/cp932 lead byte at all (sjis leads are 0x81–0x9F / 0xE0–0xEF), and 0x5C is never a valid EUC-KR / EUC-JP trail (those trails are 0xA1–0xFE).

Payload "\xbf\\' OR 1=1 -- ", escaped — ✅ proven, both sides executed:

              php                                rust
sjis    bf 5c 5c 5c 27 20 4f 52 ...        bf 5c 5c 27 20 4f 52 ...
cp932   bf 5c 5c 5c 27 ...                 bf 5c 5c 27 ...
euckr   5c bf 5c 5c 5c 27 ...              bf 5c 5c 27 ...
ujis    5c bf 5c 5c 5c 27 ...              bf 5c 5c 27 ...
gbk     bf 5c 5c 27 ...                    bf 5c 5c 27 ...   ← only this one matches

The Rust function returns the identical byte string for all five charsets — the GBK-correct one. Under sjis the server then reads bf as a standalone byte, 5c 5c as an escaped backslash, and a bare 27 that closes the literal; OR 1=1 -- executes. php's sjis output keeps that quote escaped. The escape output is strictly less escaped than php's for these four charsets, which is the unsafe direction.

Independent confirmation from Python's own codec tables, which agree with php byte for byte:

gbk        bf5c  -> ONE character          (pairing correct)
shift_jis  bf5c  -> TWO characters         (0x5C stays a live backslash)
cp932      bf5c  -> TWO characters
euc_kr     bf5c  -> invalid sequence
euc_jp     bf5c  -> invalid sequence
gbk        bf7f  -> invalid sequence       (0x7F is not a valid trail)

Reachability is direct: charsetIdentIsValid() accepts [A-Za-z0-9_], so set_charset("sjis") stores "sjis" verbatim in $currentCharset, and real_escape_string() hands exactly that to the bridge.

This also makes docs/php/mysqli.md currently overclaim — it names sjis and cp932 among the charsets whose breakout is closed.

The band is approximate even for GBK

GBK's trail range excludes 0x7F; the code's 0x40..=0xFE includes it. ✅ proven:

gbk  bf7f    → php 5c bf 7f      rust bf 7f
gbk  bf7f27  → php 5c bf 7f 5c 27  rust bf 7f 5c 27

Not injectable (the quote stays escaped), but it shows the band is a guess rather than the charset's actual table. Across 180 (charset, input) pairs, 143 match php and 37 diverge — the rest of the 37 are over-escaping (big5 8127, gb18030 bf, gbk ff27 …), harmless for safety but not byte-faithful.

Worth noting how the gap survived: dangerous_charset_classification asserts all nine names are classified dangerous, but real_escape_is_charset_aware only asserts the escaping for gbk and utf8mb4. The classifier is tested nine ways, the escape one way.

Suggested fix. Either carry the real per-charset lead/trail table for the nine names (ismbchar-style), or — if that is more than this PR wants to own — keep the current band only for the GBK family and have set_charset() reject the charsets you cannot escape faithfully. Failing loudly on sjis is far better than silently escaping it as if it were GBK. Whichever way, the 180-pair differential is cheap to keep as a fixture: capture php's output once, assert my_real_escape against it.

---

Blocker 2 — the transaction $name is an executable-comment injection (new, from this fix)

transactionComment() rejects */ in $name — good, that was the obvious escape — but nothing stops the name from opening a MySQL executable comment. A name beginning with ! turns the wrapper into /*! … */, whose body the server lexes as live SQL:

$db->begin_transaction(0, "!50000 ; DROP TABLE victims");
// elephc emits:  START TRANSACTION /*!50000 ; DROP TABLE victims*/
//                                   ^^^^^^^ executable comment → DROP runs

This is the exact mechanism the same commit taught scan_my_comment about (/*! is live SQL, so a ; inside it separates statements) — the knowledge is in the codebase, it just is not applied here. And this path bypasses the multi-statement guard entirely: begin_transaction / commit / rollback go through elephc_pdo_exec() directly, never runQuery(), so elephc_pdo_sql_has_multiple_statements never sees the string.

php does not have this hole, because it sanitises the name rather than validating it. ✅ proven — captured from php 8.5.6:

$db->commit(0, "!50000 ; DROP TABLE victims")  →  COMMIT /*50000  DROP TABLE victims*/
$db->commit(0, "has'quote")                    →  COMMIT /*hasquote*/
$db->commit(0, "back\slash")                  →  COMMIT /*backslash*/
$db->commit(0, "semi;colon")                   →  COMMIT /*semicolon*/
$db->commit(0, "a b-c_d=e")                    →  COMMIT /*a b-c_d=e*/

Blocklisting is the wrong shape here anyway: this surface targets MariaDB too, which has its own /*M!50000 … */ executable-comment form, so an M! prefix opens the same door. An allowlist closes both without having to enumerate dialects.

php is explicit about the rule — it even tells you, via a real E_WARNING:

Warning: mysqli::commit(): Transaction name has been truncated, since it can only
contain the A-Z, a-z, 0-9, "\", "-", "_", and "=" characters

Feeding it every printable ASCII byte, the characters php actually keeps are:

(space) - 0123456789 = A-Z _ a-z

So: strip $name to [A-Za-z0-9 \-_=] and warn that it was truncated. That is php's behaviour exactly, it is allowlist-shaped, and it needs no knowledge of which comment dialects exist. (Worth noting php strips and warns rather than throwing — so mirroring it also removes the */ ValueError you currently raise, which php does not.)

savepoint() / release_savepoint() are fine — they backtick-quote with doubling. It is only the three comment-carrying methods.

---

The guard and the escape disagree about where a string literal ends

Same root cause as Blocker 1, different function, lower severity — but it means the multi-statement guard is bypassable on exactly the charsets Blocker 1 is about. ✅ proven, calling the scanner directly on this branch:

sql_has_multiple_statements("SELECT '<BF><5C>' ; DROP TABLE t", no_backslash_escapes=false) -> false   ← admitted
sql_has_multiple_statements("SELECT 'a'        ; DROP TABLE t", no_backslash_escapes=false) -> true    ← rejected
sql_has_multiple_statements("SELECT '<BF><5C>' ; DROP TABLE t", no_backslash_escapes=true)  -> true    ← rejected

0xBF5C is one valid GBK character. sql_has_multiple_statements is charset-blind: it reads the 0x5C trail byte as a backslash escape, skips the byte after it — the closing quote — and from there believes it is still inside a string literal, so the ; never registers. The server, using the real GBK tables, consumes <BF><5C> as one character, closes the literal, and runs two statements.

The precondition is exactly charset_escape_is_dangerous: a charset in which 0x5C can be a trail byte. That classifier now lives in the same file — the scanner just doesn't consult it. Whichever shape Blocker 1's fix takes (per-charset tables, or narrowing the supported set), sql_has_multiple_statements wants the connection charset alongside no_backslash_escapes and the same table, or the escape and the guard will keep disagreeing about where a literal ends.

Severity is defence-in-depth, not primary: a caller that escapes properly never reaches this. But the guard is documented as the reason a concatenated "1; DROP TABLE …" cannot execute, and under a GBK-family connection a <BF><5C> prefix defeats it.

---

Smaller

The multi-statement guard fails open (defensive). elephc_pdo_sql_has_multiple_statements is wrapped in ffi_guard(0, …), and 0 means "not multiple" — a panic inside the scanner would admit the statement. The jury pushed back fairly here: the scanner is bounds-safe and no data-triggerable panic is evident, so this is hardening rather than a live hole. Still worth flipping, since 1 costs nothing and elephc_pdo_real_escape_string already degrades safely (-1 → caller returns "").

currentCharset is assumed, not negotiated. real_connect() sets $this->currentCharset = "utf8mb4" as a literal, and both character_set_name() and the charset handed to the escape depend on it. This is worse than it first looks, because the cache can go stale without anyone calling set_charset(): mysqli_options(MYSQLI_INIT_COMMAND, "SET NAMES sjis") runs server-side at connect, and a reused persistent (p:) connection carries whatever charset the previous script left behind. In both cases the server is on a multibyte charset while the escape still believes utf8mb4 and does plain byte substitution — which is the original pre-fix vulnerability, reachable again. Reading the charset back from the connection, as you already do for thread_id, makes it true by construction.

---

Verified fixed

Not taken on trust — each checked in the tree, and the bridge's own tests run (4 passed):

• #1 transactions — COMMIT [AND [NO] CHAIN] [[NO] RELEASE] /*name*/, matching my captured wire format including START TRANSACTION WITH CONSISTENT SNAPSHOT, READ WRITE. savepoint() / release_savepoint() added, correctly backtick-quoted. transactionComment() already rejects */, which was the obvious half of the new comment-carrying design — see Blocker 2 for the half it misses.
• #2 /*! — scan_my_comment returns None for /*!, and multi_statement_detection_sees_executable_comments pins SELECT 1/*!;DROP TABLE t*/ and /*!50000 ; SELECT 2 */. query() now calls the bridge scanner behind a strpos(";") fast path — sound, since a multi-statement string must contain a ;.
• #4 flags, #5 refreshStatus() on execute and transaction control, #6 requireInitialized() raising php 8's "not fully initialized" Error, #7 getSqlState(), #8 the procedural surface (incl. mysqli_fetch_lengths, field_seek/field_tell, the stmt_* accessors, $error_list), #11 zero connect-time round-trips, #12 param_count from the bridge (the divergent PHP ?-scanner is gone), #13 close() clearing hasPending, #15 the empty-$name ValueError before any SQL, #16 the single AST walk.
• The example no longer prints affected_rows after commit().

Jury note for the record: Codex, GLM 5.2 and OpenCode all confirmed Blocker 1 independently from the byte evidence — 3/3. Blocker 2 is Codex's find: I had checked */ and stopped there; it asked what a leading ! would do, and php's own sanitising behaviour then confirmed it on the wire. The 0x7F GBK trail case came from GLM. The charset-blind scanner section above started as an OpenCode remark that I had initially dismissed — I had written off a narrower version of it (String::from_utf8_lossy making the scanner analyse different bytes than the server) as harmless, which it is; the real defect was one layer down, in the escape handling, and it reproduces. The panel split on the fail-open ffi_guard (OpenCode confirmed it, Codex found no data-triggerable panic), which is why it sits under Smaller as hardening rather than a live hole.

[Guikingone]

Re-review — 770bed279a — both blockers closed

Verified, not taken on trust. The two blockers and the scanner bypass are genuinely fixed, and the way they were fixed is better than what I asked for: replacing the single lead/trail band with per-charset ismbchar-style char_len predicates, sharing that one table between the escape and the scanner, passing raw bytes instead of from_utf8_lossy, failing the guard closed, and tracking the live charset in the bridge so nothing has to assume utf8mb4. Finding your own first per-charset table still wrong for big5/ujis/euckr, by differential, before I saw it, is the part worth calling out.

Method. I re-ran the escape differential against real PHP 8.5.6 + the real mysqli extension, this time with a case set I generated rather than the one you fixed against: every lead byte 0x80..0xFF × 19 second bytes × 3 shapes, plus 3-byte EUC-JP and 4-byte gb18030 probes — 7412 inputs × 10 charsets × 2 escape modes = 148,240 comparisons.

backslash-escapes ON    74120 compared   70064 identical   4056 divergent
                        divergences:  gb18030 3498,  euckr 558,  every other charset 0
                        direction:    gb18030 3498 over-escape
                                      euckr    546 over-escape
                                      euckr     12 under-escape   ← the only ones anywhere
NO_BACKSLASH_ESCAPES    74120 compared   74120 identical   0 divergent

gbk, gb2312, big5, sjis, cp932, ujis, utf8mb4 and latin1 are now byte-identical to php across the whole sweep, in both modes. No output anywhere contains a 0x27 that is not immediately preceded by 0x5C. The 12 euckr under-escapes all begin 0x8E/0x8F and contain no quote and no backslash — none can break out.

The NO_BACKSLASH_ESCAPES column is worth a note: the escape's NBE branch returns early without consulting the charset table, which read like a latent fragility. It isn't — 74,120/74,120 identical to php, because 0x27 is not a valid trail byte in any of the nine charsets, so charset-awareness genuinely cannot matter there. php reaches the same conclusion.

Scanner re-probed on this branch — the bypass I reported is closed, and the controls still behave:

sql_has_multiple_statements("SELECT '<BF><5C>' ; DROP TABLE t", gbk)   -> true    (was false)
   idem gb18030 / big5 <A1> / sjis <81> / cp932 <81>                    -> true
"SELECT 1/*!;DROP TABLE t*/"                                           -> true
"SELECT '<BF><5C>x' FROM t"   (one statement)                          -> false
"SELECT 1;"                   (trailing semicolon)                     -> false
utf8mb4 control                                                        -> false   ← correct: there the 5C really does escape the quote, so the literal never closes and it IS one (malformed) statement

cargo test -p elephc-pdo --lib → 128 passed, 0 failed. CI 130/130, both live DB suites green.

---

One thing left, and it is a coverage gap rather than a regression

The charset tracker hooks three SQL spellings and misses the other three. Probed directly:

SET NAMES gbk                            -> tracked
SET CHARACTER SET gbk                    -> tracked
SET CHARSET gbk                          -> tracked
SET character_set_client = gbk           -> NOT tracked
SET character_set_connection = gbk       -> NOT tracked
SET @@character_set_client = 'gbk'       -> NOT tracked
SET SESSION character_set_client = gbk   -> NOT tracked

SET character_set_client = gbk changes how the server interprets the bytes you send it, so after it the escape runs on a stale utf8mb4 table: real_escape_string("\xBF'") emits BF 5C 27, the server (now in gbk) reads BF 5C as one character and 27 as a live closing quote. That is the round-2 breakout again, through a different door.

Two things keep this from being a blocker, and I want to be precise about both:

• php has exactly the same hole. ✅ proven — after query("SET character_set_client = gbk"), php's character_set_name() still reports utf8mb4 and it escapes with the utf8mb4 table. This is the documented reason the PHP manual tells you to use mysqli_set_charset() and never change the charset by SQL. So this is not a regression against the reference.
• But you are already ahead of php here, which is what makes the gap worth closing. ✅ proven: php does not track a raw query("SET NAMES gbk") either — it stays on utf8mb4 and emits bf5c5c for BF 5C, which is unsafe against a server that really is in gbk. elephc tracks it and emits bf5c. You deliberately went further than php, and the live test asserts it. Having done that, stopping at three of the six spellings means a developer who learns "elephc follows my raw charset changes" is wrong for character_set_client.

So: either extend charset_after_set_names to the character_set_client|connection|results assignments (plain, @@ and SESSION spellings), or state in the docs exactly which forms are tracked. The first is a few lines and makes the promise true. (Raised by GLM 5.2 and OpenCode; their claim that SET CHARACTER SET was also missed is refuted above — it is tracked.)

---

Nits

• commit(0, "") and rollback(0, "") throw where php does not. ✅ proven: php raises ValueError: … Argument #2 ($name) must not be empty for begin_transaction only; for commit/rollback it does not throw at all and sends COMMIT /**/ / ROLLBACK /**/. transactionComment() is shared by all three, so elephc throws for all three — an uncaught ValueError where php commits. (The message wording you moved to, "must not be empty", is exactly php's.)
• The allowlist keeps \, php drops it. php's own warning text names "\" as allowed, but feeding it every printable ASCII byte shows the kept set is (space) - 0-9 = A-Z _ a-z — no backslash. Harmless either way, since * and / are both stripped so */ cannot be formed.
• euckr follows MySQL's table, php's mysqlnd uses a wider UHC-like one. The 12 under-escapes come from 0x8E/0x8F, which MySQL's ctype-euc_kr.c does not treat as leads and mysqlnd does. Matching the server is the defensible choice for escaping — the server is what parses the result — but real_escape_string is then not byte-identical to php on euckr, which the divergence list does not mention.
• gb18030 over-escaping is safe and reversible, as documented: 5C <lead> 30 is parsed by the server as backslash-then-lead, yielding the original bytes back.

---

Jury: Codex, GLM 5.2 and OpenCode. Codex returned "nothing found" on the missed-defect pass. The charset-tracker gap came from GLM and OpenCode, and survived — but two of their claims did not: SET CHARACTER SET is tracked (probed), and persistent-connection reuse does not reset the tracked charset (it lives on the connection struct, so it travels with the pooled handle). Recording both so nobody re-litigates them.

[nahime0]

Follow-up issues filed for the work deliberately deferred from this PR (all currently disclosed as divergences in docs/php/mysqli.md or noted in the reviews):

• ci: add a MariaDB live job alongside MySQL 8.4 in pdo-live #740 — ci: MariaDB live job alongside MySQL 8.4 (server-behavior differences like binlog-default 1419 and /*M! expansion currently have no CI gate)
• Checker rejects argument unpacking into a by-ref variadic parameter (valid PHP, e.g. forwarding to mysqli_stmt::bind_param) #741 — checker rejects spread into a by-ref variadic (the reason the procedural mysqli_stmt_bind_param needs a friend channel instead of forwarding)
• No E_WARNING channel: engine warnings invisible to set_error_handler()/error_get_last() (mysqli REPORT_ERROR + transaction-name truncation rest on it) #742 — E_WARNING channel (unblocks MYSQLI_REPORT_ERROR parity and the transaction-name truncation warning)
• mysqli: true unbuffered MYSQLI_USE_RESULT streaming (use_result() aliases store_result()) #743 — true MYSQLI_USE_RESULT streaming (also makes the multiDrainCurrent mid-set error path live)
• mysqli: TLS support (MYSQLI_CLIENT_SSL + ssl_set) over the bridge's existing PDO TLS options #744 — mysqli TLS over the bridge's existing PDO TLS options
• mysqli: small PHP-parity batch (bind_param ArgumentCountError, data_seek ValueError, $info, insert_id overflow, store_result $mode) #745 — small parity batch (ArgumentCountError/ValueError classes, $info, insert_id overflow, store_result $mode)
• linker: narrow the --allow-multiple-definition escape hatch on multi-bridge Linux links #747 — narrow the --allow-multiple-definition escape hatch on multi-bridge Linux links

Compiler limits hit while building the prelude were already tracked: #703, #704, #705, #706, #707.

[nahime0]

Status: review ledger closed — ready for merge

Full reconciliation of every finding across the three reviews, verified against the current head (48fd518b8, rebased on latest main):

Review 1 (16 items + nits) — all closed: #1–#13/#15/#16 were individually re-verified in review 2 ("Verified fixed"); #14 was retracted by the reviewer (pdo-live does gate the mysqli live suite); every nit is either fixed (p: case-insensitivity, placeholder counter replaced by bridge param_count) or in the documented divergence list with consequences stated (E_WARNING vs STDERR, argument-error classes).

Review 2 — both blockers closed and re-verified in review 3 (per-charset escape tables byte-identical to php across the 148k-comparison differential; transaction-name allowlist), plus the charset-blind scanner, the ffi_guard now failing closed, and the bridge-tracked live charset.

Review 3 — the charset-tracker gap is closed on both halves of the either/or: SET character_set_client tracked in all spellings (plain/SESSION/LOCAL/@@ forms, bare or quoted) and the docs state exactly which forms are tracked and why _connection/_results/GLOBAL are deliberately not. All four nits addressed (commit/rollback empty-name parity, backslash dropped from the allowlist, euckr divergence documented, gb18030 already documented).

Final hardening round (3aa272fc3, 48fd518b8): MariaDB /*M! treated as live SQL by the scanner (case-sensitive, verified on MariaDB 11.8), close() clearing the pending buffered result for clean reconnects, the two-step stmt_init()+prepare()/execute() honoring the 2014 guard, --with-mysqli rooting the surface through reachability like --with-pdo, mid-drain error propagation in multiDrainCurrent, and the __elephc* internals privatized. Each landed red→green with unit + live coverage (validated against both MariaDB 11.8 and MySQL 8.4.11).

Deferred work is tracked, not lost: #740–#745, #747 (see the comment above), plus the pre-existing compiler-limit issues #703–#707.

CI is green on this head across all three workflows, including both live database suites. From our side this is ready to merge.