Superseded by #1640 - #1639
Conversation
Signed-off-by: Aakash Saravanan <aakash.saravanan@databricks.com>
![peco-review-bot[bot]](https://avatars.githubusercontent.com/in/3801177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 1 High
One high concern: SEA createSession now unconditionally requests execution_mode=FAST and hard-fails the connection when the response omits session_version.version_id, with no capability gating or feature flag — a backward-compat risk against endpoints that haven't rolled out Fast Path (and the PR notes live validation is still pending). The session-version tracking logic itself (max-version accumulation, session-identity guarding, detached-statement exclusion, state clearing on close) is sound and well-tested.
| throw new DatabricksSQLException( | ||
| "FAST session response did not include session_id", | ||
| DatabricksDriverErrorCode.CONNECTION_ERROR); | ||
| } |
There was a problem hiding this comment.
🟠 High — createSession now sets execution_mode=FAST on every SEA session unconditionally (line ~118) and then hard-fails the connection whenever the response does not include session_version.version_id — throwing DatabricksSQLException and best-effort deleting the just-created session. There is no connection-context flag or server-capability gate: the driver now requires server-side FAST support for all SEA connections.
This is a backward-compatibility regression risk. Any SEA endpoint that has not rolled out Fast Path (or returns CreateSessionResponse without session_version) will now fail createSession outright, where previously it connected fine with only a session_id. The PR description explicitly notes "Live endpoint validation remains pending availability of a Fast Path-capable SEA endpoint," so this contract has not been exercised against a real server.
Consider gating FAST/session-version behind a connection parameter (defaulting off until the server contract is confirmed), or treating a missing session_version as "FAST not supported" and falling back to normal session behavior rather than tearing down the connection. Note the wide blast radius: every SEA integration-test session mapping in this PR had to be edited to add session_version for tests to keep passing, which mirrors how existing production endpoints would behave if they don't yet return it.
aakash-saravanan-db
changed the title
1 participant
Superseded by #1640.