Copilot commented on code in PR #4539:
URL: https://github.com/apache/arrow-adbc/pull/4539#discussion_r3618722798
##########
java/driver/flight-sql/src/main/java/org/apache/arrow/adbc/driver/flightsql/FlightSqlDriver.java:
##########
@@ -67,4 +77,129 @@ public AdbcDatabase open(Map<String, Object> parameters)
throws AdbcException {
}
return new FlightSqlDatabase(allocator, location, (SqlQuirks) quirks,
parameters);
}
+
+ /**
+ * Parses the "uri" option into a {@link Location}, translating a {@code
flightsql://} scheme (and
+ * its {@code transport} query parameter) into the legacy {@code grpc*://}
scheme that {@link
+ * Location} understands natively. Any other scheme is passed through
unchanged.
+ */
+ static Location parseLocation(String uri) throws AdbcException {
+ final URI parsed;
+ try {
+ parsed = new URI(uri);
+ } catch (URISyntaxException e) {
+ throw AdbcException.invalidArgument(
+ String.format("[Flight SQL] Location %s is invalid: %s", uri, e))
+ .withCause(e);
+ }
+
+ if (!FLIGHTSQL_SCHEME.equals(parsed.getScheme())) {
+ return new Location(parsed);
+ }
Review Comment:
`URI` schemes are case-insensitive, but this comparison is case-sensitive.
`FLIGHTSQL://...` would not be recognized and would be passed through to
`Location`, likely failing. Consider matching the scheme case-insensitively to
mirror Go/url.Parse behavior.
##########
java/driver/flight-sql/src/main/java/org/apache/arrow/adbc/driver/flightsql/FlightSqlDatabase.java:
##########
@@ -80,6 +81,10 @@ public void close() throws AdbcException {}
@Override
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 + '\'' + '}';
Review Comment:
`toString()` now includes the configured URI verbatim. If callers ever
include userinfo (e.g. `grpc+tls://user:pass@host:port`), this will leak
credentials into logs/debug output. Consider redacting any userinfo before
returning the string.
##########
java/driver/flight-sql/src/test/java/org/apache/arrow/adbc/driver/flightsql/FlightSqlDriverUriTest.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.arrow.adbc.driver.flightsql;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.arrow.adbc.core.AdbcConnection;
+import org.apache.arrow.adbc.core.AdbcDatabase;
+import org.apache.arrow.adbc.core.AdbcDriver;
+import org.apache.arrow.adbc.core.AdbcException;
+import org.apache.arrow.adbc.core.AdbcStatusCode;
+import org.apache.arrow.flight.FlightServer;
+import org.apache.arrow.flight.Location;
+import org.apache.arrow.flight.sql.NoOpFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/** Tests for the {@code flightsql://} URI scheme handled by {@link
FlightSqlDriver}. */
+class FlightSqlDriverUriTest {
+ static BufferAllocator allocator;
+ static FlightServer server;
+ static FlightSqlDriver driver;
+
+ @BeforeAll
+ static void beforeAll() throws Exception {
+ allocator = new RootAllocator();
+ server =
+ FlightServer.builder()
+ .allocator(allocator)
+ .producer(new NoOpFlightSqlProducer())
+ .location(Location.forGrpcInsecure("localhost", 0))
+ .build();
+ server.start();
+ driver = new FlightSqlDriver(allocator);
+ }
+
+ @AfterAll
+ static void afterAll() throws Exception {
+ AutoCloseables.close(server, allocator);
+ }
+
+ @Test
+ void defaultTransportIsTls() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql://localhost:1234");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tls");
+ assertThat(location.getUri().getHost()).isEqualTo("localhost");
+ assertThat(location.getUri().getPort()).isEqualTo(1234);
+ }
+
+ @Test
+ void explicitTlsTransport() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql://localhost:1234?transport=tls");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tls");
+ }
+
+ @Test
+ void tcpTransportIsCaseInsensitive() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql://localhost:1234?transport=TCP");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tcp");
+ assertThat(location.getUri().getPort()).isEqualTo(1234);
+ }
+
+ @Test
+ void unixTransport() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql:///tmp/adbc.sock?transport=unix");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+unix");
+ assertThat(location.getUri().getPath()).isEqualTo("/tmp/adbc.sock");
+ }
+
+ @Test
+ void legacySchemesStillWork() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("grpc+tls://localhost:1234");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tls");
+ }
+
+ @Test
+ void connectsOverTcpTransport() throws Exception {
+ Map<String, Object> parameters = new HashMap<>();
+ AdbcDriver.PARAM_URI.set(
+ parameters, "flightsql://localhost:" + server.getPort() +
"?transport=tcp");
+
+ try (AdbcDatabase database = driver.open(parameters);
+ AdbcConnection connection = database.connect()) {
+ assertThat(connection).isNotNull();
+ }
+ }
+
+ @Test
+ void rejectsUnrecognizedTransport() {
+ AdbcException ex =
+ assertThrows(
+ AdbcException.class,
+ () ->
FlightSqlDriver.parseLocation("flightsql://localhost:1234?transport=bogus"));
+ assertThat(ex.getStatus()).isEqualTo(AdbcStatusCode.INVALID_ARGUMENT);
+ }
+
+ @Test
+ void rejectsHostWithUnixTransport() {
+ assertThrows(
+ AdbcException.class,
+ () ->
FlightSqlDriver.parseLocation("flightsql://localhost:1234/socket?transport=unix"));
+ }
Review Comment:
This test only asserts that an `AdbcException` is thrown, but the PR
description claims invalid combinations are rejected with `INVALID_ARGUMENT`.
Asserting the status code here would make the test enforce that contract.
##########
java/driver/flight-sql/src/test/java/org/apache/arrow/adbc/driver/flightsql/FlightSqlDriverUriTest.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.arrow.adbc.driver.flightsql;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.arrow.adbc.core.AdbcConnection;
+import org.apache.arrow.adbc.core.AdbcDatabase;
+import org.apache.arrow.adbc.core.AdbcDriver;
+import org.apache.arrow.adbc.core.AdbcException;
+import org.apache.arrow.adbc.core.AdbcStatusCode;
+import org.apache.arrow.flight.FlightServer;
+import org.apache.arrow.flight.Location;
+import org.apache.arrow.flight.sql.NoOpFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/** Tests for the {@code flightsql://} URI scheme handled by {@link
FlightSqlDriver}. */
+class FlightSqlDriverUriTest {
+ static BufferAllocator allocator;
+ static FlightServer server;
+ static FlightSqlDriver driver;
+
+ @BeforeAll
+ static void beforeAll() throws Exception {
+ allocator = new RootAllocator();
+ server =
+ FlightServer.builder()
+ .allocator(allocator)
+ .producer(new NoOpFlightSqlProducer())
+ .location(Location.forGrpcInsecure("localhost", 0))
+ .build();
+ server.start();
+ driver = new FlightSqlDriver(allocator);
+ }
+
+ @AfterAll
+ static void afterAll() throws Exception {
+ AutoCloseables.close(server, allocator);
+ }
+
+ @Test
+ void defaultTransportIsTls() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql://localhost:1234");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tls");
+ assertThat(location.getUri().getHost()).isEqualTo("localhost");
+ assertThat(location.getUri().getPort()).isEqualTo(1234);
+ }
+
+ @Test
+ void explicitTlsTransport() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql://localhost:1234?transport=tls");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tls");
+ }
+
+ @Test
+ void tcpTransportIsCaseInsensitive() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql://localhost:1234?transport=TCP");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tcp");
+ assertThat(location.getUri().getPort()).isEqualTo(1234);
+ }
+
+ @Test
+ void unixTransport() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql:///tmp/adbc.sock?transport=unix");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+unix");
+ assertThat(location.getUri().getPath()).isEqualTo("/tmp/adbc.sock");
+ }
+
+ @Test
+ void legacySchemesStillWork() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("grpc+tls://localhost:1234");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tls");
+ }
+
+ @Test
+ void connectsOverTcpTransport() throws Exception {
+ Map<String, Object> parameters = new HashMap<>();
+ AdbcDriver.PARAM_URI.set(
+ parameters, "flightsql://localhost:" + server.getPort() +
"?transport=tcp");
+
+ try (AdbcDatabase database = driver.open(parameters);
+ AdbcConnection connection = database.connect()) {
+ assertThat(connection).isNotNull();
+ }
+ }
+
+ @Test
+ void rejectsUnrecognizedTransport() {
+ AdbcException ex =
+ assertThrows(
+ AdbcException.class,
+ () ->
FlightSqlDriver.parseLocation("flightsql://localhost:1234?transport=bogus"));
+ assertThat(ex.getStatus()).isEqualTo(AdbcStatusCode.INVALID_ARGUMENT);
+ }
+
+ @Test
+ void rejectsHostWithUnixTransport() {
+ assertThrows(
+ AdbcException.class,
+ () ->
FlightSqlDriver.parseLocation("flightsql://localhost:1234/socket?transport=unix"));
+ }
+
+ @Test
+ void rejectsPathWithTcpTransport() {
+ assertThrows(
+ AdbcException.class,
+ () ->
FlightSqlDriver.parseLocation("flightsql://localhost:1234/socket?transport=tcp"));
+ }
Review Comment:
This test only asserts that an `AdbcException` is thrown, but the PR
description claims invalid combinations are rejected with `INVALID_ARGUMENT`.
Asserting the status code here would make the test enforce that contract.
##########
java/driver/flight-sql/src/test/java/org/apache/arrow/adbc/driver/flightsql/FlightSqlDriverUriTest.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.arrow.adbc.driver.flightsql;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.arrow.adbc.core.AdbcConnection;
+import org.apache.arrow.adbc.core.AdbcDatabase;
+import org.apache.arrow.adbc.core.AdbcDriver;
+import org.apache.arrow.adbc.core.AdbcException;
+import org.apache.arrow.adbc.core.AdbcStatusCode;
+import org.apache.arrow.flight.FlightServer;
+import org.apache.arrow.flight.Location;
+import org.apache.arrow.flight.sql.NoOpFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/** Tests for the {@code flightsql://} URI scheme handled by {@link
FlightSqlDriver}. */
+class FlightSqlDriverUriTest {
+ static BufferAllocator allocator;
+ static FlightServer server;
+ static FlightSqlDriver driver;
+
+ @BeforeAll
+ static void beforeAll() throws Exception {
+ allocator = new RootAllocator();
+ server =
+ FlightServer.builder()
+ .allocator(allocator)
+ .producer(new NoOpFlightSqlProducer())
+ .location(Location.forGrpcInsecure("localhost", 0))
+ .build();
+ server.start();
+ driver = new FlightSqlDriver(allocator);
+ }
+
+ @AfterAll
+ static void afterAll() throws Exception {
+ AutoCloseables.close(server, allocator);
+ }
+
+ @Test
+ void defaultTransportIsTls() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql://localhost:1234");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tls");
+ assertThat(location.getUri().getHost()).isEqualTo("localhost");
+ assertThat(location.getUri().getPort()).isEqualTo(1234);
+ }
+
+ @Test
+ void explicitTlsTransport() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql://localhost:1234?transport=tls");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tls");
+ }
+
+ @Test
+ void tcpTransportIsCaseInsensitive() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql://localhost:1234?transport=TCP");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tcp");
+ assertThat(location.getUri().getPort()).isEqualTo(1234);
+ }
+
+ @Test
+ void unixTransport() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql:///tmp/adbc.sock?transport=unix");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+unix");
+ assertThat(location.getUri().getPath()).isEqualTo("/tmp/adbc.sock");
+ }
+
+ @Test
+ void legacySchemesStillWork() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("grpc+tls://localhost:1234");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tls");
+ }
+
+ @Test
+ void connectsOverTcpTransport() throws Exception {
+ Map<String, Object> parameters = new HashMap<>();
+ AdbcDriver.PARAM_URI.set(
+ parameters, "flightsql://localhost:" + server.getPort() +
"?transport=tcp");
+
+ try (AdbcDatabase database = driver.open(parameters);
+ AdbcConnection connection = database.connect()) {
+ assertThat(connection).isNotNull();
+ }
+ }
+
+ @Test
+ void rejectsUnrecognizedTransport() {
+ AdbcException ex =
+ assertThrows(
+ AdbcException.class,
+ () ->
FlightSqlDriver.parseLocation("flightsql://localhost:1234?transport=bogus"));
+ assertThat(ex.getStatus()).isEqualTo(AdbcStatusCode.INVALID_ARGUMENT);
+ }
+
+ @Test
+ void rejectsHostWithUnixTransport() {
+ assertThrows(
+ AdbcException.class,
+ () ->
FlightSqlDriver.parseLocation("flightsql://localhost:1234/socket?transport=unix"));
+ }
+
+ @Test
+ void rejectsPathWithTcpTransport() {
+ assertThrows(
+ AdbcException.class,
+ () ->
FlightSqlDriver.parseLocation("flightsql://localhost:1234/socket?transport=tcp"));
+ }
+
+ @Test
+ void rejectsPathWithDefaultTransport() {
+ assertThrows(
+ AdbcException.class,
+ () ->
FlightSqlDriver.parseLocation("flightsql://localhost:1234/socket"));
+ }
+
+ @Test
+ void rejectsUnixTransportWithoutPath() {
+ assertThrows(
+ AdbcException.class, () ->
FlightSqlDriver.parseLocation("flightsql://?transport=unix"));
+ }
Review Comment:
This test only asserts that an `AdbcException` is thrown, but the PR
description claims invalid combinations are rejected with `INVALID_ARGUMENT`.
Asserting the status code here would make the test enforce that contract.
##########
java/driver/flight-sql/src/test/java/org/apache/arrow/adbc/driver/flightsql/FlightSqlDriverUriTest.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.arrow.adbc.driver.flightsql;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.arrow.adbc.core.AdbcConnection;
+import org.apache.arrow.adbc.core.AdbcDatabase;
+import org.apache.arrow.adbc.core.AdbcDriver;
+import org.apache.arrow.adbc.core.AdbcException;
+import org.apache.arrow.adbc.core.AdbcStatusCode;
+import org.apache.arrow.flight.FlightServer;
+import org.apache.arrow.flight.Location;
+import org.apache.arrow.flight.sql.NoOpFlightSqlProducer;
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.util.AutoCloseables;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/** Tests for the {@code flightsql://} URI scheme handled by {@link
FlightSqlDriver}. */
+class FlightSqlDriverUriTest {
+ static BufferAllocator allocator;
+ static FlightServer server;
+ static FlightSqlDriver driver;
+
+ @BeforeAll
+ static void beforeAll() throws Exception {
+ allocator = new RootAllocator();
+ server =
+ FlightServer.builder()
+ .allocator(allocator)
+ .producer(new NoOpFlightSqlProducer())
+ .location(Location.forGrpcInsecure("localhost", 0))
+ .build();
+ server.start();
+ driver = new FlightSqlDriver(allocator);
+ }
+
+ @AfterAll
+ static void afterAll() throws Exception {
+ AutoCloseables.close(server, allocator);
+ }
+
+ @Test
+ void defaultTransportIsTls() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql://localhost:1234");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tls");
+ assertThat(location.getUri().getHost()).isEqualTo("localhost");
+ assertThat(location.getUri().getPort()).isEqualTo(1234);
+ }
+
+ @Test
+ void explicitTlsTransport() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql://localhost:1234?transport=tls");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tls");
+ }
+
+ @Test
+ void tcpTransportIsCaseInsensitive() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql://localhost:1234?transport=TCP");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tcp");
+ assertThat(location.getUri().getPort()).isEqualTo(1234);
+ }
+
+ @Test
+ void unixTransport() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("flightsql:///tmp/adbc.sock?transport=unix");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+unix");
+ assertThat(location.getUri().getPath()).isEqualTo("/tmp/adbc.sock");
+ }
+
+ @Test
+ void legacySchemesStillWork() throws Exception {
+ Location location =
FlightSqlDriver.parseLocation("grpc+tls://localhost:1234");
+ assertThat(location.getUri().getScheme()).isEqualTo("grpc+tls");
+ }
+
+ @Test
+ void connectsOverTcpTransport() throws Exception {
+ Map<String, Object> parameters = new HashMap<>();
+ AdbcDriver.PARAM_URI.set(
+ parameters, "flightsql://localhost:" + server.getPort() +
"?transport=tcp");
+
+ try (AdbcDatabase database = driver.open(parameters);
+ AdbcConnection connection = database.connect()) {
+ assertThat(connection).isNotNull();
+ }
+ }
+
+ @Test
+ void rejectsUnrecognizedTransport() {
+ AdbcException ex =
+ assertThrows(
+ AdbcException.class,
+ () ->
FlightSqlDriver.parseLocation("flightsql://localhost:1234?transport=bogus"));
+ assertThat(ex.getStatus()).isEqualTo(AdbcStatusCode.INVALID_ARGUMENT);
+ }
+
+ @Test
+ void rejectsHostWithUnixTransport() {
+ assertThrows(
+ AdbcException.class,
+ () ->
FlightSqlDriver.parseLocation("flightsql://localhost:1234/socket?transport=unix"));
+ }
+
+ @Test
+ void rejectsPathWithTcpTransport() {
+ assertThrows(
+ AdbcException.class,
+ () ->
FlightSqlDriver.parseLocation("flightsql://localhost:1234/socket?transport=tcp"));
+ }
+
+ @Test
+ void rejectsPathWithDefaultTransport() {
+ assertThrows(
+ AdbcException.class,
+ () ->
FlightSqlDriver.parseLocation("flightsql://localhost:1234/socket"));
+ }
Review Comment:
This test only asserts that an `AdbcException` is thrown, but the PR
description claims invalid combinations are rejected with `INVALID_ARGUMENT`.
Asserting the status code here would make the test enforce that contract.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
