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.

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

[FLINK-37288] Add Google Cloud Spanner dialect and catalog #156

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

laughingman7743
Copy link

Copy link

boring-cyborg bot commented Feb 9, 2025

Thanks for opening this pull request! Please check out our contributing guidelines. (https://flink.apache.org/contributing/how-to-contribute.html)

@laughingman7743 laughingman7743 marked this pull request as ready for review February 9, 2025 06:45
@laughingman7743 laughingman7743 changed the title [FLINK-37288] Add flink-connector-jdbc-spnner [FLINK-37288] Add Google Cloud Spanner dialect and catalog Feb 9, 2025
Comment on lines -133 to -138
this.connectionProperties = Preconditions.checkNotNull(connectionProperties);
checkArgument(
!StringUtils.isNullOrWhitespaceOnly(connectionProperties.getProperty(USER_KEY)));
checkArgument(
!StringUtils.isNullOrWhitespaceOnly(
connectionProperties.getProperty(PASSWORD_KEY)));
Copy link
Author

Choose a reason for hiding this comment

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

Spanner does not use password authentication.

Comment on lines -579 to -588
/**
* URL has to be without database, like "jdbc:dialect://localhost:1234/" or
* "jdbc:dialect://localhost:1234" rather than "jdbc:dialect://localhost:1234/db".
*/
protected static void validateJdbcUrl(String url) {
String[] parts = url.trim().split("\\/+");

checkArgument(parts.length == 2);
}

Copy link
Author

Choose a reason for hiding this comment

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

In the case of Spanner, the URL is as follows, so I have deleted this validation.

jdbc:cloudspanner://hostname/projects/gcp_project_id/instances/instance_id/databases/database_id


/** Test for {@link AbstractJdbcCatalog}. */
class AbstractJdbcCatalogTest {
Copy link
Author

Choose a reason for hiding this comment

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

This class is no longer needed because URL validation has been removed.

@@ -120,7 +122,7 @@ public int[] getTableTypes() {
.toArray();
}

public Schema getTableSchema() {
public Schema getTableSchema(String pkConstraintName) {
Copy link
Author

Choose a reason for hiding this comment

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

In the case of Spanner, the contract name of the primary key is different, so it is possible to specify the contract name of the primary key.

@laughingman7743 laughingman7743 force-pushed the jdbc_spanner_connector branch 8 times, most recently from c7f0a5a to 3a73e0b Compare February 9, 2025 15:17
@laughingman7743
Copy link
Author

I have applied code formatting using Spotless and updated the documentation. It is ready for review.

@laughingman7743 laughingman7743 force-pushed the jdbc_spanner_connector branch 2 times, most recently from e4559c9 to 4ec422d Compare February 12, 2025 06:19
| Db2 | `com.ibm.db2.jcc` | `db2jcc` | [Download](https://www.ibm.com/support/pages/download-db2-fix-packs-version-db2-linux-unix-and-windows) |
| Trino | `io.trino` | `trino-jdbc` | [Download](https://repo1.maven.org/maven2/io/trino/trino-jdbc/) |
| OceanBase | `com.oceanbase` | `oceanbase-client` | [Download](https://repo1.maven.org/maven2/com/oceanbase/oceanbase-client/) |
| Spanner | `com.google.cloud` | `google-cloud-spanner-jdbc` | [Download](https://central.sonatype.com/artifact/com.google.cloud/google-cloud-spanner-jdbc) |
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the download url points to Maven like the spanner doc. How do I get the jar file?

Copy link
Author

Choose a reason for hiding this comment

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

@@ -702,7 +708,7 @@ SELECT * FROM given_database.test_table2;

Data Type Mapping
----------------
Flink supports connect to several databases which uses dialect like MySQL, Oracle, PostgreSQL, CrateDB, Derby, SQL Server, Db2 and OceanBase. The Derby dialect usually used for testing purpose. The field data type mappings from relational databases data types to Flink SQL data types are listed in the following table, the mapping table can help define JDBC table in Flink easily.
Flink supports connect to several databases which uses dialect like MySQL, Oracle, PostgreSQL, CrateDB, Derby, SQL Server, Db2, OceanBase and Spanner. The Derby dialect usually used for testing purpose. The field data type mappings from relational databases data types to Flink SQL data types are listed in the following table, the mapping table can help define JDBC table in Flink easily.
Copy link
Contributor

Choose a reason for hiding this comment

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

nits:
connect -> the connection
which uses dialect like -> using dialects e.g.
The Derby dialect usually used for testing purpose. -> The Derby dialect is usually used for testing purpose.
JDBC table -> a JDBC table

Copy link
Author

Choose a reason for hiding this comment

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

Fixed: 3c7d225

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants