You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "eskabetxe (via GitHub)" <gi...@apache.org> on 2023/08/24 12:50:53 UTC
[GitHub] [flink-connector-jdbc] eskabetxe opened a new pull request, #73: [FLINK-31332] Limit the use of ExecutionConfig on JdbcOutputFormat
eskabetxe opened a new pull request, #73:
URL: https://github.com/apache/flink-connector-jdbc/pull/73
This change is to remove the use of ExecutionConfig from JdbcOutputFormat, preparing the code for SinkV2 with the changes introduced on 1.18 in [FLIP-287](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=240880853)
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-connector-jdbc] eskabetxe commented on pull request #73: [FLINK-31332] Limit the use of ExecutionConfig on JdbcOutputFormat
Posted by "eskabetxe (via GitHub)" <gi...@apache.org>.
eskabetxe commented on PR #73:
URL: https://github.com/apache/flink-connector-jdbc/pull/73#issuecomment-1725055894
@snuyanzin could you check?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-31332] Limit the use of ExecutionConfig on JdbcOutputFormat [flink-connector-jdbc]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #73:
URL: https://github.com/apache/flink-connector-jdbc/pull/73#discussion_r1373838435
##########
flink-connector-jdbc/src/test/java/org/apache/flink/connector/jdbc/internal/JdbcOutputSerializerTest.java:
##########
@@ -0,0 +1,36 @@
+package org.apache.flink.connector.jdbc.internal;
+
+import org.apache.flink.api.common.ExecutionConfig;
+import org.apache.flink.api.common.typeinfo.TypeInformation;
+import org.apache.flink.api.common.typeutils.TypeSerializer;
+import org.apache.flink.types.Row;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
Review Comment:
since in most of the places there is `org.assertj.core.api.Assertions.assertThat;` and IIRC somewhere in Flink guide it was mentioned that it's better to use `org.assertj.core.api.Assertions.assertThat;`
could we use it here
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-31332] Limit the use of ExecutionConfig on JdbcOutputFormat [flink-connector-jdbc]
Posted by "eskabetxe (via GitHub)" <gi...@apache.org>.
eskabetxe commented on code in PR #73:
URL: https://github.com/apache/flink-connector-jdbc/pull/73#discussion_r1374455266
##########
flink-connector-jdbc/src/test/java/org/apache/flink/connector/jdbc/internal/JdbcOutputSerializerTest.java:
##########
@@ -0,0 +1,36 @@
+package org.apache.flink.connector.jdbc.internal;
+
+import org.apache.flink.api.common.ExecutionConfig;
+import org.apache.flink.api.common.typeinfo.TypeInformation;
+import org.apache.flink.api.common.typeutils.TypeSerializer;
+import org.apache.flink.types.Row;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
Review Comment:
Fixed
##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/table/JdbcOutputFormatBuilder.java:
##########
@@ -100,10 +98,7 @@ public JdbcOutputFormatBuilder setFieldDataTypes(DataType[] fieldDataTypes) {
return new JdbcOutputFormat<>(
new SimpleJdbcConnectionProvider(jdbcOptions),
executionOptions,
- ctx ->
- createBufferReduceExecutor(
- dmlOptions, ctx, rowDataTypeInformation, logicalTypes),
Review Comment:
Done
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-31332] Limit the use of ExecutionConfig on JdbcOutputFormat [flink-connector-jdbc]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #73:
URL: https://github.com/apache/flink-connector-jdbc/pull/73#issuecomment-1816180988
sorry for the delay, merging
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-connector-jdbc] eskabetxe commented on pull request #73: [FLINK-31332] Limit the use of ExecutionConfig on JdbcOutputFormat
Posted by "eskabetxe (via GitHub)" <gi...@apache.org>.
eskabetxe commented on PR #73:
URL: https://github.com/apache/flink-connector-jdbc/pull/73#issuecomment-1692040637
@wanglijie95, @MartijnVisser could you check this
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-31332] Limit the use of ExecutionConfig on JdbcOutputFormat [flink-connector-jdbc]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin merged PR #73:
URL: https://github.com/apache/flink-connector-jdbc/pull/73
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-31332] Limit the use of ExecutionConfig on JdbcOutputFormat [flink-connector-jdbc]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #73:
URL: https://github.com/apache/flink-connector-jdbc/pull/73#discussion_r1373843729
##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/table/JdbcOutputFormatBuilder.java:
##########
@@ -100,10 +98,7 @@ public JdbcOutputFormatBuilder setFieldDataTypes(DataType[] fieldDataTypes) {
return new JdbcOutputFormat<>(
new SimpleJdbcConnectionProvider(jdbcOptions),
executionOptions,
- ctx ->
- createBufferReduceExecutor(
- dmlOptions, ctx, rowDataTypeInformation, logicalTypes),
Review Comment:
since all the usages of `rowDataTypeInformation` are removed, could we remove the field itself?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org