feat(java/driver/flight-sql): add session management by unikdahal · Pull Request #4747 · apache/arrow-adbc

unikdahal · 2026-09-01T22:00:20Z

Summary

Add Flight SQL session management to the Java ADBC driver.

This supersedes #4444, rebuilt from current main as a clean single commit.

Add typed session option get, set, and erase support
Add GetSessionOptions, SetSessionOptions, and CloseSession
Add JSON serialization and validation
Add unit and edge-case tests

Changes since the previous review

Session RPCs preserve connection-level call options through FlightSqlClientWithCallOptions. The explicit
callOptions argument was removed because the wrapper already applies them, avoiding duplicate options.
CloseSession is included in AutoCloseables.close as a selectively best-effort lambda.
Non-finite doubles are converted inside the JSON visitor and represented as strings.
Added edge-case coverage.

Closes #4443.
Closes #2821.
Relates to #745.

Kyperr · 2026-09-02T14:59:46Z

+          () -> {
+            try {
+              // Best-effort: the Go driver also ignores all errors closing the session.
+              client.closeSession(new CloseSessionRequest());


Some servers will throw an exception if this does not pass the callOptions. Looks like you went through the effort to implement those options, but they aren't used here.

client here is FlightSqlClientWithCallOptions, so closeSession combines the connection-level call options even when no additional per-call options are supplied. The configured call options are preserved here. There’s also a test covering this with testCloseSessionReceivesConnectionHeaderExactlyOnce.

Oh, that's right. I saw that in the original MR but forgot because that was a while ago. Thanks @unikdahal.

Kyperr · 2026-09-02T15:04:16Z

+          () -> {
+            try {
+              // Best-effort: the Go driver also ignores all errors closing the session.
+              client.closeSession(new CloseSessionRequest());


I think it would be worth making the private client gettable() from this class to allow a user to manage sessions while re-using the connection object. Thoughts?

That sounds reasonable to me.

Done. I added FlightSqlConnection#getClient() returning the FlightSqlClientWithCallOptions wrapper, so callers can manage sessions while retaining the connection-level call options. I also documented that the returned client is owned by the connection and shouldn’t be closed independently.

lidavidm

Thanks, this needs to be rebased.

lidavidm · 2026-09-10T04:44:44Z

+          () -> {
+            try {
+              // Best-effort: the Go driver also ignores all errors closing the session.
+              client.closeSession(new CloseSessionRequest());


That sounds reasonable to me.

lidavidm · 2026-09-10T04:48:00Z

+
+    if (k.equals(FlightSqlConnectionProperties.SESSION_OPTIONS)) {
+      if (key.getType() != String.class) {
+        return AdbcConnection.super.getOption(key);


(why can't we just write super?)

FlightSqlConnection implements AdbcConnection directly, and this is invoking the interface default method. Java requires the qualified AdbcConnection.super.getOption(...) form here; plain super.getOption(...) would look for a superclass implementation and won’t compile.

Copilot

🟡 Changes recommended

String-list parsing currently accepts trailing JSON tokens despite promising strict validation.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds Flight SQL session management to the Java ADBC driver, aligning it with existing Go and Python support.

Changes:

Adds typed session option retrieval, mutation, deletion, and JSON serialization.
Sends CloseSession during connection shutdown while preserving call options.
Adds integration and edge-case tests plus Jackson support.

File summaries

File	Description
`java/pom.xml`	Defines the Jackson version.
`java/driver/flight-sql/pom.xml`	Adds Jackson Databind.
`FlightSqlConnectionProperties.java`	Defines session option keys.
`FlightSqlClientWithCallOptions.java`	Wraps session RPCs with connection options.
`FlightSqlConnection.java`	Implements session options and lifecycle handling.
`FlightSqlSessionUtil.java`	Implements conversion, validation, and JSON handling.
`FlightSqlSessionTest.java`	Tests session RPC behavior and lifecycle.
`FlightSqlSessionEdgeCaseTest.java`	Tests validation and dispatch edge cases.

Review details

Files reviewed: 8/8 changed files
Comments generated: 1
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+  /** Parses a strict JSON string array (used when a string-list option is supplied as JSON). */
+  static String[] parseJsonArray(String json) throws AdbcException {
+    try {
+      final JsonNode root = MAPPER.readTree(json);


unikdahal · 2026-09-15T20:55:34Z

Looks like an unrelated CI failure. Could you rerun CI, @lidavidm?

unikdahal requested a review from lidavidm as a code owner September 1, 2026 22:00

unikdahal mentioned this pull request Sep 1, 2026

feat(java/driver/flight-sql): implement Flight SQL session management #4444

Closed

Kyperr reviewed Sep 2, 2026

View reviewed changes

lidavidm reviewed Sep 10, 2026

View reviewed changes

unikdahal force-pushed the feat/java-flight-sql-session-management branch 2 times, most recently from fcaf66f to 29df662 Compare September 14, 2026 18:50

unikdahal changed the title ~~feat(java/driver/flight-sql): add session management~~ feat(java/driver/flight-sql): add session management Sep 14, 2026

lidavidm requested a balanced review from Copilot September 15, 2026 00:56

Copilot started reviewing on behalf of lidavidm September 15, 2026 00:56 View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

feat(java/driver/flight-sql): add session management

07afeb1

unikdahal force-pushed the feat/java-flight-sql-session-management branch from d588801 to 07afeb1 Compare September 15, 2026 16:10

unikdahal requested a review from lidavidm September 17, 2026 17:01

Navigation Menu

feat(java/driver/flight-sql): add session management - #4747

feat(java/driver/flight-sql): add session management#4747
unikdahal wants to merge 1 commit into
apache:mainfrom
unikdahal:feat/java-flight-sql-session-management

unikdahal commented Sep 1, 2026 •

edited

Loading

Uh oh!

Kyperr Sep 2, 2026 •

edited

Loading

Uh oh!

unikdahal Sep 14, 2026

Uh oh!

Kyperr Sep 15, 2026

Uh oh!

Kyperr Sep 2, 2026

Uh oh!

lidavidm Sep 10, 2026

Uh oh!

unikdahal Sep 14, 2026

Uh oh!

lidavidm left a comment

Uh oh!

lidavidm Sep 10, 2026

Uh oh!

lidavidm Sep 10, 2026

Uh oh!

unikdahal Sep 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

unikdahal commented Sep 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

unikdahal commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes since the previous review

Uh oh!

Kyperr Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lidavidm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

unikdahal commented Sep 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

unikdahal commented Sep 1, 2026 •

edited

Loading

Kyperr Sep 2, 2026 •

edited

Loading