Skip to content

Conversation

@wcy-fdu
Copy link
Contributor

@wcy-fdu wcy-fdu commented Nov 11, 2022

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

What's changed and what's your intention?

This PR add cache for materialize executor and is used to do pk conflict check. If some cases like double insert or delete wrong value occurs, materialize need to fix it instead of ignore. Some unit tests are to verify the implement,

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

Types of user-facing changes

  • SQL commands, functions, and operators

Release note

This PR has implemented the pk constraint check for the materialized source and table. If the following changes conflict with the existing record in storage, we will let the change overwrite the data in the table and the MVs which depend on it. This behavior also can support the "Upsert" connector. When the connector can not give the whole old value of the delete/update operations, It can just give the pk.

Refer to a related PR or issue link (optional)

close #6239
part of #5949

@wcy-fdu wcy-fdu marked this pull request as draft November 11, 2022 06:26
@github-actions github-actions bot added the type/feature Type: New feature. label Nov 11, 2022
@codecov
Copy link

codecov bot commented Nov 11, 2022

Codecov Report

Merging #6320 (590c556) into main (ddae931) will decrease coverage by 0.17%.
The diff coverage is 49.03%.

@@            Coverage Diff             @@
##             main    #6320      +/-   ##
==========================================
- Coverage   74.28%   74.10%   -0.18%     
==========================================
  Files         960      966       +6     
  Lines      156525   157743    +1218     
==========================================
+ Hits       116277   116899     +622     
- Misses      40248    40844     +596     
Flag Coverage Δ
rust 74.10% <49.03%> (-0.18%) ⬇️

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

Impacted Files Coverage Δ
src/common/src/types/chrono_wrapper.rs 46.66% <0.00%> (-15.91%) ⬇️
src/common/src/types/interval.rs 72.10% <0.00%> (-7.47%) ⬇️
src/common/src/types/mod.rs 72.40% <0.00%> (ø)
src/ctl/src/cmd_impl/table/scan.rs 0.00% <0.00%> (ø)
src/expr/src/expr/build_expr_from_prost.rs 53.76% <0.00%> (-1.91%) ⬇️
src/expr/src/expr/expr_binary_nonnull.rs 62.80% <0.00%> (-8.89%) ⬇️
src/expr/src/expr/mod.rs 47.91% <0.00%> (-1.02%) ⬇️
src/expr/src/vector_op/date_trunc.rs 0.00% <0.00%> (ø)
src/frontend/src/observer/observer_manager.rs 0.00% <0.00%> (ø)
src/frontend/src/optimizer/plan_node/stream.rs 13.30% <0.00%> (ø)
... and 67 more

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

@wcy-fdu wcy-fdu marked this pull request as ready for review November 14, 2022 08:44
@wcy-fdu wcy-fdu requested a review from st1page November 15, 2022 07:38
Copy link
Contributor

@st1page st1page left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify mergify bot merged commit 8c40184 into main Nov 16, 2022
@mergify mergify bot deleted the wcy/add_cache_for_materialize branch November 16, 2022 07:37
@st1page st1page mentioned this pull request Nov 17, 2022
9 tasks
@st1page st1page added the user-facing-changes Contains changes that are visible to users label Nov 17, 2022
xxchan pushed a commit that referenced this pull request Nov 19, 2022
)

* temp commit

* framework

* temp_commit

* temp_commit

* temp commit

* framework done

* temp commit

* bug need to fix

* need to add ut

* ensure correctness

* refactor

* todo: add more unit test

* todo: add comments, clean up code

* todo: clean up code

* typo fix

* typo fix

* refactor code

* ready for review

* code refactor

* add e2e and ut

* ready for review

* dashboard

* change cache row to compacted_row and add get_compacted_row for state table

* change cache row to compacted_row and add get_compacted_row for state table

* materialize cache do not store vnode in key

* clean up state table get_row

* materialize source handle pk conflict

* merge main

* merge main

* mv check concurrent read

* improve panic msg

* fix

* refine the code

* clippy

* Delete package-lock.json

* Delete package.json

Co-authored-by: Yuhao Su <[email protected]>
Co-authored-by: st1page <[email protected]>
Co-authored-by: stonepage <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/feature Type: New feature. user-facing-changes Contains changes that are visible to users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicate primary keys cause panic.

4 participants