Skip to content

feat: restore one session variable with RESET <name> - #978

Merged
apstndb merged 3 commits into
mainfrom
cursor/reset-single-960-c42b
Sep 14, 2026
Merged

apstndb merged 3 commits into
mainfrom
cursor/reset-single-960-c42b

Conversation

@apstndb

@apstndb apstndb commented Sep 14, 2026

Copy link
Copy Markdown
Owner

Summary

Adds RESET <name> as the single-variable companion to RESET ALL (#484 / #976).

Fixes #960.

Behavior

  • Reuses the existing startup snapshot, prepareReset, and commitPersistentReset path. The SQL statement does not call the low-level Reset(name) wrapper, which does not retire SET LOCAL undo.
  • Restores the post-default/config/flag/--set connection-start value, including --set overrides. Init-command assignments remain resettable.
  • Resolves canonical names and aliases. Unknown and unresettable names fail without mutation.
  • Resetting one variable leaves others unchanged.
  • Successful RESET retires only that variable's LOCAL undo (including equal-value resets) and preserves unrelated undo.
  • Rejected or guarded RESET mutates neither values nor undo.
  • TRANSACTION_TAG continues to compare and restore the writable next-owner slot (ResetSnapshot), not SHOW.
  • RESET LOCAL and SET x=DEFAULT are not implemented.

Docs

Validation

  • Focused unit/fake-RPC tests for isolation, aliases, excluded names, guards, LOCAL undo, --set startup, DDL vars, and TRANSACTION_TAG slot semantics
  • make check exit 0 (log: .tmp/driver-parity-960/make-check.log)
  • ./internal/mycli short coverage 82.4% (floor 80%)
  • make check-race not run: no transaction concurrency or lifecycle changes
Open in Web Open in Cursor 

Add RESET <name> on the existing prepareReset/commitPersistentReset
startup-snapshot path so a successful reset retires only that
variable's LOCAL undo. RESET LOCAL and SET x=DEFAULT stay out of scope.

Fixes #960

Co-authored-by: apstndb <apstndb@users.noreply.github.com>
@github-actions

github-actions Bot commented Sep 14, 2026

Copy link
Copy Markdown

Code Metrics Report

📊 View detailed coverage report (available for 7 days)

main (6b9600a) #978 (96bc201) +/-
Coverage 86.5% 86.5% +0.0%
Code to Test Ratio 1:2.2 1:2.3 +0.0
Test Execution Time 1m42s 1m24s -18s
Details
  |                     | main (6b9600a) | #978 (96bc201) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          86.5% |          86.5% | +0.0% |
  |   Files             |            113 |            113 |     0 |
  |   Lines             |          10678 |          10690 |   +12 |
+ |   Covered           |           9242 |           9255 |   +13 |
+ | Code to Test Ratio  |          1:2.2 |          1:2.3 |  +0.0 |
  |   Code              |          23860 |          23897 |   +37 |
+ |   Test              |          54538 |          54985 |  +447 |
+ | Test Execution Time |          1m42s |          1m24s |  -18s |

Code coverage of files in pull request scope (90.0% → 90.2%, patch 83.3%)

Files Coverage +/- Patch Coverage Status
internal/mycli/client_side_statement_def.go 91.1% +0.1% 100.0% modified
internal/mycli/feature.go 95.3% 0.0% - modified
internal/mycli/statements_system_variable.go 92.2% -0.4% 75.0% modified
internal/mycli/var_defs.go 85.8% 0.0% - modified
internal/mycli/var_handler.go 88.4% +1.2% - affected
internal/mycli/var_registry.go 95.3% +0.9% - affected
internal/mycli/var_reset.go 86.2% 0.0% - modified

Reported by octocov

@apstndb

apstndb commented Sep 14, 2026

Copy link
Copy Markdown
Owner Author

Independent coordinator confirmation on exact HEAD 7afe8fcd272c21732307af8fb11d39d541922d80 (base 83ae3abccc3105687d34931727e1db694bb34950, #976).

SQL RESET <name> uses prepareReset + commitPersistentReset. Worker reports no RESET LOCAL / SET x=DEFAULT; TRANSACTION_TAG writable-slot semantics reused; local make check exit 0 and ./internal/mycli coverage 82.4%. Keepalive session vars were not on this main, so those cases were skipped. 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":978,"head":"7afe8fcd272c21732307af8fb11d39d541922d80","base":"83ae3abccc3105687d34931727e1db694bb34950","validation":["hosted test/lint/race/readme-sync/govulncheck/cross-compile success on exact HEAD","worker local make check exit 0","worker ./internal/mycli coverage 82.4%","MERGEABLE CLEAN","no unresolved review threads"],"deviations":["KEEP_TRANSACTION_ALIVE RESET cases skipped; that var is not on this main pending #977"],"blockers":[]}

@apstndb

apstndb commented Sep 14, 2026

Copy link
Copy Markdown
Owner Author

Withdrawing the ready JSON for 7afe8fcd272c21732307af8fb11d39d541922d80. Main moved to 6b9600acf40d1eb8b18eeab845b3141dc68a9305 (#977). This PR will be additively merged with origin/main and KEEP_TRANSACTION_ALIVE RESET cases added. Do not squash-merge until a new HEAD is independently CI-green.

cursoragent and others added 2 commits September 14, 2026 13:39
Additive merge of #977 keepalive squash (6b9600a). No rebase.

Co-authored-by: apstndb <apstndb@users.noreply.github.com>
Add statement and registry RESET cases for the keepalive session
variable now on main, including --set startup restore and isolation
from unrelated LOCAL undo.

Co-authored-by: apstndb <apstndb@users.noreply.github.com>
@cursor

cursor Bot commented Sep 14, 2026

Copy link
Copy Markdown

Additive merge of origin/main (6b9600acf40d1eb8b18eeab845b3141dc68a9305, #977 keepalive) is in ba447e00cad5063cb41217356eb0859a4e399f9d. KEEP_TRANSACTION_ALIVE RESET cases landed in 77fb7a3f14c8b145f8680939eaa62659b79aa6bf.

Current HEAD: 77fb7a3f14c8b145f8680939eaa62659b79aa6bf

prepareReset / commitPersistentReset and TRANSACTION_TAG writable-slot semantics are unchanged. make check exit 0 (.tmp/driver-parity-960/make-check-after-977-merge.log). Not squash-merged.

@apstndb

apstndb commented Sep 14, 2026

Copy link
Copy Markdown
Owner Author

Independent coordinator confirmation on exact HEAD 77fb7a3f14c8b145f8680939eaa62659b79aa6bf (base 6b9600acf40d1eb8b18eeab845b3141dc68a9305, #977).

Additive merge of origin/main plus KEEP_TRANSACTION_ALIVE RESET cases. Worker reports prepareReset/commitPersistentReset and TRANSACTION_TAG writable-slot semantics unchanged; local make check 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. Prior ready JSON on 7afe8fc stays withdrawn.

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

@apstndb

apstndb commented Sep 14, 2026

Copy link
Copy Markdown
Owner Author

Independent review of exact HEAD 77fb7a3f14c8b145f8680939eaa62659b79aa6bf: no actionable findings remain.

The statement reuses startup capture, reset preparation/commit and targeted LOCAL undo retirement. I reviewed canonical/alias handling, exclusions and guards, unchanged-value behavior, detached execution, help/completion, and TRANSACTION_TAG's writable-slot semantics.

Independent focused tests and full Docker make check passed at feature HEAD 7afe8fcd272c21732307af8fb11d39d541922d80. The main integration tree exactly matches Git's automatic merge of that feature and reviewed main 6b9600a. The subsequent change only adds keepalive RESET tests, which I reviewed and ran with the reset/startup tests on current HEAD; they passed. The worker also reports full make check passing on current HEAD. All applicable CI passed, and no unresolved review threads or review-level findings remain.

@apstndb
apstndb marked this pull request as ready for review September 14, 2026 13:50
@apstndb
apstndb merged commit bc7d76c into main Sep 14, 2026
10 checks passed
@apstndb
apstndb deleted the cursor/reset-single-960-c42b branch September 14, 2026 13:51
cursor Bot pushed a commit that referenced this pull request Sep 14, 2026
Co-authored-by: apstndb <apstndb@users.noreply.github.com>
cursor Bot pushed a commit that referenced this pull request Sep 14, 2026
#978 added RESET <name>. Drive the automatic DML policy freeze
scenario through that statement instead of the registry API.

Co-authored-by: apstndb <apstndb@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support RESET of a single session variable using startup values

2 participants