Verify expected update counts when automatic DML batches flush

[apstndb]

Summary

Adds AUTO_BATCH_DML_UPDATE_COUNT (INT64, default 1) and AUTO_BATCH_DML_UPDATE_COUNT_VERIFICATION (BOOL, default FALSE) for existing automatic DML batching.

Verification is opt-in so current arbitrary UPDATE/DELETE statements keep succeeding. Each queue entry freezes its expected-count policy at enqueue. A successful BatchUpdate compares each enabled actual count before a journal receipt; [1,1] vs [0,2] fails at statement 1. RPC and partial BatchUpdate errors keep their original cause. Mismatch uses the existing SAVEPOINT failure/recovery path. Replay still checks journaled actual counts after verification is turned off.

Manual, THEN RETURN, unbuffered, and partitioned DML are unchanged. No TRANSACTION_TIMEOUT / #482 owner-deadline edits.

Fixes #401

Test plan

• Focused short tests for per-entry match/mismatch, zero rows, frozen policy, RPC failure, SAVEPOINT recovery, SELECT/SAVEPOINT/RUN BATCH/COMMIT flush, enqueue presentation, and replay actual-count check
• make check before push
• make check-race for flush/owner lifecycle
• Coverage of transaction_manager_auto_dml.go at or above 80%

  

[cursor]

Additive merge of origin/main 6b9600acf40d1eb8b18eeab845b3141dc68a9305 (#977) is on this branch.

New HEAD: 368745d355b8d61a115988385abdadb8141e3bf4

Published implementation tip 776e3bab6b03d1c65483234d130fec1478d42369 is still an ancestor (no reset, rebase, or force-push). Shared docs/registry conflicts were resolved additively so both AUTO_BATCH_DML_UPDATE_COUNT* and KEEP_TRANSACTION_ALIVE remain. No TRANSACTION_TIMEOUT / owner-deadline files were edited beyond accepting #977's keepalive merge.

make check exit 0 (/workspace/.tmp/driver-parity-401/make-check-merge-977.log). make check-race exit 0 (/workspace/.tmp/driver-parity-401/make-check-race-merge-977.log).

[github-actions]

Code Metrics Report

📊 View detailed coverage report (available for 7 days)

	main (6b9600a)	#979 (fa3f3a9)	+/-
Coverage	86.5%	86.5%	+0.0%
Code to Test Ratio	1:2.2	1:2.3	+0.0
Test Execution Time	1m42s	1m59s	+17s

Details

  |                     | main (6b9600a) | #979 (fa3f3a9) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          86.5% |          86.5% | +0.0% |
  |   Files             |            113 |            113 |     0 |
  |   Lines             |          10678 |          10715 |   +37 |
+ |   Covered           |           9242 |           9279 |   +37 |
+ | Code to Test Ratio  |          1:2.2 |          1:2.3 |  +0.0 |
  |   Code              |          23860 |          23964 |  +104 |
+ |   Test              |          54538 |          55774 | +1236 |
- | Test Execution Time |          1m42s |          1m59s |  +17s |

Code coverage of files in pull request scope (91.1% → 91.4%, patch 100.0%)

Files	Coverage	+/-	Patch Coverage	Status
internal/mycli/client_side_statement_def.go	91.1%	+0.1%	-	affected
internal/mycli/execute_partitioned.go	90.6%	-0.9%	-	affected
internal/mycli/session_transaction_context.go	84.2%	0.0%	-	modified
internal/mycli/statements_system_variable.go	92.2%	-0.4%	-	affected
internal/mycli/system_variables.go	96.5%	0.0%	100.0%	modified
internal/mycli/transaction_manager_auto_dml.go	96.5%	+1.0%	100.0%	modified
internal/mycli/var_defs.go	86.1%	+0.3%	100.0%	modified
internal/mycli/var_handler.go	88.4%	+1.2%	-	affected
internal/mycli/var_registry.go	95.3%	+0.9%	-	affected

---

Reported by octocov

[apstndb]

Independent coordinator confirmation on exact HEAD 368745d355b8d61a115988385abdadb8141e3bf4 (base 6b9600acf40d1eb8b18eeab845b3141dc68a9305, #977).

Additive merge of origin/main. Worker reports verification default FALSE, expected count default 1, policy frozen per queue entry; original RPC/partial failure and SAVEPOINT recovery preserved; no TRANSACTION_TIMEOUT/owner-deadline edits; local make check and make check-race exit 0. All applicable hosted checks succeeded on that SHA (test, lint, race, readme-sync, govulncheck, all four cross-compile jobs). Dependabot automerge skipped. MERGEABLE CLEAN. No unresolved review threads.

{"repo":"apstndb/spanner-mycli","pr":979,"head":"368745d355b8d61a115988385abdadb8141e3bf4","base":"6b9600acf40d1eb8b18eeab845b3141dc68a9305","validation":["hosted test/lint/race/readme-sync/govulncheck/cross-compile success on exact HEAD","worker local make check exit 0","worker local make check-race exit 0","MERGEABLE CLEAN","no unresolved review threads"],"deviations":[],"blockers":[]}

[apstndb]

Reviewed feature HEAD 776e3bab6b03d1c65483234d130fec1478d42369. The production path captures each entry's policy and checks counts before the successful journal receipt, preserving RPC errors. No production defect is established so far. Two acceptance-test gaps remain before final review:

1. TestAutomaticDMLExpectedCountRPCFailurePreserved injects a transport-level RPC error (ExecuteBatchDml returns nil, err). It does not exercise the Issue's successful-prefix/partial-statement failure contract. Add a fake response with one successful ResultSet/count and a non-OK embedded response Status for the next statement. Make the successful prefix's count differ from its expectation; assert that the original statement-error code/cause wins over count verification, the queue is cleared, no successful batch receipt is retained, and COMMIT is not issued for that failed attempt. Exercise the valid-marker recovery boundary as well as the no-marker case if the existing harness makes this compact.
2. The frozen-policy test mutates struct fields directly. Add one end-to-end policy scenario using the actual SET/SET LOCAL/RESET ALL paths (single-variable RESET can be included after feat: restore one session variable with RESET <name> #978): enqueue entries with differing captured policies, reset/change current defaults before flushing, verify the old entries keep their expectations and later entries use the new values, and verify transaction-end LOCAL restoration. This checks the new settings' integration with the reset/local infrastructure rather than only the entry helper.

Keep the additions focused; no new production abstraction is needed merely for tests. Final integration order is #978 then this PR. Continue the review fixes now; defer a new final main merge until #978 merges. If a merge is already committed/pushed, preserve it and report it: a wait instruction never authorizes resetting, rebasing or force-pushing published history. Run make check before each push and own required CI as usual. Codex retains exact-head semantic acceptance and squash merging.

[apstndb]

Withdrawing the ready JSON for 368745d355b8d61a115988385abdadb8141e3bf4 pending Codex's two acceptance-test gaps. Published additive merge 368745d is preserved (no reset/rebase/force-push). Fixes will be additive commits. No new main merge until #978 squash-merges.

[cursor]

Codex acceptance-test gaps from comment 5665017247 are covered additively.

New HEAD: 322c951a59c7a0a4a449b97cfd681b622e8fd58a

• beae9a7 — partial BatchUpdate (one successful ResultSet plus embedded non-OK Status; prefix count differs from expectation) covering no-marker and valid-marker recovery; SET / SET LOCAL / RESET ALL frozen-policy scenario
• f4cdbf8 — additive merge of origin/main bc7d76c22fa11841e516ed26d38bf0bffe4595a0 (feat: restore one session variable with RESET <name> #978)
• 322c951 — freeze test now drives single-var RESET through the feat: restore one session variable with RESET <name> #978 RESET <name> statement

Published 368745d355b8d61a115988385abdadb8141e3bf4 and implementation tip 776e3bab6b03d1c65483234d130fec1478d42369 remain ancestors (no reset, rebase, or force-push). No production abstraction was added.

make check exit 0 (/workspace/.tmp/driver-parity-401/make-check-merge-978.log). make check-race exit 0 (/workspace/.tmp/driver-parity-401/make-check-race-merge-978.log).

[apstndb]

Independent review of exact HEAD 322c951a59c7a0a4a449b97cfd681b622e8fd58a: no actionable findings remain. The two validation gaps from comment 5665017247 are addressed by beae9a7 and the actual single-variable RESET follow-up in 322c951.

The partial-batch tests now return a successful prefix plus a non-OK embedded response Status and verify original error precedence, queue/journal cleanup, no failed-attempt commit, and valid-marker recovery. The SET/SET LOCAL/RESET scenario verifies captured per-entry policy and later LOCAL restoration. I independently ran those tests normally and with the race detector. A separate independent negative probe also confirmed that RESET ALL disabling future verification does not let an already-queued, previously enabled mismatch succeed.

Independent focused tests, full Docker make check, and make check-race passed on feature HEAD 776e3ba. I reviewed the keepalive integration conflicts and the test-only review fixes separately. The final main integration matches Git's automatic merge of reviewed beae9a7 and main bc7d76c; current-head policy/reset/default tests passed. The worker reports both full gates passed on current HEAD. All applicable hosted checks passed, with no unresolved review threads or review-level findings.