Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support connecting to Firebird via jdbcUrl containing the absolute path to fdb #34335

Merged
merged 1 commit into from
Jan 14, 2025
Merged

Support connecting to Firebird via jdbcUrl containing the absolute path to fdb #34335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE-NOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
1. Agent: Simplify the use of Agent's Docker Image - [#33356](https://github.com/apache/shardingsphere/pull/33356)
1. Mode: Support modifying Hikari-CP configurations via props in standalone mode [#34185](https://github.com/apache/shardingsphere/pull/34185)
1. Encrypt: Support insert statement rewrite use quote [#34259](https://github.com/apache/shardingsphere/pull/34259)
1. Infra: Support connecting to Firebird via jdbcUrl containing the absolute path to fdb - [#34335](https://github.com/apache/shardingsphere/pull/34335)

### Bug Fixes

Expand Down
7 changes: 7 additions & 0 deletions infra/database/type/firebird/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@
<version>${project.version}</version>
</dependency>

<dependency>
<groupId>org.firebirdsql.jdbc</groupId>
<artifactId>jaybird</artifactId>
<scope>provided</scope>
<optional>true</optional>
</dependency>

<dependency>
<groupId>org.apache.shardingsphere</groupId>
<artifactId>shardingsphere-test-util</artifactId>
Expand Down
21 changes: 15 additions & 6 deletions .../shardingsphere/infra/database/firebird/connector/FirebirdConnectionPropertiesParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,14 @@

package org.apache.shardingsphere.infra.database.firebird.connector;

import lombok.SneakyThrows;
import org.apache.shardingsphere.infra.database.core.connector.ConnectionProperties;
import org.apache.shardingsphere.infra.database.core.connector.ConnectionPropertiesParser;
import org.apache.shardingsphere.infra.database.core.connector.StandardConnectionProperties;
import org.apache.shardingsphere.infra.database.core.connector.url.JdbcUrl;
import org.apache.shardingsphere.infra.database.core.connector.url.StandardJdbcUrlParser;
import org.firebirdsql.gds.impl.DbAttachInfo;
import org.firebirdsql.gds.impl.GDSFactory;
import org.firebirdsql.gds.impl.GDSType;
import org.firebirdsql.jdbc.FBDriver;

import java.util.Properties;

Expand All @@ -30,12 +33,18 @@
*/
public final class FirebirdConnectionPropertiesParser implements ConnectionPropertiesParser {

private static final int DEFAULT_PORT = 3050;

@SneakyThrows(Exception.class)
@Override
public ConnectionProperties parse(final String url, final String username, final String catalog) {
JdbcUrl jdbcUrl = new StandardJdbcUrlParser().parse(url);
return new StandardConnectionProperties(jdbcUrl.getHostname(), jdbcUrl.getPort(DEFAULT_PORT), jdbcUrl.getDatabase(), null, jdbcUrl.getQueryProperties(), new Properties());
GDSType type = GDSFactory.getTypeForProtocol(url);
String databaseURL = GDSFactory.getDatabasePath(type, url);
DbAttachInfo dbAttachInfo = DbAttachInfo.parseConnectString(databaseURL);
String attachObjectName = dbAttachInfo.getAttachObjectName();
String databaseName = attachObjectName.contains("?") ? attachObjectName.split("\\?")[0] : attachObjectName;
Properties queryProperties = new Properties();
queryProperties.putAll(FBDriver.normalizeProperties(url, new Properties()));
return new StandardConnectionProperties(dbAttachInfo.getServerName(), dbAttachInfo.getPortNumber(),
databaseName, null, queryProperties, new Properties());
}

@Override
Expand Down
23 changes: 20 additions & 3 deletions ...rdingsphere/infra/database/firebird/connector/FirebirdConnectionPropertiesParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,24 @@

import org.apache.shardingsphere.infra.database.core.connector.ConnectionProperties;
import org.apache.shardingsphere.infra.database.core.connector.ConnectionPropertiesParser;
import org.apache.shardingsphere.infra.database.core.exception.UnrecognizedDatabaseURLException;
import org.apache.shardingsphere.infra.database.core.spi.DatabaseTypedSPILoader;
import org.apache.shardingsphere.infra.database.core.type.DatabaseType;
import org.apache.shardingsphere.infra.spi.type.typed.TypedSPILoader;
import org.apache.shardingsphere.test.util.PropertiesBuilder;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.ArgumentsProvider;
import org.junit.jupiter.params.provider.ArgumentsSource;

import java.sql.SQLNonTransientConnectionException;
import java.util.Properties;
import java.util.stream.Stream;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertThrows;

class FirebirdConnectionPropertiesParserTest {
Expand All @@ -54,14 +56,29 @@ void assertNewConstructor(final String name, final String url, final String host

@Test
void assertNewConstructorFailure() {
assertThrows(UnrecognizedDatabaseURLException.class, () -> parser.parse("jdbc:firebirdsql:xxxxxxxx", null, null));
assertDoesNotThrow(() -> parser.parse("jdbc:firebirdsql:xxxxxxxx", null, null));
assertThrows(SQLNonTransientConnectionException.class, () -> parser.parse("jdbc:firebirdsql://localhost:c:/data/db/test.fdb", null, null));
Comment on lines +59 to +60
Copy link
Member Author

@linghengqian linghengqian Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@TedCraft TedCraft Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the manual (https://github.com/FirebirdSQL/jaybird-manual/blob/master/src/docs/asciidoc/chapters/connection/connection.adoc#pure_java-type) the jdbcUrl format jdbc:firebirdsql:xxxxxxxxxxxx kinda makes sense, since it uses the legacy url format used in firebird. In this case, we just specify the database alias (or path) and the host and port are used by default (localhost:3050). But even though this url type is supported, according to the same manual, it is recommended to use the new url format.

Copy link
Member Author

@linghengqian linghengqian Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • My understanding is that I should respect the behavior of the Firebird JDBC Driver's classes.

  • There seems to be only the LGPL LICENSE for the Firebird JDBC Driver, so I would assume that everyone who uses both shardingsphere and the Firebird JDBC Driver would be affected by the LGPL. Maven's optional property is enough to comply with the ASF's requirements when distributing. I think there is no problem with PR at the moment.

}

private static class NewConstructorTestCaseArgumentsProvider implements ArgumentsProvider {

@Override
public Stream<? extends Arguments> provideArguments(final ExtensionContext extensionContext) {
return Stream.of(Arguments.of("simple", "jdbc:firebirdsql://127.0.0.1/foo_ds", "127.0.0.1", 3050, "foo_ds", null, new Properties()));
return Stream.of(
Arguments.of("simple_first", "jdbc:firebirdsql://127.0.0.1/foo_ds", "127.0.0.1", 3050, "foo_ds", null, new Properties()),
Arguments.of("simple_second", "jdbc:firebird://localhost:32783//var/lib/firebird/data/demo_ds_2.fdb",
"localhost", 32783, "/var/lib/firebird/data/demo_ds_2.fdb", null, new Properties()),
Arguments.of("simple_third", "jdbc:firebirdsql://localhost/database?socket_buffer_size=32767", "localhost", 3050, "database", null, PropertiesBuilder.build(
new PropertiesBuilder.Property("socketBufferSize", "32767"))),
Arguments.of("complex",
"jdbc:firebirdsql://localhost/database?socket_buffer_size=32767"
+ "&TRANSACTION_REPEATABLE_READ=concurrency,write,no_wait&columnLabelForName&soTimeout=1000&nonStandard2=value2",
"localhost", 3050, "database", null, PropertiesBuilder.build(
new PropertiesBuilder.Property("socketBufferSize", "32767"),
new PropertiesBuilder.Property("TRANSACTION_REPEATABLE_READ", "concurrency,write,no_wait"),
new PropertiesBuilder.Property("columnLabelForName", ""),
new PropertiesBuilder.Property("soTimeout", "1000"),
new PropertiesBuilder.Property("nonStandard2", "value2"))));
}
}
}
7 changes: 7 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@
<hive-server2-jdbc-driver-thin.version>1.6.0</hive-server2-jdbc-driver-thin.version>
<hadoop.version>3.3.6</hadoop.version>
<presto.version>0.288.1</presto.version>
<jaybird.version>5.0.6.java8</jaybird.version>

<hikari-cp.version>4.0.3</hikari-cp.version>

Expand Down Expand Up @@ -489,6 +490,12 @@
<version>${presto.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.firebirdsql.jdbc</groupId>
<artifactId>jaybird</artifactId>
<version>${jaybird.version}</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>com.zaxxer</groupId>
Expand Down
Loading