Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[hotfix-#1799][connector][jdbc] Fix related issues when the write mode is replace/update #1800

Merged
merged 1 commit into from
Sep 13, 2023

Conversation

libailin
Copy link
Contributor

@libailin libailin commented Sep 6, 2023

Purpose of this pull request

Which issue you fix

Fixes #1799

Checklist:

  • [] I have executed the 'mvn spotless:apply' command to format my code.
  • [] I have a meaningful commit message (including the issue id, the template of commit message is '[label-type-#issue-id][fixed-module] a meaningful commit message.')
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have checked my code and corrected any misspellings.
  • My commit is only one. (If there are multiple commits, you can use 'git squash' to compress multiple commits into one.)

@Paddy0523
Copy link
Contributor

@libailin

@Paddy0523
Copy link
Contributor

image

@libailin
Copy link
Contributor Author

看到你回复的非常详细,我仔细想了想,如果截图这段去掉的话,
8ea65985f25eab2e1dd414d6a807de32
那么sql 模式下,就必须要在 create table时声明 PRIMARY KEY ,这样是不是不太方便。json模式是可以自动获取的。
我有个两个想法,麻烦指教下。
1、能否把我新加的写入模式 WRITE_MODE 改成 sql模式下自动获取主键(索引)的开关配置,默认关闭,打开的时候再执行自动获取逻辑
2、保持原来逻辑,从pr里去掉新增的 写入模式 和 自动获取主键(索引)相关代码。必须要用户手动声明主键字段。

经过讨论 采用 第2种

@Paddy0523
Copy link
Contributor

主要是为了让插件的行为和SQL语义保持一致

…ite mode is replace/update

[hotfix-DTStack#1799][connector][jdbc] Fixed related issues when the write mode is replace/update
Copy link
Contributor

@Paddy0523 Paddy0523 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -52,4 +54,10 @@ public class JdbcSinkOptions {
.stringType()
.noDefaultValue()
.withDescription("sink.post-sql");

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的WriteMode看起来只是决定了OuputFormat#OpenInternal里面是否会从元数据中获取UniqueKey,不会导致WriteMode真正的变更,看起来不太恰当
解释一下为何之前没有地方设置写入方式:之前WriteMode会取决于是否有主键,一般来说有主键的情况下基本就是回撤流,需要upsert语义,而UpsertDeleteWrapper也能兼容appendOnly的场景,这里应该是没有问题的(我们没法好的办法判断当前connector是否会接受回撤流,所以只好用主键来决策)

@@ -103,6 +106,23 @@ protected void openInternal(int taskNumber, int numTasks) {
log.info("updateKey = {}", JsonUtil.toJson(tableIndex));
}
}
if (!CollectionUtil.isNullOrEmpty(jdbcConfig.getUniqueKey())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里会强制获取表中的主键、唯一索引信息。 SQL作业应该尽量遵从SQL本身的语义。比如SQL中只写了一个主键,前面的groupby等操作一般都是基于这个唯一的主键去做的。但是在这里获取了额外的信息,那么实际写操作的时候,就会和SQL本身语义不符

@Paddy0523
Copy link
Contributor

LGTM

@Paddy0523 Paddy0523 merged commit 2fb4967 into DTStack:master Sep 13, 2023
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] [connector][jdbc] 当写入模式是replace/update时相关问题
2 participants