feat(java/driver/flight-sql): support flightsql:// URI scheme#4539
Open
unikdahal wants to merge 1 commit into
Open
feat(java/driver/flight-sql): support flightsql:// URI scheme#4539unikdahal wants to merge 1 commit into
unikdahal wants to merge 1 commit into
Conversation
Mirrors Go's flightsql:// scheme (transport=tls/tcp/unix query param, default TLS, legacy grpc*:// schemes still accepted). Translates to Location's grpc+tls/grpc+tcp/grpc+unix schemes with explicit validation of host/path combinations per transport.
There was a problem hiding this comment.
Pull request overview
Adds flightsql:// URI support to the Java Flight SQL ADBC driver (aligning with the Go driver), including transport selection via a transport query parameter and tighter validation/error surfacing. This updates URI parsing to translate flightsql:// into the legacy grpc+{tls,tcp,unix}:// forms that Arrow Flight Location understands, and expands test coverage for the new scheme.
Changes:
- Add
FlightSqlDriver.parseLocationto translate/validateflightsql://...URIs (default TLS;transport=tcp|tls|unixcase-insensitive; reject invalid combinations, userinfo, fragments, and malformed percent-encoding intransport). - Add a new
FlightSqlDriverUriTestsuite covering translations, rejections, and a real TCP connection against an in-process server. - Update
TlsTestto coverflightsql://default TLS and explicit?transport=tls, and updateFlightSqlDatabase#toStringto include configured URI.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| java/driver/flight-sql/src/main/java/org/apache/arrow/adbc/driver/flightsql/FlightSqlDriver.java | Implements flightsql:// parsing/translation and validation into Flight Location. |
| java/driver/flight-sql/src/main/java/org/apache/arrow/adbc/driver/flightsql/FlightSqlDatabase.java | Enhances toString() to report configured URI alongside resolved target. |
| java/driver/flight-sql/src/test/java/org/apache/arrow/adbc/driver/flightsql/FlightSqlDriverUriTest.java | Adds unit/integration tests for new scheme behavior and validation. |
| java/driver/flight-sql/src/test/java/org/apache/arrow/adbc/driver/flightsql/TlsTest.java | Adds TLS coverage for flightsql:// default and explicit TLS transport. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+96
to
+98
| if (!FLIGHTSQL_SCHEME.equals(parsed.getScheme())) { | ||
| return new Location(parsed); | ||
| } |
Comment on lines
83
to
+88
| public String toString() { | ||
| return "FlightSqlDatabase{" + "target='" + location + '\'' + '}'; | ||
| Object configuredUri = AdbcDriver.PARAM_URI.get(parameters); | ||
| if (configuredUri == null) { | ||
| configuredUri = parameters.get(AdbcDriver.PARAM_URL); | ||
| } | ||
| return "FlightSqlDatabase{" + "uri='" + configuredUri + "', target='" + location + '\'' + '}'; |
Comment on lines
+118
to
+123
| @Test | ||
| void rejectsHostWithUnixTransport() { | ||
| assertThrows( | ||
| AdbcException.class, | ||
| () -> FlightSqlDriver.parseLocation("flightsql://localhost:1234/socket?transport=unix")); | ||
| } |
Comment on lines
+125
to
+130
| @Test | ||
| void rejectsPathWithTcpTransport() { | ||
| assertThrows( | ||
| AdbcException.class, | ||
| () -> FlightSqlDriver.parseLocation("flightsql://localhost:1234/socket?transport=tcp")); | ||
| } |
Comment on lines
+132
to
+137
| @Test | ||
| void rejectsPathWithDefaultTransport() { | ||
| assertThrows( | ||
| AdbcException.class, | ||
| () -> FlightSqlDriver.parseLocation("flightsql://localhost:1234/socket")); | ||
| } |
Comment on lines
+139
to
+143
| @Test | ||
| void rejectsUnixTransportWithoutPath() { | ||
| assertThrows( | ||
| AdbcException.class, () -> FlightSqlDriver.parseLocation("flightsql://?transport=unix")); | ||
| } |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What's Changed
Adds support for
flightsql://URIs to the Java Flight SQL driver, mirroring the Go driver's implementation (#4488).flightsql://<host>:<port>— secure TLS by defaultflightsql://<host>:<port>?transport=tls— explicit TLSflightsql://<host>:<port>?transport=tcp— plaintext gRPCflightsql:///<path/to/socket>?transport=unix— Unix domain socketThe
transportvalue is matched case-insensitively; an unrecognized value is rejected withINVALID_ARGUMENTrather than silently falling back. Invalidcombinations (socket path with
tcp/tls, host withunix, missing host/path) are rejected, matching the Go driver's validation and error messages. Thelegacy
grpc://,grpc+tcp://,grpc+tls://, andgrpc+unix://schemes are still accepted unchanged.URIs with userinfo or a fragment are rejected explicitly, since
Locationwould otherwise drop them silently. Parse failures thatjava.net.URIreports onlyvia
Location'sIllegalArgumentException(e.g. hostnames that fail strict authority parsing) are caught and surfaced asINVALID_ARGUMENTAdbcExceptions.FlightSqlDatabase#toStringnow reports the originally configured URI alongside the resolved target.Testing
FlightSqlDriverUriTest: 18 unit tests covering translation of all transports, case-insensitivity, legacy scheme passthrough, a real connection over?transport=tcpagainst an in-process Flight server, and rejection cases (unrecognized transport, invalid host/path combinations, userinfo, fragment, malformedpercent-encoding, unparseable hosts).
TlsTestexercisingflightsql://default and explicit?transport=tlsagainst the TLS test server.Closes #4516. Part of #4453.