You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "Mrart (via GitHub)" <gi...@apache.org> on 2024/03/21 07:03:36 UTC
[PR] [FLINK-34901][connectors/jdbc]Improve the performance of the update Postgres database [flink-connector-jdbc]
Mrart opened a new pull request, #108:
URL: https://github.com/apache/flink-connector-jdbc/pull/108
Improve the performance of the update Postgres database
--
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-34901][connectors/jdbc]Update clause must EXCLUDED unique index. [flink-connector-jdbc]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #108:
URL: https://github.com/apache/flink-connector-jdbc/pull/108#issuecomment-3591042014
This PR is being marked as stale since it has not had any activity in the last 90 days.
If you would like to keep this PR alive, please leave a comment asking for a review.
If the PR has merge conflicts, update it with the latest from the base branch.
If you are having difficulty finding a reviewer, please reach out to the
community, contact details can be found here: https://flink.apache.org/what-is-flink/community/
If this PR is no longer valid or desired, please feel free to close it.
If no activity occurs in the next 30 days, it will be automatically closed.
--
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-34901][connectors/jdbc]Update clause must EXCLUDED unique index. [flink-connector-jdbc]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui commented on code in PR #108:
URL: https://github.com/apache/flink-connector-jdbc/pull/108#discussion_r1615444568
##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/AbstractPostgresCompatibleDialect.java:
##########
@@ -50,12 +50,14 @@ public String getLimitClause(long limit) {
@Override
public Optional<String> getUpsertStatement(
String tableName, String[] fieldNames, String[] uniqueKeyFields) {
+ Set<String> uniqueKeyFieldsSet = Arrays.stream(uniqueKeyFields).collect(Collectors.toSet());
String uniqueColumns =
- Arrays.stream(uniqueKeyFields)
+ uniqueKeyFieldsSet.stream()
.map(this::quoteIdentifier)
.collect(Collectors.joining(", "));
String updateClause =
Arrays.stream(fieldNames)
+ .filter(f -> !uniqueKeyFieldsSet.contains(f))
Review Comment:
It's better to add a test to check it if it's a bug.
--
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-34901][connectors/jdbc]Update clause must EXCLUDED unique index. [flink-connector-jdbc]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui commented on code in PR #108:
URL: https://github.com/apache/flink-connector-jdbc/pull/108#discussion_r1615442044
##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/AbstractPostgresCompatibleDialect.java:
##########
@@ -50,12 +50,14 @@ public String getLimitClause(long limit) {
@Override
public Optional<String> getUpsertStatement(
String tableName, String[] fieldNames, String[] uniqueKeyFields) {
+ Set<String> uniqueKeyFieldsSet = Arrays.stream(uniqueKeyFields).collect(Collectors.toSet());
String uniqueColumns =
- Arrays.stream(uniqueKeyFields)
+ uniqueKeyFieldsSet.stream()
.map(this::quoteIdentifier)
.collect(Collectors.joining(", "));
String updateClause =
Arrays.stream(fieldNames)
+ .filter(f -> !uniqueKeyFieldsSet.contains(f))
Review Comment:
I'm curious is this a bug or performance improvement?
Also cc @RocMarshal , would you mind reviewing this PR as well (as you are the active contributor of jdbc connector)?
--
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-34901][connectors/jdbc]Improve the performance of the update Postgres database [flink-connector-jdbc]
Posted by "Mrart (via GitHub)" <gi...@apache.org>.
Mrart commented on PR #108:
URL: https://github.com/apache/flink-connector-jdbc/pull/108#issuecomment-2109064865
@1996fanrui Can you help me review?
--
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-34901][connectors/jdbc]Update clause must EXCLUDED unique index. [flink-connector-jdbc]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui commented on PR #108:
URL: https://github.com/apache/flink-connector-jdbc/pull/108#issuecomment-2132593834
Hi @Mrart , the CI is failed, could you rebase the main branch and check the CI?
--
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-34901][connectors/jdbc]Improve the performance of the update Postgres database [flink-connector-jdbc]
Posted by "1996fanrui (via GitHub)" <gi...@apache.org>.
1996fanrui commented on code in PR #108:
URL: https://github.com/apache/flink-connector-jdbc/pull/108#discussion_r1599270530
##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/AbstractPostgresCompatibleDialect.java:
##########
@@ -56,6 +56,7 @@ public Optional<String> getUpsertStatement(
.collect(Collectors.joining(", "));
String updateClause =
Arrays.stream(fieldNames)
+ .filter(f -> !Arrays.asList(uniqueKeyFields).contains(f))
Review Comment:
Would you mind elaborate on why adding this filter?
If adding this filter makes sense, I left 2 comments about code logic:
- We can initialize the `Arrays.asList(uniqueKeyFields)` only once. IIUC, this array will be converted to List multiple times when `fieldNames` length > 0.
- We call the contains method, it's better to use Set instead of List. right?
--
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-34901][connectors/jdbc]Update clause must EXCLUDED unique index. [flink-connector-jdbc]
Posted by "eskabetxe (via GitHub)" <gi...@apache.org>.
eskabetxe commented on PR #108:
URL: https://github.com/apache/flink-connector-jdbc/pull/108#issuecomment-2265871686
@Mrart can you rebase and add a test?
--
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-34901][connectors/jdbc]Update clause must EXCLUDED unique index. [flink-connector-jdbc]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #108:
URL: https://github.com/apache/flink-connector-jdbc/pull/108#issuecomment-3695580885
This PR has been closed since it has not had any activity in 120 days.
If you feel like this was a mistake, or you would like to continue working on it,
please feel free to re-open the PR and ask for a review.
--
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-34901][connectors/jdbc]Improve the performance of the update Postgres database [flink-connector-jdbc]
Posted by "Mrart (via GitHub)" <gi...@apache.org>.
Mrart commented on code in PR #108:
URL: https://github.com/apache/flink-connector-jdbc/pull/108#discussion_r1611757713
##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/AbstractPostgresCompatibleDialect.java:
##########
@@ -56,6 +56,7 @@ public Optional<String> getUpsertStatement(
.collect(Collectors.joining(", "));
String updateClause =
Arrays.stream(fieldNames)
+ .filter(f -> !Arrays.asList(uniqueKeyFields).contains(f))
Review Comment:
Thanks @1996fanrui , I think that was a little bit inaccurate, update clause must exclude unique index . If not will case execptions like 'modification of distribution columns in OnConflictUpdate is not supported Call getNextException to see other errors in the batch'.
--
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-34901][connectors/jdbc]Improve the performance of the update Postgres database [flink-connector-jdbc]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #108:
URL: https://github.com/apache/flink-connector-jdbc/pull/108#issuecomment-2011343599
Thanks for opening this pull request! Please check out our contributing guidelines. (https://flink.apache.org/contributing/how-to-contribute.html)
--
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-34901][connectors/jdbc]Update clause must EXCLUDED unique index. [flink-connector-jdbc]
Posted by "RocMarshal (via GitHub)" <gi...@apache.org>.
RocMarshal commented on code in PR #108:
URL: https://github.com/apache/flink-connector-jdbc/pull/108#discussion_r1615654170
##########
flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/AbstractPostgresCompatibleDialect.java:
##########
@@ -50,12 +50,14 @@ public String getLimitClause(long limit) {
@Override
public Optional<String> getUpsertStatement(
String tableName, String[] fieldNames, String[] uniqueKeyFields) {
+ Set<String> uniqueKeyFieldsSet = Arrays.stream(uniqueKeyFields).collect(Collectors.toSet());
String uniqueColumns =
- Arrays.stream(uniqueKeyFields)
+ uniqueKeyFieldsSet.stream()
.map(this::quoteIdentifier)
.collect(Collectors.joining(", "));
String updateClause =
Arrays.stream(fieldNames)
+ .filter(f -> !uniqueKeyFieldsSet.contains(f))
Review Comment:
Thanks @Mrart for the contribution & @1996fanrui for the ping.
> It's better to add a test to check it if it's a bug.
+1.
--
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-34901][connectors/jdbc]Update clause must EXCLUDED unique index. [flink-connector-jdbc]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #108: [FLINK-34901][connectors/jdbc]Update clause must EXCLUDED unique index.
URL: https://github.com/apache/flink-connector-jdbc/pull/108
--
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