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

[ARCTIC-1025][FLINK] Fix data duplication even if there is a primary key table with upsert enabled #1180

Closed
wants to merge 4 commits into from

Conversation

zstraw
Copy link
Contributor

@zstraw zstraw commented Mar 1, 2023

FIX #1025

Why are the changes needed?

To make sure only one primary key in key table with upsert enabled, it's necessary to delete data firstly when update_after not followed by update_before.

It may lead to some unnecessary delete data if there are some data interspersed with other primary key data. e.g. K1-UB, K2-UB, K1-UA, K2-UB

Brief change log

  • The flink 1.12 and 1.14, 1.15 versions are affected.

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before making a pull request

Documentation

  • Does this pull request introduces a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@github-actions github-actions bot added the module:mixed-flink Flink moduel for Mixed Format label Mar 1, 2023
@zstraw zstraw requested review from zhoujinsong and YesOrNo828 March 1, 2023 11:40
@zstraw zstraw added the type:bug Something isn't working label Mar 1, 2023
@zstraw zstraw added this to the Release 0.4.1 milestone Mar 1, 2023
@zstraw zstraw changed the title [ARCTIC-1025] Fix data duplication even if there is a primary key table with upsert enabled [ARCTIC-1025][FLINK] Fix data duplication even if there is a primary key table with upsert enabled Mar 1, 2023
Copy link
Contributor

@YesOrNo828 YesOrNo828 left a comment

Choose a reason for hiding this comment

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

@zstraw Thanks for your contribution, I left some comments.

@codecov
Copy link

codecov bot commented Mar 3, 2023

Codecov Report

Patch coverage has no change and project coverage change: +28.09 🎉

Comparison is base (e5c7166) 27.86% compared to head (f65ce9d) 55.95%.

Additional details and impacted files
@@              Coverage Diff              @@
##             master    #1180       +/-   ##
=============================================
+ Coverage     27.86%   55.95%   +28.09%     
+ Complexity     4928     1884     -3044     
=============================================
  Files           657      287      -370     
  Lines         69172    12824    -56348     
  Branches       7978     1222     -6756     
=============================================
- Hits          19274     7176    -12098     
+ Misses        47986     5023    -42963     
+ Partials       1912      625     -1287     
Flag Coverage Δ
core ?
flink 55.95% <ø> (?)
trino ?

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

Impacted Files Coverage Δ
...flink/write/hidden/pulsar/HiddenPulsarFactory.java 83.33% <ø> (ø)
...va/com/netease/arctic/hive/op/UpdateHiveFiles.java 88.04% <0.00%> (ø)
.../netease/arctic/hive/op/ReplaceHivePartitions.java 69.28% <0.00%> (ø)
...ms/server/utils/JDBCSqlSessionFactoryProvider.java
...m/netease/arctic/spark/sql/utils/expressions.scala
...se/arctic/spark/sql/catalyst/plans/MergeRows.scala
...com/netease/arctic/ams/server/ArcticMetaStore.java
...ease/arctic/ams/server/model/CompactRangeType.java
...ctic/ams/server/controller/RestBaseController.java
...etease/arctic/trino/keyed/KeyedConnectorSplit.java
... and 363 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@YesOrNo828 YesOrNo828 left a comment

Choose a reason for hiding this comment

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

LGTM

@zstraw zstraw closed this Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:mixed-flink Flink moduel for Mixed Format type:bug Something isn't working
Projects
None yet
3 participants