You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Medium Risk
Changes query-setting merge and session suppression used by JDBC KILL QUERY cancel; wrong null handling could leak session_id on some requests or break timezone overrides.
Overview Session clearing now writes session_id, session_check, and session_timeout as explicit null entries in request settings (via new Session.clearSession(Map)), instead of only removing keys with resetOption. That lets per-request settings override and drop session values inherited from the client or JDBC connection when building the HTTP request—session_timezone is still left unchanged.
client-v2 adds InsertSettings#resetOption, documents that null settings are omitted on the wire, and adds integration coverage for overriding client session_timezone with setOption(..., null).
jdbc-v2 extends session-cancel regression tests so connections get a session from Session.applyTo(Properties) or custom_http_params, aligning with the fix for Statement.cancel() / KILL QUERY no longer running inside the locked session (#2881).
Reviewed by Cursor Bugbot for commit 22f15ad. Bugbot is set up for automated code reviews on this repo. Configure here.
Repository collaborators can run the JMH benchmark suite against this PR by commenting:
/benchmark
Optional regression threshold override (Δ% on Time or Alloc/op; defaults to 10%):
/benchmark threshold=15
Only one benchmark run per PR is active at a time — issuing a new /benchmark comment cancels the previous run. After the run finishes a separate comment will be posted comparing it against the latest scheduled run on main; the PR check fails if any benchmark regresses by more than the threshold.
Summary
This PR is a follow-up to #2868, extending the SESSION_IS_LOCKED cancel fix (issue #2690) to also cover sessions passed via custom_http_params (issue #2881). The root problem: CommonSettings.clearSession() previously used resetOption() which only removes session keys from the request settings map; any client-level session_id would be re-merged on top, so the KILL QUERY still carried the session id and hit the lock. The fix moves clearSession into Session as a static method that explicitly sets session_id, session_check, and session_timeout to null—null values are now suppressed on the wire, blocking the inherited defaults. Additionally, session_id/session_check/session_timeout are now accepted as first-class JDBC connection properties via Session.applyTo(Properties), so users no longer need custom_http_params for sessions. New resetOption() methods are added to InsertSettings, QuerySettings, and CommonSettings, and integration tests are updated to exercise both the connection-property and custom_http_params session-id paths.
What this impacts
client-v2 — Session.clearSession(Map) is new public API; Session.applyTo() signature widened to Map<? super String, Object> (backward-compatible); null-value semantics introduced into settings merge path
client-v2 / jdbc-v2 — resetOption() added to InsertSettings, QuerySettings, CommonSettings (new public API surface)
JDBC-v2 cancel path — Statement.cancel() now reliably strips all three session keys regardless of how the session was configured
Concerns
Medium rule fired — behavioral change in client-v2 hot-path module: The semantics of clearSession() changed from "remove key" to "set key to null"; any code that inspects or iterates the settings map post-clear will now see explicit null entries rather than absent keys. This is intentional and correct, but reviewers should verify null handling is consistent everywhere in the settings-merge pipeline.
New public API (Session.clearSession(Map), resetOption() on three settings classes, null-suppression contract in docs/features.md) introduced without a feature flag. Additive-only, but expands the compatibility surface documented in docs/features.md.
Required reviewer action
At least one human reviewer required (medium risk).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
clearSession()method to Session object.clearSession()now sets session settings to null to suppress them even if set in client.Closes #2881
Checklist
Delete items not relevant to your PR:
Note
Medium Risk
Changes query-setting merge and session suppression used by JDBC
KILL QUERYcancel; wrong null handling could leak session_id on some requests or break timezone overrides.Overview
Session clearing now writes
session_id,session_check, andsession_timeoutas explicitnullentries in request settings (via newSession.clearSession(Map)), instead of only removing keys withresetOption. That lets per-request settings override and drop session values inherited from the client or JDBC connection when building the HTTP request—session_timezoneis still left unchanged.client-v2 adds
InsertSettings#resetOption, documents thatnullsettings are omitted on the wire, and adds integration coverage for overriding clientsession_timezonewithsetOption(..., null).jdbc-v2 extends session-cancel regression tests so connections get a session from
Session.applyTo(Properties)orcustom_http_params, aligning with the fix forStatement.cancel()/KILL QUERYno longer running inside the locked session (#2881).Reviewed by Cursor Bugbot for commit 22f15ad. Bugbot is set up for automated code reviews on this repo. Configure here.