Skip to content

Conversation

@nextdreamblue
Copy link
Contributor

…t wrong result

Proposed changes

Issue Number: close #17183

Problem summary

Describe your changes.

Checklist(Required)

  • Does it affect the original behavior
  • Has unit tests been added
  • Has document been added or modified
  • Does it need to update dependencies
  • Is this PR support rollback (If NO, please explain WHY)

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@nextdreamblue
Copy link
Contributor Author

run buildall

@nextdreamblue
Copy link
Contributor Author

will add regression later

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@yiguolei yiguolei changed the title [fix](delete) fix 'is null' or 'is not null' delete predicate will ge… [fix](delete) fix 'is null' or 'is not null' delete predicate will get wrong result Feb 27, 2023
…t wrong result

Signed-off-by: nextdreamblue <zxw520blue1@163.com>
Signed-off-by: nextdreamblue <zxw520blue1@163.com>
@nextdreamblue
Copy link
Contributor Author

run buildall

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@nextdreamblue
Copy link
Contributor Author

run p0

@nextdreamblue
Copy link
Contributor Author

run buildall


bool evaluate_del(const std::pair<WrapperField*, WrapperField*>& statistic) const override {
// evaluate_del only use for delete condition to filter page, need use delete condition origin value,
// when opposite==true, origin value 'is null'->'is not null' and 'is not null'->'is null',
Copy link
Member

Choose a reason for hiding this comment

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

no need to check if opposite==false ?

Copy link
Contributor Author

@nextdreamblue nextdreamblue Mar 1, 2023

Choose a reason for hiding this comment

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

only deletehandler set opposite=true and evaluate_del only use for delete condition to filter page.
when opposite==false, evaluate_del will not be used to filter page.

Copy link
Member

@xy720 xy720 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Mar 1, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2023

PR approved by at least one committer and no changes requested.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2023

PR approved by anyone and no changes requested.

@xy720 xy720 merged commit 9155d8b into apache:master Mar 2, 2023
@morningman morningman added usercase Important user case type label dev/1.2.3-merged and removed dev/1.2.3 labels Mar 3, 2023
morningman pushed a commit that referenced this pull request Mar 6, 2023
…t wrong result (#17190)

fix 'is null' or 'is not null' delete predicate will get wrong result

Signed-off-by: nextdreamblue <zxw520blue1@163.com>
yagagagaga pushed a commit to yagagagaga/doris that referenced this pull request Mar 9, 2023
…t wrong result (apache#17190)

fix 'is null' or 'is not null' delete predicate will get wrong result

Signed-off-by: nextdreamblue <zxw520blue1@163.com>
luwei16 pushed a commit to luwei16/Doris that referenced this pull request Apr 7, 2023
* [fix](delete) fix delete from bug which can get wrong result (apache#17146)

理论上,如果是两次独立的删除,比如delete from table where a=1; delete from table where a=2;其实这个地方应该可以使用的,但是目前的代码,是把所有不同版本的delete predicates和不同列的delete predicates都放到一起了,失去了版本信息、失去了谓词间可能是and的关系,统一弱化成了delete predicates都是独立的,有一个delete predicates满足条件,就把page都去掉。
这个pr的修改方式,就是在当前代码的基础上,当只有一个delete predicate的时候才能保证后续淘汰page的正确性,所以这里一律加了 == 1的判断才传递delete predicates。
如果要把不同版本的delete predicates和不同列的delete predicates作为完整和严谨的逻辑去判断page,需要修改的设计就有点多了,目前的方案算是一种优先解决bug的思路,后续可以进一步把delete predicates这块加速zone判断进行page淘汰的逻辑完善,提高delete predicates使用的场景。

* [fix](delete) fix 'is null' or 'is not null' delete predicate will get wrong result (apache#17190)

fix 'is null' or 'is not null' delete predicate will get wrong result

Signed-off-by: nextdreamblue <zxw520blue1@163.com>

---------

Signed-off-by: nextdreamblue <zxw520blue1@163.com>
Co-authored-by: xueweizhang <zxw520blue1@163.com>
gnehil pushed a commit to gnehil/doris that referenced this pull request Apr 21, 2023
…t wrong result (apache#17190)

fix 'is null' or 'is not null' delete predicate will get wrong result

Signed-off-by: nextdreamblue <zxw520blue1@163.com>
swjtu-zhanglei pushed a commit to swjtu-zhanglei/incubator-doris that referenced this pull request Jul 25, 2023
* [fix](delete) fix delete from bug which can get wrong result (apache#17146)

理论上,如果是两次独立的删除,比如delete from table where a=1; delete from table where a=2;其实这个地方应该可以使用的,但是目前的代码,是把所有不同版本的delete predicates和不同列的delete predicates都放到一起了,失去了版本信息、失去了谓词间可能是and的关系,统一弱化成了delete predicates都是独立的,有一个delete predicates满足条件,就把page都去掉。
这个pr的修改方式,就是在当前代码的基础上,当只有一个delete predicate的时候才能保证后续淘汰page的正确性,所以这里一律加了 == 1的判断才传递delete predicates。
如果要把不同版本的delete predicates和不同列的delete predicates作为完整和严谨的逻辑去判断page,需要修改的设计就有点多了,目前的方案算是一种优先解决bug的思路,后续可以进一步把delete predicates这块加速zone判断进行page淘汰的逻辑完善,提高delete predicates使用的场景。

* [fix](delete) fix 'is null' or 'is not null' delete predicate will get wrong result (apache#17190)

fix 'is null' or 'is not null' delete predicate will get wrong result

Signed-off-by: nextdreamblue <zxw520blue1@163.com>

---------

Signed-off-by: nextdreamblue <zxw520blue1@163.com>
Co-authored-by: xueweizhang <zxw520blue1@163.com>
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. dev/1.2.3-merged kind/test reviewed usercase Important user case type label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] delete from with is null predicate and then query will get wrong result with is not null predicate

3 participants