fix(model): Oracle insertAll uses INSERT ALL form to avoid RETURNING-with-VALUES error

[wheels-bot]

On BoxLang × Oracle, model.insertAll() was erroring with ORA: returning clause is not allowed with INSERT and Table Value Constructor for multi-row batches and ORA: no statement parsed for single-row batches. The bulk-insert SQL was always emitted as the SQL-standard multi-row table value constructor — INSERT INTO t (cols) VALUES (?,?), (?,?), ... — which Oracle 23 rejects in combination with the JDBC driver's implicit RETURN_GENERATED_KEYS handling: the driver expands RETURN_GENERATED_KEYS into a RETURNING clause, and Oracle 23 disallows RETURNING paired with multi-row VALUES.

This PR moves bulk-insert SQL building off the model mixin (vendor/wheels/model/bulk.cfc::$buildBulkInsertSQL, removed) onto the database adapter ($bulkInsertSQL on databaseAdapters/Base.cfc), mirroring the existing $upsertSQL adapter-override pattern. OracleModel overrides it with INSERT ALL INTO t (cols) VALUES (...) ... SELECT 1 FROM dual — Oracle's idiomatic multi-row insert form, which neither uses the table value constructor nor triggers the RETURNING expansion. Non-Oracle adapters (MySQL, Postgres, SQLite, H2, SQL Server, CockroachDB) keep the standard multi-row VALUES shape unchanged.

The migrator-rename Closed statement error in the same compat-matrix run is a separate Oracle JDBC lifecycle issue and is intentionally left for a follow-up — it touches vendor/wheels/migrator/**, which falls under the bot's safety-net policy requiring human-in-the-loop review before code is written. The parent issue tracks both.

Fixes #2745

Checklist

• Tests — new adapter-level SQL-builder spec at vendor/wheels/tests/specs/model/oracleBulkInsertSqlSpec.cfc (5 cases). Failing pre-fix: Component [wheels.databaseAdapters.Oracle.OracleModel] has no function with name [$bulkInsertSQL] × 5. Passing post-fix: 5/5. End-to-end coverage on the live Oracle datasource continues via bulkOperationsSpec.cfc (the original 5 failing cases this issue reports).
• Framework Docs — handled separately by bot-update-docs.yml
• AI Reference Docs — handled separately by bot-update-docs.yml
• CLAUDE.md — handled separately by bot-update-docs.yml
• CHANGELOG.md
• Test runner passes — bash tools/test-local.sh model → 835 passed, 0 failed, 0 errors (was 830 pass, 0 fail, 5 error pre-fix; the 5 errors were the new spec demonstrating the missing adapter method)

[wheels-bot]

Wheels Bot — Docs updated

Added a doc commit to this PR:

• .ai/wheels/cross-engine-compatibility.md — added Oracle subsection under "Database-Specific Gotchas" explaining why Oracle 23 rejects multi-row VALUES with RETURN_GENERATED_KEYS, and pointing to OracleModel.$bulkInsertSQL as the canonical adapter-override pattern

No MDX user-guide page needed (no public API change — insertAll() behavior is unchanged from the caller's perspective). CLAUDE.md unchanged (no model/controller/view convention change).

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: This PR correctly fixes a real Oracle 23 × JDBC incompatibility in insertAll() by moving bulk-insert SQL generation onto the adapter layer (mirroring $upsertSQL) and overriding it in OracleModel to use Oracle's idiomatic INSERT ALL ... SELECT 1 FROM dual form. The implementation is clean, the design is architecturally consistent, tests cover the SQL-builder contract on every engine, and the commit history is tidy. Verdict: approve (blocked from self-approval by GitHub — human reviewer may merge).

---

Correctness

No issues found.

• SQL shape is valid Oracle. INSERT ALL INTO t (cols) VALUES (...) INTO t (cols) VALUES (...) SELECT 1 FROM dual is the correct Oracle multitable-insert form and does not trigger the JDBC driver's RETURN_GENERATED_KEYS → RETURNING expansion that Oracle 23 rejects with multi-row VALUES.
• Single-row batch is covered. The Oracle form degenerates correctly to one INTO clause — the old INSERT INTO t VALUES (?) path was also broken (ORA: no statement parsed), and the new path avoids it.
• insertAll does not consume the generated-key result. bulk.cfc:60 computes totalInserted from the batch bounds, so the absence of RETURN_GENERATED_KEYS with INSERT ALL has no user-visible effect.
• $quoteColumn → $quoteIdentifier substitution is safe. The removed $buildBulkInsertSQL called $quoteColumn(local.col); miscellaneous.cfc:198 shows $quoteColumn is a thin wrapper around variables.wheels.class.adapter.$quoteIdentifier(). The new Base.cfc:761 calls $quoteIdentifier(local.col) directly on the adapter — identical behavior.

---

Cross-engine

No issues found.

• Oracle's $quoteIdentifier (OracleModel.cfc:173) uppercases and double-quote-wraps identifiers — both the column list ($quoteIdentifier inside $bulkInsertSQL) and the table name ($quotedTableName() at the call site in bulk.cfc:46) go through the correct engine-specific path.
• Non-Oracle adapters inherit the Base.cfc default, which keeps the existing multi-row VALUES shape unchanged.
• No obj["key"]() bracket-call anti-pattern, no inline-closure-as-constructor-arg, no Left(str, 0) risk.

---

Tests

No issues found.

• oracleBulkInsertSqlSpec.cfc extends wheels.WheelsTest and uses BDD syntax.
• 5 cases cover: INSERT ALL shape, one INTO per record, parameterized values (confirms no inline interpolation of user data), single-row batch, and the non-Oracle base-adapter shape.
• Instantiating OracleModel directly (no datasource required for SQL-builder methods) is the right approach for adapter-level unit tests; the live Oracle end-to-end path remains in bulkOperationsSpec.cfc.
• No unescaped # in string literals in the new spec.

---

Commits

Clean. Single commit fix(model): Oracle insertAll uses INSERT ALL form to avoid RETURNING-with-VALUES error — type fix, scope model, subject under 100 chars, body explains the why (JDBC RETURN_GENERATED_KEYS to RETURNING expansion), DCO Signed-off-by trailer present.

---

Docs

• CHANGELOG.md [Unreleased] entry present.
• Framework/AI/CLAUDE.md docs deferred to bot-update-docs.yml — noted in checklist.

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is accurate and well-evidenced. No sycophancy, no false positives. Two minor observations below, neither of which affect the correctness verdict.

Sycophancy

None detected. A verifies every claim against specific diff lines and explains the reasoning.

False positives

None detected.

• $quoteColumn → $quoteIdentifier claim — verified: vendor/wheels/model/miscellaneous.cfc:197-199 confirms $quoteColumn is exactly a one-line wrapper around variables.wheels.class.adapter.$quoteIdentifier(). A's equivalence claim is correct.
• $buildBulkParam availability — verified: Base.cfc:826 defines the method, so OracleModel inherits it directly. The adapter-level unit tests can instantiate OracleModel without a live datasource and call $bulkInsertSQL cleanly.
• Oracle INSERT ALL shape — verified against the diff. The generated SQL closes each record's VALUES tuple before the next INTO clause and appends SELECT 1 FROM dual at the end. Valid Oracle multitable-insert syntax.

Missed issues

Two items A didn't raise, both minor:

1. Docs are already in the PR — A's verdict says they're deferred. A's Docs section reads: "Framework/AI/CLAUDE.md docs deferred to bot-update-docs.yml — noted in checklist." But bot-update-docs.yml already ran and committed the .ai/wheels/cross-engine-compatibility.md Oracle subsection (97d638..., the current HEAD). The checklist item is ticked in the PR body and the diff shows the doc addition. A's description is stale — the docs are done, not deferred. This does not change the approve verdict.

2. A's SHA marker in the review body references a stale commit. The marker reads wheels-bot:review-a:2749:a4c819b4ba39b47ffb1f1b676e643578cd9f00f2 but the GitHub API reports the review's commit_id as 97d6380631bd2722eb96b58b5f53cb19600df8cd (the docs commit, current HEAD). Review A ran against the correct HEAD but stamped the previous commit's SHA into its own marker. Future pipeline runs keying off the body marker rather than the API's commit_id would see the current SHA as unreviewed and re-trigger. Not a blocker for merging, but worth noting for the bot pipeline.

Verdict alignment

A's approve is consistent with the findings: the implementation is correct, tests cover the SQL-builder contract, no cross-engine regressions introduced, and docs are present.

Convergence

Aligned. The two observations above are not actionable blockers — one is a stale description in an otherwise correct review, and the other is a pipeline meta-issue. The joint recommendation is approve. The PR is review-clean on this SHA; a human reviewer may merge.

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: The previous review (at a4c819b4b) covered the core fix in full. The only change since is commit 97d638063, a clean docs-only addition to .ai/wheels/cross-engine-compatibility.md documenting the Oracle INSERT ALL constraint for future adapter authors. No correctness, cross-engine, security, or test findings on the full PR. Verdict: approve (blocked from self-approval by GitHub — human reviewer may merge).

---

Correctness

No issues.

• SQL shape is valid Oracle. INSERT ALL INTO t (cols) VALUES (...) SELECT 1 FROM dual is the correct Oracle multi-table-insert form. It avoids both the table value constructor and the JDBC RETURN_GENERATED_KEYS → RETURNING expansion that Oracle 23 rejects.
• $identitySelect does not fire for INSERT ALL. Both Base.cfc:179 and OracleModel.cfc:124 guard with Left(local.sql, 11) == "INSERT INTO". The Oracle INSERT ALL output starts with "INSERT ALL" (11 chars = INSERT ALL ≠ INSERT INTO), so the identity-select branch is correctly skipped for bulk inserts — consistent with insertAll() computing totalInserted from batch bounds, not from generated keys.
• Values are parameterized. OracleModel.cfc:224 and Base.cfc:778 both route every value through $buildBulkParam(), which returns a typed parameter struct. The table name (arguments.tableName) comes from $quotedTableName() at the call site (bulk.cfc:46), not user input.
• Docs commit is accurate. The new section in .ai/wheels/cross-engine-compatibility.md correctly describes the JDBC RETURN_GENERATED_KEYS → Oracle RETURNING expansion, points to OracleModel.cfc::$bulkInsertSQL as the canonical reference, and gives actionable guidance for future adapter authors.

---

Cross-engine

No issues.

• Non-Oracle adapters inherit the unchanged Base.cfc multi-row VALUES path. The Oracle override is isolated to OracleModel.cfc.
• $quoteIdentifier is called on the adapter instance in both Base.cfc:761 and OracleModel.cfc:210, so identifier casing (Oracle uppercases) is handled correctly per engine.
• No obj["key"]() bracket-call pattern, no inline-closure-as-constructor-arg, no Left(str, 0) risk, no private mixin issue ($bulkInsertSQL is correctly declared public).

---

Tests

No issues.

• oracleBulkInsertSqlSpec.cfc extends wheels.WheelsTest and uses BDD syntax (5 cases: INSERT ALL shape, one INTO per record, parameterized values, single-row batch, non-Oracle base shape).
• Direct adapter instantiation without $init() is safe: $buildBulkParam is a pure function that only reads the passed-in propertyInfo struct — no datasource access.
• No unescaped # in string literals. Double-quoted Oracle identifiers are correctly represented as ""AUTHORS"" in CFML string literals, yielding "AUTHORS" in the actual value.
• The expect(collapsed).notToMatch(...) regex correctly targets the multi-row tuple-list shape that triggers the Oracle JDBC bug.

---

Commits

Clean.

• fix(model): Oracle insertAll uses INSERT ALL form to avoid RETURNING-with-VALUES error — type fix, scope model, body explains the JDBC RETURN_GENERATED_KEYS → RETURNING expansion, DCO Signed-off-by present.
• docs: note Oracle INSERT ALL requirement for bulk insert in cross-engine guide — type docs, no scope (valid), 66-char subject, DCO Signed-off-by present.
• Both sign-offs match the configured git author (claude[bot]).

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: The two prior reviews (at a4c819b4b and 97d638063) covered the substantive Oracle insertAll fix in full. The only change since is commit bc75bc5b1, a pre-merge sync that merges develop into the branch — bringing in three other already-merged PRs (#2748 MySQL text-default carve-out, #2743 CockroachDB advisory-locks flag, and the cross-engine-compat docs from 97d638063). The merge is clean: no conflict resolutions, only parallel additions to CHANGELOG.md and .ai/wheels/cross-engine-compatibility.md. No correctness, cross-engine, security, or test issues on the full PR. Verdict: approve — PR is already merged; this review records the final-state assessment.

---

Correctness

No issues.

• Merge is additive-only. bc75bc5b1 merges 97d638063 (PR branch) + 76f3b8f9c (develop tip). The combined diff is a clean union: the Oracle INSERT ALL CHANGELOG entry from the PR and the MySQL/CockroachDB entries from develop land in adjacent lines with no conflicts resolved by overwriting.
• No functional code changed in the merge commit. The five files in git show bc75bc5b1 --stat are CHANGELOG.md, .ai/wheels/cross-engine-compatibility.md, CockroachDBModel.cfc, MySQLMigrator.cfc, and addColumnOptionsSpec.cfc — none of which are part of the Oracle bulk-insert fix itself. CockroachDBModel.cfc and MySQLMigrator.cfc arrive unchanged from develop (already reviewed in #2743 / #2748).
• Prior analysis still holds. OracleModel.$bulkInsertSQL emits the correct INSERT ALL INTO ... SELECT 1 FROM dual shape, avoiding the Oracle 23 JDBC RETURN_GENERATED_KEYS -> RETURNING expansion. Base.cfc.$bulkInsertSQL keeps the standard multi-row VALUES path for all other adapters. bulk.cfc delegates to the adapter instance (variables.wheels.class.adapter.$bulkInsertSQL(...)), correctly passing tableName and propertyInfo that the old mixin-level $buildBulkInsertSQL read from this scope.

---

Commits

Clean. Two substantive commits (fix(model), docs) plus the pre-merge sync. All three have DCO Signed-off-by trailers. Subject lines are under 100 characters and not ALL-CAPS.

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review of the merge-sync commit is accurate and terse. No sycophancy, no false positives, no missed issues. Aligned on approve.

Sycophancy

None detected. A's approval cites specific commit analysis — the merge is additive-only, no conflicts resolved by overwriting, no functional code changed in the Oracle bulk-insert fix itself.

False positives

None detected. A's file list for the merge commit (CHANGELOG.md, .ai/wheels/cross-engine-compatibility.md, CockroachDBModel.cfc, MySQLMigrator.cfc, addColumnOptionsSpec.cfc) is consistent with what #2748 and #2743 contributed to develop. The Oracle fix files (Base.cfc, OracleModel.cfc, bulk.cfc) were already on the feature branch prior to the sync and correctly do not appear in the merge commit's stat.

Missed issues

None detected.

Verdict alignment

A submitted a COMMENTED-type review (not APPROVED) because GitHub disallows APPROVE on a merged PR — A explains this explicitly. The approve verdict in the body is consistent with the findings.

Convergence

Aligned. The prior B round on SHA 97d638063 converged-approve on the substantive Oracle fix. A's review of the merge-sync commit adds no new concerns. Joint recommendation: approve.