[FLINK-31551] Add support for CrateDB #29

matriv · 2023-03-21T15:24:54Z

Add support for CrateDB

boring-cyborg · 2023-03-21T15:24:59Z

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

This is required by https://github.com/crate/cratedb-flink-jobs until apache/flink-connector-jdbc#29 is merged.

libenchao

@matriv Thanks for the PR, generally it looks good, I've left some comments inlined.

Besides, I see a lot "Postgres" in your code, is that because you copied from PG dialect and forgot to change some places, or CrateDB is just very like Postgres so that you chose to keep "Postgres" in the code?

libenchao · 2023-03-25T15:43:09Z

docs/content/docs/connectors/table/jdbc.md

+| PostgreSQL | `org.postgresql`           | `postgresql`           | [Download](https://jdbc.postgresql.org/download.html) |
+| Derby      | `org.apache.derby`         | `derby`                | [Download](http://db.apache.org/derby/derby_downloads.html) |
+| SQL Server | `com.microsoft.sqlserver`  | `mssql-jdbc`           | [Download](https://docs.microsoft.com/en-us/sql/connect/jdbc/download-microsoft-jdbc-driver-for-sql-server?view=sql-server-ver16) |
+| CrateDB    | `io.crate`                 | `crate-jdbc`           | [Download](https://repo1.maven.org/maven2/io/crate/crate-jdbc/) |


Could you also help to update the corresponding section in 'zh' doc?

Also add a section in the "JDBC Catalog".

Done, but this part requires translation to chinese, could you please take care of this?

...k-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/catalog/CrateDBTablePath.java

flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/catalog/CrateDBCatalog.java

...ector-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/cratedb/CrateDBDialect.java

...nnector-jdbc/src/test/java/org/apache/flink/connector/jdbc/catalog/CrateDBTablePathTest.java

eskabetxe

@matriv its seems that 80% of classes are the same from Postgres.. Could make sense to create a abstraction with common code and both extends this new abstraction?

flink-connector-jdbc/archunit-violations/6b9ab1b0-c14d-4667-bab5-407b81fba98b

matriv · 2023-03-27T15:36:10Z

Thx a lot both of you for the review.
I'll try to address the comments soon and ask for another round.

@eskabetxe True, CrateDB is very similar in connectivity with PostgreSQL and some things could be even simpler in this PR if we implement a change to support this: https://issues.apache.org/jira/browse/FLINK-31553, because then we wouldn't even need the CrateDB fork of the JDBC , and the cratedb:// urls.

Definitely I will try to avoid duplication and use more from the PostgreSQL code.

matriv · 2023-03-29T13:02:06Z

Apologies for the review requests.
I thought it was some github hiccup, but I apparently I cannot ask for re-review from both?

flink-connector-jdbc/archunit-violations/6b9ab1b0-c14d-4667-bab5-407b81fba98b

...-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/catalog/PostgresTablePath.java

...or-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/cratedb/CrateDBTypeMapper.java

...ctor-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/psql/PostgresTypeMapper.java

...or-jdbc/src/test/java/org/apache/flink/connector/jdbc/databases/cratedb/CrateDBDatabase.java

...bc/src/main/java/org/apache/flink/connector/jdbc/internal/converter/CrateDBRowConverter.java

docs/content.zh/docs/connectors/table/jdbc.md

...g/apache/flink/connector/jdbc/internal/converter/AbstractPostgresCompatibleRowConverter.java

...ector-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/cratedb/CrateDBDialect.java

matriv · 2023-03-31T08:18:19Z

@eskabetxe When you have the time, please take another look.

matriv · 2023-04-05T10:52:21Z

Checking the test failure because of different Timezone (UTC vs Europe/Athens that I have locally)

MartijnVisser · 2023-05-24T09:44:16Z

@matriv Could you rebase and take the latest changes from @eskabetxe into account that were introduced via #22 ?

matriv · 2023-05-24T11:24:55Z

@MartijnVisser done!

matriv · 2023-05-24T12:11:45Z

apologies, I think you need to rerun the CI as I've pushed one more commit.

MartijnVisser · 2023-05-24T12:16:15Z

apologies, I think you need to rerun the CI as I've pushed one more commit.

I started it after you pushed it, so it's reflecting the current state of the PR.

matriv · 2023-05-24T12:18:02Z

Asked because I see that jobs are cancelled: https://github.com/apache/flink-connector-jdbc/actions/runs/5068186731/jobs/9100199661?pr=29#step:13:347

MartijnVisser · 2023-05-24T12:25:17Z

Asked because I see that jobs are cancelled

That's because the run for Flink 1.17.0 on Java 11 already failed, so then it cancels all others run too

matriv · 2023-05-24T15:11:10Z

Any hints? locally I don't get such error.

snuyanzin · 2023-05-24T15:18:22Z

looks like different jna versions do not pass convergence check
this command fails locally

mvn clean install -DskipTests -Dflink.convergence.phase=install -Pcheck-convergence

Dependency convergence error for net.java.dev.jna:jna:4.2.1 paths to dependency are:
+-org.apache.flink:flink-connector-jdbc:3.2-SNAPSHOT
  +-io.crate:crate-jdbc:2.7.0
    +-com.github.dblock.waffle:waffle-jna:1.7.5
      +-net.java.dev.jna:jna:4.2.1
and
+-org.apache.flink:flink-connector-jdbc:3.2-SNAPSHOT
  +-io.crate:crate-jdbc:2.7.0
    +-net.java.dev.jna:jna:4.2.1
and
+-org.apache.flink:flink-connector-jdbc:3.2-SNAPSHOT
  +-io.crate:crate-jdbc:2.7.0
    +-net.java.dev.jna:jna-platform:4.2.1
      +-net.java.dev.jna:jna:4.2.1
and
+-org.apache.flink:flink-connector-jdbc:3.2-SNAPSHOT
  +-org.testcontainers:junit-jupiter:1.17.2
    +-org.testcontainers:testcontainers:1.17.2
      +-com.github.docker-java:docker-java-transport-zerodep:3.2.13
        +-net.java.dev.jna:jna:5.8.0

[WARNING] Rule 0: org.apache.maven.plugins.enforcer.DependencyConvergence failed with message:
Failed while enforcing releasability. See above detailed error message.

do we need crate's dependency on jna or can we exclude it?

matriv · 2023-05-24T15:45:54Z

do we need crate's dependency on jna or can we exclude it?

We can exclude it.

matriv · 2023-05-24T16:01:30Z

thx @snuyanzin, excluded the jna dependency from crate-jdbc.

matriv · 2023-05-26T13:16:34Z

@MartijnVisser @snuyanzin I've rebased again, can you please run ci again?
It would be great to merge it if everything passes, thx.

...k-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/catalog/JdbcCatalogUtils.java

snuyanzin · 2023-05-31T13:55:36Z

Thanks for addressing comments
it looks ok from my side

Thanks for review @eskabetxe @libenchao, please add anything if you have something not covered

libenchao · 2023-06-01T05:05:23Z

@snuyanzin I don't have further comments. There is one thing we can do is to create a Jira about the translation for newly added docs about CrateDB.

matriv · 2023-06-01T08:15:42Z

Thx @libenchao! Created subtask issue: https://issues.apache.org/jira/browse/FLINK-32235

snuyanzin · 2023-06-02T15:09:10Z

thanks for the contribution @matriv
thanks for the review @libenchao , @eskabetxe

i'm about to merge it this weekend

matriv · 2023-06-02T15:10:07Z

Thank you all involved in the PR and for all the iterations!

boring-cyborg · 2023-06-03T13:41:16Z

Awesome work, congrats on your first merged pull request!

boring-cyborg bot added component=Connectors/JDBC component=Documentation labels Mar 21, 2023

matriv force-pushed the mt/add-cratedb branch from 5c9002a to 20802c2 Compare March 21, 2023 16:42

amotl mentioned this pull request Mar 22, 2023

Improve documentation to clarify purpose of the driver crate/crate-jdbc#365

Merged

matriv added a commit to crate-workbench/jarfiles that referenced this pull request Mar 24, 2023

Add temp build of flink-jdbc-connector with CrateDB support

e16572d

This is required by https://github.com/crate/cratedb-flink-jobs until apache/flink-connector-jdbc#29 is merged.

libenchao reviewed Mar 27, 2023

View reviewed changes

eskabetxe reviewed Mar 27, 2023

View reviewed changes

flink-connector-jdbc/archunit-violations/6b9ab1b0-c14d-4667-bab5-407b81fba98b Outdated Show resolved Hide resolved

amotl mentioned this pull request Mar 29, 2023

[META] Improve presentation and UX around Apache Flink crate/cratedb-flink-jobs#19

Open

12 tasks

matriv requested review from eskabetxe and libenchao and removed request for eskabetxe and libenchao March 29, 2023 11:14

eskabetxe suggested changes Mar 29, 2023

View reviewed changes

matriv requested a review from eskabetxe March 29, 2023 14:36

eskabetxe suggested changes Mar 29, 2023

View reviewed changes

matriv requested a review from eskabetxe March 29, 2023 15:24

Merge remote-tracking branch 'upstream/main' into mt/add-cratedb

66d65fd

revert uneeded change

e44173b

exclude jna as transitive dep of crate-jdbc

f2dd226

matriv added 2 commits May 26, 2023 15:50

Merge remote-tracking branch 'upstream/main' into mt/add-cratedb

5fcd588

fixup after merging with main

8499707

snuyanzin reviewed May 31, 2023

View reviewed changes

...k-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/catalog/JdbcCatalogUtils.java Outdated Show resolved Hide resolved

Move CrateDB below postgres

217ba92

snuyanzin approved these changes Jun 2, 2023

View reviewed changes

snuyanzin merged commit d67de5f into apache:main Jun 3, 2023

matriv deleted the mt/add-cratedb branch January 12, 2024 07:52

amotl mentioned this pull request Mar 14, 2024

Apache Kafka+Flink: Update to cratedb-flink-jobs-0.5, using Flink 1.18 crate/cratedb-examples#367

Merged

grzegorz8 mentioned this pull request Oct 29, 2024

[FLINK-36271] Support reading json and jsonb types in PostgreSQL dialect #141

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-31551] Add support for CrateDB #29

[FLINK-31551] Add support for CrateDB #29

matriv commented Mar 21, 2023

boring-cyborg bot commented Mar 21, 2023

libenchao left a comment

libenchao Mar 25, 2023

libenchao Mar 26, 2023

matriv Mar 29, 2023

eskabetxe left a comment •

edited

Loading

matriv commented Mar 27, 2023 •

edited

Loading

matriv commented Mar 29, 2023

matriv commented Mar 31, 2023

matriv commented Apr 5, 2023

MartijnVisser commented May 24, 2023

matriv commented May 24, 2023

matriv commented May 24, 2023

MartijnVisser commented May 24, 2023

matriv commented May 24, 2023

MartijnVisser commented May 24, 2023

matriv commented May 24, 2023

snuyanzin commented May 24, 2023 •

edited

Loading

matriv commented May 24, 2023

matriv commented May 24, 2023

matriv commented May 26, 2023

snuyanzin commented May 31, 2023

libenchao commented Jun 1, 2023

matriv commented Jun 1, 2023

snuyanzin commented Jun 2, 2023

matriv commented Jun 2, 2023

boring-cyborg bot commented Jun 3, 2023

[FLINK-31551] Add support for CrateDB #29

[FLINK-31551] Add support for CrateDB #29

Conversation

matriv commented Mar 21, 2023

boring-cyborg bot commented Mar 21, 2023

libenchao left a comment

Choose a reason for hiding this comment

libenchao Mar 25, 2023

Choose a reason for hiding this comment

libenchao Mar 26, 2023

Choose a reason for hiding this comment

matriv Mar 29, 2023

Choose a reason for hiding this comment

eskabetxe left a comment • edited Loading

Choose a reason for hiding this comment

matriv commented Mar 27, 2023 • edited Loading

matriv commented Mar 29, 2023

matriv commented Mar 31, 2023

matriv commented Apr 5, 2023

MartijnVisser commented May 24, 2023

matriv commented May 24, 2023

matriv commented May 24, 2023

MartijnVisser commented May 24, 2023

matriv commented May 24, 2023

MartijnVisser commented May 24, 2023

matriv commented May 24, 2023

snuyanzin commented May 24, 2023 • edited Loading

matriv commented May 24, 2023

matriv commented May 24, 2023

matriv commented May 26, 2023

snuyanzin commented May 31, 2023

libenchao commented Jun 1, 2023

matriv commented Jun 1, 2023

snuyanzin commented Jun 2, 2023

matriv commented Jun 2, 2023

boring-cyborg bot commented Jun 3, 2023

eskabetxe left a comment •

edited

Loading

matriv commented Mar 27, 2023 •

edited

Loading

snuyanzin commented May 24, 2023 •

edited

Loading