[dengn]

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

Closes #30059

What this PR does / why we need it:

Behavior and design

Legacy string APIs substituted markers inside SQL literals/comments and confused named prefixes such as :id and :id2. Sync/async clients and single/batch/vector builders now share lexical parameter rendering and typed literal/identifier encoding. Quoted bytes are preserved, inserted values are never rescanned, parameter mismatches and non-finite numbers fail explicitly, and backslash/NUL data is encoded independently of SQL mode. Nested NumPy vector elements and NumPy scalar types retain their supported behavior. The same scalar owner explicitly supports native TIME timedelta values and UUIDs. Duration formatting uses integer microseconds, preserves signed/fractional/extended hours without wrapping, and handles both Python extrema without float rounding. Ordinary positive-duration text retains the legacy 1:02:03 representation. UUIDs share the existing text quoting path; arbitrary objects and nested unsupported JSON types remain rejected. No new production helper, fallback, cache or conversion framework is introduced; the redundant explicit datetime check/import is removed because it is already covered by date. Native driver-binding API migration remains separate (#30069).

The renderer handles MatrixOne //, #, whitespace-qualified --, inert block comments, executable MySQL comments, quoted identifiers, and tagged/untagged dollar strings. Executable-comment version prefixes and decimal/signed-exponent boundaries are consumed before unquoted identifiers, preventing adjacent dollar openers from being swallowed. Dollar bodies end at the first dollar and closing tags must match, following the native scanner rather than PostgreSQL delimiter-search semantics. Unquoted identifier tokens containing dollars remain identifiers. Identifier builders reuse SQLAlchemy's MySQL keyword authority instead of a hand-picked list, covering window, rows, integer and related keywords while preserving ordinary ASCII formatting. This authority is not a complete MatrixOne-specific keyword catalogue.

Parent SQLAlchemy Session execution remains authoritative for hooks, routing, autoflush, options, result typing, transaction and connection ownership. Mode-sensitive raw SQL is rendered on the selected connection, including wrapped statements and unbound routed Sessions; snapshots forward original parameters to that same owner. The Engine listener immediately returns for ordinary executions. Only per-call deferred SDK executions inspect their graph, visiting each distinct node once and probing the executing connection's live mode once. There is no persistent mode cache or per-connection marker.

Rebased onto main bcefda15b8260bba4c71618e16fdf5715aaec003. Retained main's MERGE/PICK autocommit selection, restoration/disposal, cancellation and query deadline behavior. Mode discovery/rendering fits inside that existing connection owner; preparation failures release the checkout before changing autocommit. Restored all original merge-only NumPy/live-mode fixes explicitly; kernel fixes already in main are not replayed as a Go delta.

Validation

• New scalar regression reproduced through public insert/batch_insert: duration and UUID failed while date controls passed. After correction, current-main legacy builders and the new builders store identical positive-duration/UUID/date text through the same public SQLite execution path.
• Python 3.13.5, full offline suite plus the explicitly enabled native SDK module: SQLAlchemy 1.4.54: 1750 passed, 2.0.55: 1746 passed, 2.1.5: 1754 passed; each additionally has 95 passing subtests, zero failures/errors/skips. Each family includes 87 native tests.
• One reused native scalar table covers decoded TIME reinsertion plus UUID/date across all eight sync/async client/vector single/batch insertion paths. Invalid objects execute zero INSERTs for single rows, complete client batches and the first vector chunk; rows and pool capacity remain intact. Zero, negative microseconds, negative subhours, fractional extended durations, server range rejection and subsequent reuse pass. Existing later-vector-chunk rollback and caller-owned Session tests remain in place.
• Earlier lexical boundaries, numeric/dollar adjacency, live modes, routed/wrapped Sessions, snapshots, NumPy vectors, autocommit restoration/error/cancellation, unique-DAG listener work and ordinary-query no-traversal controls remain covered by the full matrix. No driver-binding API migration or persistent format change.
• Black, isort, fatal Flake8 and whole-PR git diff --check pass for all 15 changed Python files. Final wheel and sdist build successfully and both pass twine check. No Go files changed; Go race/SCA are inapplicable. Native SDK tests supply real frontend acceptance; no kernel BVT files changed.
• Previous-head (f72d078254) paired renderer-only measurements against the original PR head were 7.21 → 5.84 µs for a simple query, 67.40 → 78.46 µs for 100 parameters, and 3.05 → 2.75 µs for a 1 KB comment. These are historical local parsing measurements, not new-head server throughput claims. This follow-up adds no SQL-mode probes, graph traversal, state or IO; numeric early-return paths remain unchanged.
• Actual gpt-6.1-sol / xhigh design and final overall review are recorded outside tracked source. Review documents are not committed.

Initial lexical reproduction used pinned MatrixOne 4.2.5. Final native acceptance reused an isolated one-CN main-kernel fixture at 2f8011faecaa402801288ca4e34270d8bd404a92; its scanner, grammar and TIME parser are unchanged versus target bcefda15b8. The intervening MySQL command-executor change only affects DROP ACCOUNT and is outside this scalar-query acceptance. The full target kernel was not rebuilt. Only Python 3.13.5 was run locally; the repository's Python 3.10/3.11/3.12 CI matrix remains CI coverage.

Reviewer reassessment and new-head CI remain pending; no prior request-changes review was dismissed.

[XuPeng-SH]

mo-bot review · mo-bot-xupeng · done

Commit: c3bbb7f7af97a0d650742af94750be39ffa2783b

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-xupeng · review

Commit: c3bbb7f7af97a0d650742af94750be39ffa2783b

Assessment: request_changes

Reviewed the complete 13-file delta and supplied review concerns. All 237 focused tests pass, but the new global execution listener introduces a confirmed performance regression for ordinary SQLAlchemy queries.

Model: gpt-6-astra / xhigh

Session: 01a14d34-4e51-79a3-a62d-6c9c16279dc8 · slot 0 · generation 0

• P2 clients/python/matrixone/_session_execution.py:73: After an SDK Session raw-SQL execution installs the Engine-wide listener, execute ordinary cached Core/ORM statements—even on an unrelated engine and without deferred SDK literals.
  Impact: Every execution traverses the entire expression graph before checking whether rendering is needed. Shared CTE references cause exponential revisits, imposing substantial CPU and latency costs on unrelated queries. Restrict this work to executions containing deferred SDK literals and avoid repeated traversal of shared nodes.
  Evidence: Retained probe_shared_cte.py executes real SQLite queries with result assertions. An 883-character, 11-level shared-CTE query causes 20,477 visits and takes 77.48 ms median with the listener versus 38–52 µs when only that listener is removed. probe_regular_sql.py also measures 4.3× and 7.8× overhead for cached 50- and 500-predicate statements; reinstalling the listener restores the overhead.

Validation:

• Evidence retained under /home/xupeng/.local/state/mo-bot/jobs/bf7944239fa47c357c0fcf50/attempt-1; review-record.json maps commands, receipts, logs, artifact hashes, scope and comment reconciliation.

• focused.log and focused.xml confirm 237 passed, zero failures/errors/skips on Python 3.13.5 and SQLAlchemy 2.0.55. Receipt: checks/1792325437605442177-4.json.

• Performance probes completed successfully: regular-sql-cost.log with checks/1792325330066847415-4.json; shared-cte-cost.log with checks/1792325402297217116-4.json.

• Preflight passed. All four receipts identify the assigned head and clean source fingerprint; final checkout remained unchanged.

• Frozen evidence records six successful SDK offline CI cells. Source review and focused tests cover the earlier NumPy, SQL-mode, executable-comment, Session-routing and snapshot concerns.

Limitations:

• Performance measurements isolate client-side SQLAlchemy overhead using SQLite; they are not MatrixOne server throughput measurements.

• local_validation is empty. The author's reported 207 native passes lack supplied execution receipts, and online CI was skipped; native behavior was not independently revalidated.

• The standalone linked issue body was not supplied; its contract was assessed through the PR description, supplied comments and frozen source.

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: f4c3d8a4e829825b287cd6101ceaa875a580fa36

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-xupeng · superseded

Commit: f4c3d8a4e829825b287cd6101ceaa875a580fa36

"interrupted"

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: f4c3d8a4e829825b287cd6101ceaa875a580fa36

Assessment: request_changes

Reviewed the complete 14-file delta and affected callers. Centralized literal encoding and connection-local rendering are reasonable designs, and the earlier routing, ownership and listener-performance concerns are addressed. Two missing MatrixOne lexical forms introduce confirmed compatibility regressions.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1517-7f90-b3f9-c5f9f9e7031b · slot 0 · generation 1

• P2 clients/python/matrixone/_sql_literals.py:147: Call Client.execute("SELECT ? // ?\n", (7,)). MatrixOne recognizes // as a line comment, but the renderer counts its second question mark as another parameter.
  Impact: A previously valid query now fails with 'Not enough SQL parameters' before reaching the driver. Named markers inside these comments similarly cause missing-parameter errors. The shared renderer propagates this regression to client and session APIs.
  Evidence: The frozen server scanner handles // comments at pkg/sql/parsers/dialect/mysql/scanner.go:277–286. Public-entrypoint checks demonstrate eight cases passing on the frozen base and failing on this head. Recognize // comments through newline or EOF and add positional/named regression coverage. Reproductions and results are retained in test_review_lexical.py and lexical-comparison.json.

• P2 clients/python/matrixone/_sql_literals.py:130: Call Client.execute("SELECT $$O'Reilly$$, ?", (7,)), or use a tagged dollar string with a named parameter outside it.
  Impact: The apostrophe inside the dollar string incorrectly starts an ordinary quoted region, hiding the actual parameter. Previously valid queries now fail with 'Too many SQL parameters' or unused named-parameter errors.
  Evidence: Dollar strings are supported by scanner.go:215–233 and 543–575, with parser cases in mysql_sql_test.go:4957–4965. Eight public-entrypoint cases pass on the frozen base and fail on this head. Recognize tagged and untagged dollar-string boundaries before interpreting their contents; add apostrophe and parameter-marker coverage. Results are retained in lexical-comparison.json.

Validation:

• 244 focused SDK tests passed on Python 3.13.5 / SQLAlchemy 2.0.55.

• Public API regression matrix: head 20 failed/10 passed; frozen base 4 failed/26 passed. Comparison confirms 16 new failures and 10 passing controls. Four preexisting async named-parameter failures were excluded from findings.

• Listener enabled/disabled median costs were 30.06/30.09 µs for 50 predicates, 243.20/243.68 µs for 500 predicates, and 9398.65/9427.61 µs for an 11-level shared CTE. These SQLite measurements show no meaningful ordinary-path overhead.

• Reconciled eight command/source/exit receipts with retained logs and verified all 49 extracted base SDK files. Final audit confirmed the exact head, clean source and passing diff check.

• Evidence and change-map records: /home/xupeng/.local/state/mo-bot/jobs/09769c148ad53130e3b85f6f/attempt-1/evidence-index.json and review-record.json. Final audit receipt: checks/1792383819570984969-5.json.

• Frozen CI evidence reports six successful offline matrix cells; online validation was skipped.

Limitations:

• Regression checks exercised real SDK and SQLAlchemy entrypoints with explicit SQLite syntax translation. MatrixOne syntax support was verified against frozen scanner/parser source; no native server execution was performed locally.

• local_validation is empty. Author-reported native results lack retained receipts here and were not treated as independently verified execution.

• The standalone linked issue #30059 body was not supplied; its contract was assessed from the frozen PR description and comments.

[aptend]

Reviewed exact head f4c3d8a4e829825b287cd6101ceaa875a580fa36 against exact base 3fc00709eccc277c175c7a09eed2605ca1aaee29. REQUEST_CHANGES: two current-head lexical regressions remain independently reproducible on a real MatrixOne CN.

1. [P2] Exclude native // line comments from parameter rendering — clients/python/matrixone/_sql_literals.py:147.

client.execute("SELECT ? // ?\n", (7,))
client.execute("SELECT :v // :ignored\n", {"v": 7})

MatrixOne accepts // comments through newline/EOF. The renderer recognizes only # and --, so it interprets inert comment markers as executable parameters. These valid calls now raise Not enough SQL parameters / Missing named SQL parameter: ignored before execution. Recognize the server's // boundary and retain comment bytes; cover positional/named markers and newline/EOF controls.

2. [P2] Preserve tagged and untagged dollar-string boundaries — clients/python/matrixone/_sql_literals.py:130.

client.execute("SELECT $$O'Reilly$$ AS marker, ? AS value", (7,))
client.execute("SELECT $txt$O'Reilly$txt$ AS marker, :v AS value", {"v": 7})

Both are valid MatrixOne SQL. The apostrophe inside the dollar string incorrectly starts an ordinary quoted region, hiding the real outside marker. Calls fail with Too many SQL parameters / unused named parameters. Recognize actual server dollar-string delimiters before interpreting their contents; preserve apostrophes and placeholder-looking literal data.

Fresh causal evidence: the unchanged 20-case public API matrix runs against the same native server built from this candidate. Exact-base SDK: 20/20 passed. Candidate SDK: 16 failed / 4 raw server controls passed, exactly eight failures for each finding. Coverage includes Client, both synchronous Session exports, AsyncClient and AsyncSession; async named-parameter cases were deliberately excluded because the base does not support them. Controls directly execute rendered native // and tagged/untagged dollar SQL and verify returned values. This uses no SQLite syntax translation or injected mode. Only SDK source differs between the separate head/base worktrees; the complete PR diff is under clients/python, so native/kernel inputs match.

Full review/evidence:

• All 14 files and the latest two-file delta since c3bbb7f7af; all 4 reviews, 5 discussion comments, 3 inline comments and 3 unresolved threads; complete issue #30059 contract.
• Independent design-first architecture assessment: shared literal owner and selected-connection mode discovery are appropriate; authoritative parent Session routing/options retained. The two lexical gaps violate the stated preservation contract.
• Fresh full offline suites: SQLAlchemy 1.4.54: 1261, 2.0.55: 1257, 2.1.5: 1265, each plus 86 subtests, no failures/skips (Python 3.13.5). Fresh committed native test file: 44/44 passed.
• Historical NumPy-vector, quote/NBSE, executable-comment, hook-routing and snapshot issues are closed in the inspected code and fresh relevant tests. The new listener's per-call guard, unique DAG discovery and next-unmarked-call controls pass; no ordinary-query graph walk or persistent mode cache remains.
• Exact-head CI 39197235400 has six successful offline SDK cells; online jobs were skipped. Passing those suites does not cover the counterexamples above.

Unhappy-path audit	Evidence
Q1 ownership	Parent Session/actual connection remains authoritative; renderer shared; no global mode owner.
Q2 preparation failure/cleanup	Existing result-owned connection cleanup and public error paths checked; native fixture/database cleaned afterward.
Q3 reuse/growth	Copied options, unique-node discovery and subsequent unmarked-query controls pass. No fresh throughput claim.

No source fixes or pushes. Existing reviewers' findings were checked independently, not used as a reason to omit this review.

[XuPeng-SH]

I found one identifier regression that this renderer still misses: window is a MatrixOne/MySQL parser keyword (WINDOW is a token in pkg/sql/parsers/dialect/mysql/keywords.go and is not accepted by the grammar's ident alternatives), but it is absent from _RESERVED. Consequently BaseMatrixOneClient._build_insert_sql('t', {'window': 1}) emits INSERT INTO t (window) VALUES (1) instead of quoting the column, which fails for a table that has a quoted window column. Please derive the reserved set from the parser policy or add a regression for this keyword (and similar unlisted reserved words).

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: f72d078254520892c8efba75f54d7e13af921c3f

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: f72d078254520892c8efba75f54d7e13af921c3f

Assessment: request_changes

Reviewed the complete 15-file delta and supplied concerns. Shared encoding and rendering on the parent-selected connection are sound designs; earlier lexical, routing and listener-cost concerns are addressed. One confirmed scalar compatibility regression remains.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1517-7f90-b3f9-c5f9f9e7031b · slot 0 · generation 1

• P2 clients/python/matrixone/_sql_literals.py:94: Call Client.insert or batch_insert, or their asynchronous equivalents, with datetime.timedelta(hours=1, minutes=2, seconds=3) or a uuid.UUID value. PyMySQL returns TIME values as timedelta.
  Impact: The new encoder raises TypeError before execution, breaking previously successful inserts and reinsertion of TIME values. Preserve these scalar types in the shared encoder and add public insertion regressions.
  Evidence: probe_scalar_compatibility.py exercises actual sync/async insert and batch_insert APIs using SQLite transport. All 12 cases pass with the exact merge-base SDK; the candidate fails eight timedelta/UUID cases with 'Unsupported SQL parameter type', while four date controls pass. Results are retained in scalar-{base,head}-verified.json; receipts are checks/1792947598284379671-3.json and checks/1792947597281647672-3.json.

Validation:

• 402 focused offline tests passed with zero failures, errors or skips; receipt checks/1792947170547423873-4.json matches focused.log and focused.xml.

• Base/candidate compatibility probes and wrapped-result controls were reconciled with explicit import paths, source hashes, exit receipts and logs.

• Frozen evidence reports six successful SDK offline CI matrix cells; online CI was skipped.

• Preflight and git diff --check passed; source remains unchanged. Evidence index: /home/xupeng/.local/state/mo-bot/jobs/749d256e6ae252c828b4c511/attempt-1/review-record.json.

Limitations:

• No fresh native-server run. SQLite probes establish the pre-driver rejection, not native TIME/UUID round trips.

• Author native-test and benchmark claims lack retained receipts in the supplied evidence and were not counted as independently verified results.

• The linked issue's standalone body was absent; the PR contract and all supplied comments and reviews were assessed.

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: 997a69d1a944cb8109fc11668199cd4c4238886b

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: 997a69d1a944cb8109fc11668199cd4c4238886b

Assessment: approve

Reviewed the complete 15-file delta, affected callers and supplied concerns. Shared literal encoding and rendering on the parent-selected connection are sound designs. Earlier lexical, routing, listener-cost and timedelta/UUID findings are addressed. No actionable defects remain.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1517-7f90-b3f9-c5f9f9e7031b · slot 0 · generation 1

Validation:

• 416 focused tests passed with zero failures, errors or skips on Python 3.13.5 / SQLAlchemy 2.0.55. focused.log and focused.xml match receipt checks/1792957638127182343-3.json.

• All eight public sync/async client/vector insertion paths passed 32 SQLite scalar round trips plus rejection-before-write and pool-return checks. Receipt: checks/1792957720614809744-4.json. An initial reviewer-fixture failure was retained separately and corrected.

• Confirmed ordinary SQLAlchemy queries avoid expression traversal; deferred discovery visits shared nodes once. Mode-sensitive execution retains routing, options, cancellation and cleanup behavior.

• Frozen evidence records six successful exact-head SDK offline CI cells. Preflight and diff checks passed; source remains unchanged.

• Commands, source fingerprints, exits and retained logs were reconciled in /home/xupeng/.local/state/mo-bot/jobs/a7d5c11afe914bbee2624df4/attempt-1/review-record.json. Final audit receipt: checks/1792957839977611432-4.json.

Limitations:

• No fresh native MatrixOne execution. SQLite checks establish SDK serialization and lifecycle behavior, not native TIME/UUID storage. Online CI was skipped; author-reported native runs lack supplied receipts.

• No current-head throughput measurement; historical benchmark claims were not treated as fresh results.

• The standalone linked issue body was absent; its contract was assessed through the frozen PR description, comments and source.

resolved

[mergify]

Merge Queue Status

• ✅ Entered queue — 2026-10-26 01:29 UTC · Rule: main · triggered by rule Automatic queue on approval for main
• ✅ Checks skipped · PR is already up-to-date
• ✅ Merged — 2026-10-26 01:30 UTC · at 0e8763ae9769574d7a68414d894fe158ef0137b8 · squash

This pull request spent 11 seconds in the queue, with no time running CI.

Required conditions to merge

• #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • fix(python-sdk): preserve SQL parameter boundaries and safe literals #30073
• github-review-approved [🛡 GitHub branch protection]
  • fix(python-sdk): preserve SQL parameter boundaries and safe literals #30073
• any of [🛡 GitHub branch protection]:
  • check-success = CI Required
  • check-neutral = CI Required
  • check-skipped = CI Required