Skip to content

Conversation

@acelyc111
Copy link
Member

Proposed changes

This patch mainly do the following refactors:

  • Use int64_t instead of int32_t for 'version' in DeleteHandler
  • Move some comments from .cpp to .h file, add some new comments in .h files, and also remove some meaningless comments
  • Use switch...case... instead of multiple if..else.. for DeleteConditionHandler::is_condition_value_valid
  • Use range loop to simplify code
  • Reduce some compare operations in Cond::del_eval
  • Improve some branch predictions in Reader
  • Fix and improve some unit tests

Compare with #4925 , this patch fix two bugs:

  • Initialize '_collect_iter' which missing initialized when resolve conflict
  • Make DeleteHandler and Conditions not copiable and not assignable to avoid double free

Types of changes

What types of changes does your code introduce to Doris?
Put an x in the boxes that apply

  • [] Bugfix (non-breaking change which fixes an issue)
  • [] New feature (non-breaking change which adds functionality)
  • [] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [] Documentation Update (if none of the other choices apply)
  • Code refactor (Modify the code structure, format the code, etc...)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • [] I have create an issue on (Fix #ISSUE), and have described the bug/feature there in detail
  • Compiling and unit tests pass locally with my changes
  • [] I have added tests that prove my fix is effective or that my feature works
  • [] If this change need a document change, I have updated the document
  • [] Any dependent changes have been merged

This patch mainly do the following refactors:
- Use int64_t instead of int32_t for 'version' in DeleteHandler
- Move some comments from .cpp to .h file, add some new comments in .h files, and also remove some meaningless comments
- Use switch...case... instead of multiple if..else.. for DeleteConditionHandler::is_condition_value_valid
- Use range loop to simplify code
- Reduce some compare operations in Cond::del_eval
- Improve some branch predictions in Reader
- Fix and improve some unit tests
@morningman morningman added approved Indicates a PR has been approved by one committer. kind/refactor Issues or PRs to refactor code labels Dec 7, 2020
Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

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

LGTM

@yangzhg yangzhg merged commit 49f7eb6 into apache:master Dec 8, 2020
morningman pushed a commit to morningman/doris that referenced this pull request Dec 17, 2020
If a column does not have any null value, and execute a delete operation
with "where k1 is null" on it, BE may crash.

This bug is introducaed from apache#5030
morningman added a commit that referenced this pull request Dec 18, 2020
…5109)

If a column does not have any null value, and execute a delete operation
with "where k1 is null" on it, BE may crash.

This bug is introducaed from #5030
morningman added a commit to baidu-doris/incubator-doris that referenced this pull request Jan 5, 2021
…pache#5109)

If a column does not have any null value, and execute a delete operation
with "where k1 is null" on it, BE may crash.

This bug is introducaed from apache#5030
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. kind/refactor Issues or PRs to refactor code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants