Skip to content

fix(jdbc-sink): use upsert SQL to handle potential duplicate event #10326

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Jun 19, 2023
Merged

fix(jdbc-sink): use upsert SQL to handle potential duplicate event #10326

merged 11 commits into from
Jun 19, 2023

Conversation

@StrikeW
Copy link
Contributor

@StrikeW StrikeW commented Jun 14, 2023 •
edited
Loading

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

  1. use upsert SQL for INSERT and UPDATE event
  2. create prepare statement once and reuse it multiple times
  3. refactor sql dialect implementation

close #10251
close #10303

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
  • My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
  • All checks passed in ./risedev check (or alias, ./risedev c)
  • My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

Documentation

  • My PR contains user-facing changes.
Click here for Documentation

Types of user-facing changes

Please keep the types that apply to your changes, and remove the others.

  • Installation and deployment
  • Connector (sources & sinks)
  • SQL commands, functions, and operators
  • RisingWave cluster configuration changes
  • Other (please specify in the release note below)

Release note

@github-actions github-actions bot added the type/fix Type: Bug fix. Only for pull requests. label Jun 14, 2023
@StrikeW StrikeW requested review from fuyufjh, hzxa21, wenym1 and xx01cyx June 14, 2023 02:46
@StrikeW StrikeW force-pushed the siyuan/jdbc-sink-fix branch from 3f6853b to bee4770 Compare June 14, 2023 03:47
@StrikeW StrikeW requested a review from liurenjie1024 June 14, 2023 07:14
@StrikeW StrikeW changed the title fix(sink): use upsert SQL to handle potential duplicate event fix(jdbc-sink): use upsert SQL to handle potential duplicate event Jun 14, 2023
@codecov
Copy link

codecov bot commented Jun 14, 2023 •
edited
Loading

Codecov Report

Merging #10326 (09ad3d3) into main (ade16ec) will increase coverage by 0.03%.
The diff coverage is n/a.

❗ Current head 09ad3d3 differs from pull request most recent head c2cd72e. Consider uploading reports for the commit c2cd72e to get more accurate results

@@            Coverage Diff             @@
##             main   #10326      +/-   ##
==========================================
+ Coverage   70.48%   70.52%   +0.03%     
==========================================
  Files        1258     1255       -3     
  Lines      214803   213963     -840     
==========================================
- Hits       151403   150890     -513     
+ Misses      63400    63073     -327
Flag Coverage Δ
rust 70.52% <ø> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 34 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

liurenjie1024
liurenjie1024 approved these changes Jun 15, 2023
View reviewed changes
Copy link
Contributor

@liurenjie1024 liurenjie1024 left a comment

Choose a reason for hiding this comment

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

LGTM

wenym1
wenym1 reviewed Jun 16, 2023
View reviewed changes
Copy link
Contributor

@wenym1 wenym1 left a comment

Choose a reason for hiding this comment

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

Rest LGTM

@StrikeW StrikeW enabled auto-merge June 19, 2023 07:57
@StrikeW StrikeW added this pull request to the merge queue Jun 19, 2023
Merged via the queue into main with commit 04edaf3 Jun 19, 2023
@StrikeW StrikeW deleted the siyuan/jdbc-sink-fix branch June 19, 2023 09:08
@StrikeW StrikeW mentioned this pull request Jun 29, 2023
Closed
StrikeW added a commit that referenced this pull request Jun 29, 2023
@StrikeW
fix(jdbc-sink): use upsert SQL to handle potential duplicate event (#…
4640a89 
…10326)
lmatz added a commit that referenced this pull request Jun 30, 2023
@StrikeW @lmatz
fix(jdbc-sink): cherry-pick #10326 and #10576 to v0.19.0-rc (#10651)
2dfba7f 
Co-authored-by: lmatz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wenym1 wenym1 wenym1 left review comments

@hzxa21 hzxa21 Awaiting requested review from hzxa21

@fuyufjh fuyufjh Awaiting requested review from fuyufjh

@xx01cyx xx01cyx Awaiting requested review from xx01cyx

+1 more reviewer

@liurenjie1024 liurenjie1024 liurenjie1024 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

type/fix Type: Bug fix. Only for pull requests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

JDBC sink creates prepare statement for each row which is unnecessary JDBC sink may encounter duplicate key on downstream

5 participants

@StrikeW @liurenjie1024 @wenym1 @lmatz