You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by GitBox <gi...@apache.org> on 2022/11/21 10:47:28 UTC
[GitHub] [inlong] gong opened a new pull request, #6595: [INLONG-6594][Sort] Fix ClickHouse sort connector throw exception when source is change log stream
gong opened a new pull request, #6595:
URL: https://github.com/apache/inlong/pull/6595
### Prepare a Pull Request
- [INLONG-6594][Sort] Fix ClickHouse sort connector throw exception when source is change log stream
- Fixes #6594
### Motivation
Fix ClickHouse sort connector throw exception when source is change log stream
* ClickHouse throw exception "Can't UPDATE key column id" when update source data. It means that alter table update ... where can't update primaryKey. [refer to](https://stackoverflow.com/questions/55976700/is-it-possible-to-update-primary-key-in-clickhouse-using-mergetree-engine) .
* ClickHouse throw exception "Table default.test_user doesn't exist". But jdbc-url have database name. https://github.com/ClickHouse/ClickHouse/issues/19278. Maybe we should specify database.
### Modifications
* Modify clickhouse Dialect
* Modify clickhouse load node UT, tableName should include 'database.table'
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] gong commented on a diff in pull request #6595: [INLONG-6594][Sort] Fix ClickHouse sort connector throw exception when source is change log stream
Posted by GitBox <gi...@apache.org>.
gong commented on code in PR #6595:
URL: https://github.com/apache/inlong/pull/6595#discussion_r1028163739
##########
inlong-sort/sort-connectors/jdbc/src/main/java/org/apache/inlong/sort/jdbc/dialect/ClickHouseDialect.java:
##########
@@ -154,6 +178,67 @@ public String getDeleteStatement(String tableName, String[] conditionFields) {
Arrays.stream(conditionFields)
.map(f -> format("%s = :%s", quoteIdentifier(f), f))
.collect(Collectors.joining(" AND "));
- return "ALTER TABLE " + quoteIdentifier(tableName) + " DELETE WHERE " + conditionClause;
+ Pair<String, String> pair = getDatabaseAndTableName(tableName);
Review Comment:
ok
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] gong commented on a diff in pull request #6595: [INLONG-6594][Sort] Fix ClickHouse sort connector throw exception when source is change log stream
Posted by GitBox <gi...@apache.org>.
gong commented on code in PR #6595:
URL: https://github.com/apache/inlong/pull/6595#discussion_r1029185263
##########
inlong-sort/sort-connectors/jdbc/src/main/java/org/apache/inlong/sort/jdbc/dialect/ClickHouseDialect.java:
##########
@@ -129,22 +135,40 @@ public List<LogicalTypeRoot> unsupportedTypes() {
@Override
public String getUpdateStatement(
String tableName, String[] fieldNames, String[] conditionFields) {
+ List<String> conditionFieldList = Arrays.asList(conditionFields);
String setClause =
Arrays.stream(fieldNames)
+ .filter(fieldName -> !conditionFieldList.contains(fieldName))
.map(f -> format("%s = :%s", quoteIdentifier(f), f))
.collect(Collectors.joining(", "));
+
String conditionClause =
Arrays.stream(conditionFields)
.map(f -> format("%s = :%s", quoteIdentifier(f), f))
.collect(Collectors.joining(" AND "));
+ Pair<String, String> databaseAndTableName = getDatabaseAndTableName(tableName);
return "ALTER TABLE "
- + quoteIdentifier(tableName)
+ + quoteIdentifier(databaseAndTableName.getLeft())
Review Comment:
> if here has bug, maybe delete statement have the same problem?
@thesumery I test delete is ok. I hava add it for delete.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] EMsnap merged pull request #6595: [INLONG-6594][Sort] Fix ClickHouse connector throw exception when source is change log stream
Posted by GitBox <gi...@apache.org>.
EMsnap merged PR #6595:
URL: https://github.com/apache/inlong/pull/6595
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] gong commented on a diff in pull request #6595: [INLONG-6594][Sort] Fix ClickHouse sort connector throw exception when source is change log stream
Posted by GitBox <gi...@apache.org>.
gong commented on code in PR #6595:
URL: https://github.com/apache/inlong/pull/6595#discussion_r1029179421
##########
inlong-sort/sort-connectors/jdbc/src/main/java/org/apache/inlong/sort/jdbc/dialect/ClickHouseDialect.java:
##########
@@ -129,22 +135,40 @@ public List<LogicalTypeRoot> unsupportedTypes() {
@Override
public String getUpdateStatement(
String tableName, String[] fieldNames, String[] conditionFields) {
+ List<String> conditionFieldList = Arrays.asList(conditionFields);
String setClause =
Arrays.stream(fieldNames)
+ .filter(fieldName -> !conditionFieldList.contains(fieldName))
.map(f -> format("%s = :%s", quoteIdentifier(f), f))
.collect(Collectors.joining(", "));
+
String conditionClause =
Arrays.stream(conditionFields)
.map(f -> format("%s = :%s", quoteIdentifier(f), f))
.collect(Collectors.joining(" AND "));
+ Pair<String, String> databaseAndTableName = getDatabaseAndTableName(tableName);
return "ALTER TABLE "
- + quoteIdentifier(tableName)
+ + quoteIdentifier(databaseAndTableName.getLeft())
Review Comment:
I test delete op, not to found it. But I can add it.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] gong commented on a diff in pull request #6595: [INLONG-6594][Sort] Fix ClickHouse sort connector throw exception when source is change log stream
Posted by GitBox <gi...@apache.org>.
gong commented on code in PR #6595:
URL: https://github.com/apache/inlong/pull/6595#discussion_r1029180297
##########
inlong-sort/sort-connectors/jdbc/src/main/java/org/apache/inlong/sort/jdbc/dialect/ClickHouseDialect.java:
##########
@@ -129,22 +135,40 @@ public List<LogicalTypeRoot> unsupportedTypes() {
@Override
public String getUpdateStatement(
String tableName, String[] fieldNames, String[] conditionFields) {
+ List<String> conditionFieldList = Arrays.asList(conditionFields);
String setClause =
Arrays.stream(fieldNames)
+ .filter(fieldName -> !conditionFieldList.contains(fieldName))
.map(f -> format("%s = :%s", quoteIdentifier(f), f))
.collect(Collectors.joining(", "));
+
String conditionClause =
Arrays.stream(conditionFields)
.map(f -> format("%s = :%s", quoteIdentifier(f), f))
.collect(Collectors.joining(" AND "));
+ Pair<String, String> databaseAndTableName = getDatabaseAndTableName(tableName);
return "ALTER TABLE "
- + quoteIdentifier(tableName)
+ + quoteIdentifier(databaseAndTableName.getLeft())
+ + POINT
+ + quoteIdentifier(databaseAndTableName.getRight())
+ " UPDATE "
+ setClause
+ " WHERE "
+ conditionClause;
}
+ private Pair<String, String> getDatabaseAndTableName(String tableName) {
Review Comment:
ok
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] thesumery commented on a diff in pull request #6595: [INLONG-6594][Sort] Fix ClickHouse sort connector throw exception when source is change log stream
Posted by GitBox <gi...@apache.org>.
thesumery commented on code in PR #6595:
URL: https://github.com/apache/inlong/pull/6595#discussion_r1029059489
##########
inlong-sort/sort-connectors/jdbc/src/main/java/org/apache/inlong/sort/jdbc/dialect/ClickHouseDialect.java:
##########
@@ -129,22 +135,40 @@ public List<LogicalTypeRoot> unsupportedTypes() {
@Override
public String getUpdateStatement(
String tableName, String[] fieldNames, String[] conditionFields) {
+ List<String> conditionFieldList = Arrays.asList(conditionFields);
String setClause =
Arrays.stream(fieldNames)
+ .filter(fieldName -> !conditionFieldList.contains(fieldName))
.map(f -> format("%s = :%s", quoteIdentifier(f), f))
.collect(Collectors.joining(", "));
+
String conditionClause =
Arrays.stream(conditionFields)
.map(f -> format("%s = :%s", quoteIdentifier(f), f))
.collect(Collectors.joining(" AND "));
+ Pair<String, String> databaseAndTableName = getDatabaseAndTableName(tableName);
return "ALTER TABLE "
- + quoteIdentifier(tableName)
+ + quoteIdentifier(databaseAndTableName.getLeft())
+ + POINT
+ + quoteIdentifier(databaseAndTableName.getRight())
+ " UPDATE "
+ setClause
+ " WHERE "
+ conditionClause;
}
+ private Pair<String, String> getDatabaseAndTableName(String tableName) {
Review Comment:
add some coment to describe this issue?
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] EMsnap commented on a diff in pull request #6595: [INLONG-6594][Sort] Fix ClickHouse sort connector throw exception when source is change log stream
Posted by GitBox <gi...@apache.org>.
EMsnap commented on code in PR #6595:
URL: https://github.com/apache/inlong/pull/6595#discussion_r1027881543
##########
inlong-sort/sort-connectors/jdbc/src/main/java/org/apache/inlong/sort/jdbc/dialect/ClickHouseDialect.java:
##########
@@ -154,6 +178,67 @@ public String getDeleteStatement(String tableName, String[] conditionFields) {
Arrays.stream(conditionFields)
.map(f -> format("%s = :%s", quoteIdentifier(f), f))
.collect(Collectors.joining(" AND "));
- return "ALTER TABLE " + quoteIdentifier(tableName) + " DELETE WHERE " + conditionClause;
+ Pair<String, String> pair = getDatabaseAndTableName(tableName);
Review Comment:
use a name that illustrate the actual data, since "pair" can be infered from its class type
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org