-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[Fix](auto-increment) Fix duplicate auto-increment column value problem #43774
Conversation
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
run cloud_p0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
1cfd1ea
to
d0e6d79
Compare
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
run beut |
d0e6d79
to
18f1c92
Compare
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
18f1c92
to
6ef086d
Compare
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
6ef086d
to
803da63
Compare
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
803da63
to
ca8de90
Compare
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
TeamCity be ut coverage result: |
run p0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…em (#43774) ```cpp Result<int64_t> AutoIncIDBuffer::_fetch_ids_from_fe(size_t length) { // ... return _rpc_status; } ``` should be ```cpp Result<int64_t> AutoIncIDBuffer::_fetch_ids_from_fe(size_t length) { // ... return ResultError(_rpc_status); } ``` Otherwise, the returned `Result<int64_t>`'s `m_has_val` will be `true`, then `AutoIncIDBuffer::_launch_async_fetch_task()` will wrongly add an auto-increment range [0, length) to `_buffers` which will cause duplicate value problem. ### Release note Fix duplicate auto-increment column value problem in some situations.
…em (#43774) ```cpp Result<int64_t> AutoIncIDBuffer::_fetch_ids_from_fe(size_t length) { // ... return _rpc_status; } ``` should be ```cpp Result<int64_t> AutoIncIDBuffer::_fetch_ids_from_fe(size_t length) { // ... return ResultError(_rpc_status); } ``` Otherwise, the returned `Result<int64_t>`'s `m_has_val` will be `true`, then `AutoIncIDBuffer::_launch_async_fetch_task()` will wrongly add an auto-increment range [0, length) to `_buffers` which will cause duplicate value problem. ### Release note Fix duplicate auto-increment column value problem in some situations.
… value problem #43774 (#43983) Cherry-picked from #43774 Co-authored-by: bobhan1 <[email protected]>
… value problem #43774 (#43984) Cherry-picked from #43774 Co-authored-by: bobhan1 <[email protected]>
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
should be
Otherwise, the returned
Result<int64_t>
'sm_has_val
will betrue
, thenAutoIncIDBuffer::_launch_async_fetch_task()
will wrongly add an auto-increment range [0, length) to_buffers
which will cause duplicate value problem.Release note
Fix duplicate auto-increment column value problem in some situations.
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)