Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#issuecomment-2020439861 Thanks for the hard work @eskabetxe and thanks everyone for the review @eskabetxe can you also please provide a PR to update `jdbc.md`? -- This is an automated

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin merged PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2 -- 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:

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

wanglijie95 commented on PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#issuecomment-2016528614 @snuyanzin @RocMarshal @eskabetxe Sorry for the late reply. I'm busy this period and have no time to review this PR in detail. Only one minor comment: it would be nice

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#issuecomment-2014972105 Thanks for taking a look @RocMarshal if there is no objections I'm going to merge it in coming days -- This is an automated message from the Apache Git Service. To

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1529961727 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/datasource/transactions/xa/domain/TransactionId.java: ## @@ -0,0 +1,273 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

RocMarshal commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1529646574 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/datasource/transactions/xa/domain/TransactionId.java: ## @@ -0,0 +1,273 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1528693061 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/xa/JdbcXaSinkFunctionState.java: ## @@ -31,7 +31,7 @@ /** Thread-safe (assuming

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1528691110 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/writer/JdbcWriterStateSerializer.java: ## @@ -0,0 +1,76 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1528689986 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/writer/JdbcWriter.java: ## @@ -0,0 +1,219 @@ +/* + * Licensed to the Apache

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1528657911 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/writer/JdbcWriter.java: ## @@ -0,0 +1,219 @@ +/* + * Licensed to the Apache

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1528644716 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/writer/JdbcWriterStateSerializer.java: ## @@ -0,0 +1,76 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1528634655 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/xa/JobSubtask.java: ## @@ -0,0 +1,50 @@ +package org.apache.flink.connector.jdbc.xa; +

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1528627627 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/xa/JdbcXaSinkFunctionState.java: ## @@ -31,7 +31,7 @@ /** Thread-safe (assuming

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#issuecomment-1963784832 @snuyanzin @wanglijie95 is passing now the tests.. could you check when have some time -- This is an automated message from the Apache Git Service. To respond to the

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

wanglijie95 commented on PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#issuecomment-1958712785 @eskabetxe FYI, the failed tests should have been fixed in FLINK-34358 -- This is an automated message from the Apache Git Service. To respond to the message, please log on

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#issuecomment-1923662905 @snuyanzin @MartijnVisser I already change an test to mock Sink.InitContext as in 1.19 add a method with a new class.. But now is falling all dialects for TIMESTAMP_LTZ

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1475842140 ## .github/workflows/weekly.yml: ## @@ -27,25 +27,13 @@ jobs: strategy: matrix: flink_branches: [{ - flink: 1.16-SNAPSHOT, -

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1475842140 ## .github/workflows/weekly.yml: ## @@ -27,25 +27,13 @@ jobs: strategy: matrix: flink_branches: [{ - flink: 1.16-SNAPSHOT, -

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1475815795 ## .github/workflows/weekly.yml: ## @@ -27,25 +27,13 @@ jobs: strategy: matrix: flink_branches: [{ - flink: 1.16-SNAPSHOT, -

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1475815795 ## .github/workflows/weekly.yml: ## @@ -27,25 +27,13 @@ jobs: strategy: matrix: flink_branches: [{ - flink: 1.16-SNAPSHOT, -

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1475775129 ## .github/workflows/weekly.yml: ## @@ -27,25 +27,13 @@ jobs: strategy: matrix: flink_branches: [{ - flink: 1.16-SNAPSHOT, -

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1475772527 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/xa/JobSubtask.java: ## @@ -0,0 +1,50 @@ +package org.apache.flink.connector.jdbc.xa; +

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1475771264 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/writer/JdbcWriter.java: ## @@ -0,0 +1,216 @@ +/* + * Licensed to the Apache

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1475718751 ## pom.xml: ## @@ -47,7 +47,7 @@ under the License. -1.16.2 +1.18.0 Review Comment: I think in that case we probably need

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1475725109 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/datasource/connections/xa/PoolingXaConnectionProvider.java: ## @@ -0,0 +1,207 @@ +/* +

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1475723420 ## .github/workflows/weekly.yml: ## @@ -27,25 +27,13 @@ jobs: strategy: matrix: flink_branches: [{ - flink: 1.16-SNAPSHOT, -

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1475718751 ## pom.xml: ## @@ -47,7 +47,7 @@ under the License. -1.16.2 +1.18.0 Review Comment: I think in that case we probably need

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1452321599 ## flink-connector-jdbc/src/test/java/org/apache/flink/connector/jdbc/sink/writer/AlLeastOnceJdbcWriterTest.java: ## @@ -0,0 +1,88 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1452321352 ## flink-connector-jdbc/src/test/java/org/apache/flink/connector/jdbc/sink/writer/ExactlyOnceJdbcWriterTest.java: ## @@ -0,0 +1,105 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1452316874 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/committer/JdbcCommitableSerializer.java: ## @@ -0,0 +1,37 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1452316648 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/committer/JdbcCommitable.java: ## @@ -0,0 +1,39 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#issuecomment-1881065785 @RocMarshal, @snuyanzin thanks for the review.. I address all code changes, Im working on java docs that are missing a lot. -- This is an automated message from the Apache

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1444686739 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/writer/JdbcWriterState.java: ## @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1444672855 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/writer/JdbcWriterStateSerializer.java: ## @@ -0,0 +1,76 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1444608746 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/datasource/transactions/xa/domain/TransactionId.java: ## @@ -0,0 +1,271 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1444596937 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/JdbcSink.java: ## @@ -107,5 +108,9 @@ public static SinkFunction exactlyOnceSink(

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1444594096 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/datasource/statements/JdbcQueryStatement.java: ## @@ -0,0 +1,34 @@ +/* + * Licensed to

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1444576236 ## pom.xml: ## @@ -47,7 +47,7 @@ under the License. -1.16.2 +1.18.0 Review Comment: Yes, we need the code introduced on

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1444114157 ## flink-connector-jdbc/src/test/java/org/apache/flink/connector/jdbc/sink/writer/ExactlyOnceJdbcWriterTest.java: ## @@ -0,0 +1,105 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1444114075 ## flink-connector-jdbc/src/test/java/org/apache/flink/connector/jdbc/sink/writer/AlLeastOnceJdbcWriterTest.java: ## @@ -0,0 +1,88 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1444113959 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/xa/JobSubtask.java: ## @@ -0,0 +1,50 @@ +package org.apache.flink.connector.jdbc.xa; +

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1444113628 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/committer/JdbcCommitter.java: ## @@ -0,0 +1,62 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1444113533 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/committer/JdbcCommitable.java: ## @@ -0,0 +1,39 @@ +package

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1444113024 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/writer/JdbcWriterState.java: ## @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

snuyanzin commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1444112306 ## pom.xml: ## @@ -47,7 +47,7 @@ under the License. -1.16.2 +1.18.0 Review Comment: Are we going to drop support for flink

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

RocMarshal commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1442655533 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/datasource/statements/JdbcQueryStatement.java: ## @@ -0,0 +1,34 @@ +/* + * Licensed to

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

RocMarshal commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1442712519 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/datasource/statements/JdbcQueryStatement.java: ## @@ -0,0 +1,34 @@ +/* + * Licensed to

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#issuecomment-1876772728 @RocMarshal thanks for the review.. the 3 commits are there to allow to review each semantic separately (I thing its easy), we could squash all in one after finishing the

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1441520406 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/JdbcSinkBuilder.java: ## @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1441520106 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/sink/JdbcSinkBuilder.java: ## @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software

Re: [PR] [FLINK-25421] Port JDBC Sink to new Unified Sink API (FLIP-143) [flink-connector-jdbc]

eskabetxe commented on code in PR #2: URL: https://github.com/apache/flink-connector-jdbc/pull/2#discussion_r1441516846 ## flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/datasource/transactions/xa/XaTransaction.java: ## @@ -0,0 +1,295 @@ +package