[Feature] Support InPredicate in delete statement #4006

caiconghui · 2020-07-02T14:09:30Z

This PR is to add inPredicate support to delete statement, and add max_allowed_in_element_num_of_delete variable to limit element num of InPredicate in delete statement.

caiconghui · 2020-07-02T14:14:24Z

for #4008

morningman · 2020-07-02T14:36:01Z

In fact, we are also trying to implement a batch delete function that can support the deletion of a large number of specified keys (including specifying multiple values in inPredicate)

In your current implementation, if there are too many values in InPredicate (for example, tens of thousands), then each column must be looped through tens of thousands of conditions during query, which may be inefficient.

Maybe you can discuss it with @yangzhg about it.

fe/src/main/java/org/apache/doris/load/DeleteHandler.java

fe/src/main/java/org/apache/doris/qe/SessionVariable.java

be/src/olap/delete_handler.cpp

be/src/olap/olap_cond.cpp

be/src/olap/olap_cond.h

fe/src/main/java/org/apache/doris/load/DeleteHandler.java

fe/fe-core/src/main/java/org/apache/doris/analysis/DeleteStmt.java

fe/fe-core/src/main/java/org/apache/doris/load/DeleteHandler.java

morningman · 2020-07-29T14:50:51Z

be/src/olap/olap_cond.cpp

+        wrapperField.release_field();
+        return ret;
+    }
+    case OP_NOT_IN: {


These 2 case (OP_IN and OP_NOT_IN) looks same?

one is "!="(line 196), the other is "=="(line 202)

morningman · 2020-07-29T15:03:48Z

be/src/olap/olap_cond.cpp

                                 tcond.column_name.c_str(), operand.c_str(), op);
                return res;
            }
+            if (min_value_field == nullptr || f->cmp(min_value_field) > 0) {


I am a little confused.

A->cmp(B) > 0 means A > B or A < B?

my bad, I will fix it A->cmp(B) > 0 means A > B

morningman · 2020-07-29T15:07:26Z

be/src/olap/olap_cond.h

+    // valid when op is OP_IN or OP_NOT_IN
    typedef std::unordered_set<const WrapperField*, FieldHash, FieldEqual> FieldSet;
    FieldSet operand_set;
+    WrapperField* min_value_field;


Add comment to explains these 2 new fields.

morningman

LGTM

morningman · 2020-08-01T10:05:49Z

UT failed.
FE：
DeleteStmtTest.testAnalyze

fe/fe-core/src/main/java/org/apache/doris/common/Config.java

morningman · 2020-08-01T13:42:34Z

be/src/olap/delete_handler.cpp

-        string condition_str = construct_sub_predicates(condition);
-        del_pred->add_sub_predicates(condition_str);
-        LOG(INFO) << "store one sub-delete condition. condition=" << condition_str;
+        if (condition.condition_values.size() > 1) {


I think it's better to check the condition.condition_op to see if this condition is InPredicate, not by the size of condition.condition_values.

because inPredicate with one element to stored as BinaryPredicate which is more more light and efficient.

morningman

LGTM

gensrc/proto/olap_file.proto

kangkaisen

protobuf LGTM

morningman

LGTM

‌When the IN predicate in an expression contains parameters generated through CAST operations, utilizing inverted indexes may produce erroneous query results.

morningman reviewed Jul 5, 2020

View reviewed changes

caiconghui force-pushed the delete_in branch from ec82218 to 1057777 Compare July 6, 2020 15:52

wutiangan reviewed Jul 7, 2020

View reviewed changes

fe/src/main/java/org/apache/doris/load/DeleteHandler.java Outdated Show resolved Hide resolved

wutiangan reviewed Jul 7, 2020

View reviewed changes

fe/src/main/java/org/apache/doris/load/DeleteHandler.java Show resolved Hide resolved

wutiangan reviewed Jul 7, 2020

View reviewed changes

fe/src/main/java/org/apache/doris/load/DeleteHandler.java Show resolved Hide resolved

caiconghui force-pushed the delete_in branch from 1e44c70 to ae4c78f Compare July 28, 2020 07:35

morningman reviewed Jul 29, 2020

View reviewed changes

morningman self-assigned this Jul 29, 2020

morningman added area/delete Issues or PRs related to DELETE operation kind/feature Categorizes issue or PR as related to a new feature. labels Jul 29, 2020

caiconghui force-pushed the delete_in branch from d0b8a9d to c28df3a Compare July 30, 2020 02:44

caiconghui [蔡聪辉] and others added 17 commits July 30, 2020 13:04

support in predicate in delete

28085ae

support in predicate in delete stmt

2b73213

Support in and not in predicate in be delete handler

e1c81a9

Support delete with in predicate

4cfbde5

remove some unused function and log

dd90e85

Add max_allowed_in_element_num_of_delete variable

1d98a7f

Add doc content for max_allowed_in_element_num_of_delete variable

e8739b6

Improve performance of in predicate

a7387ab

Fix bug for in predicate

ad93769

support fast delete filter with in predicate

b9a1e0d

fix

8773704

fix deletehandler unit test

27108af

add unit test for delete_handler

7227371

fix by review

e049558

remove unused content from docs

cafcb41

fix in predicate str error

1e7e2a9

fix by review

4d1c423

caiconghui added 2 commits July 30, 2020 13:07

Add doc content about delete

ad3dc45

small fix

7fc8ae4

caiconghui force-pushed the delete_in branch from c28df3a to 7fc8ae4 Compare July 30, 2020 05:27

morningman previously approved these changes Aug 1, 2020

View reviewed changes

fix unit test failed

2777547

caiconghui dismissed morningman’s stale review via 2777547 August 1, 2020 10:49

morningman reviewed Aug 1, 2020

View reviewed changes

morningman previously approved these changes Aug 1, 2020

View reviewed changes

morningman added approved Indicates a PR has been approved by one committer. branch-0.13 PR which need to merge to branch 0.13 labels Aug 1, 2020

kangkaisen reviewed Aug 3, 2020

View reviewed changes

gensrc/proto/olap_file.proto Show resolved Hide resolved

Change all required to optional for InPredicatePB

1858a94

caiconghui dismissed morningman’s stale review via 1858a94 August 3, 2020 07:20

kangkaisen previously approved these changes Aug 3, 2020

View reviewed changes

fix be ut failed

d090f67

caiconghui dismissed kangkaisen’s stale review via d090f67 August 3, 2020 11:19

morningman approved these changes Aug 4, 2020

View reviewed changes

morningman merged commit eefad13 into apache:master Aug 6, 2020

EmmyMiao87 mentioned this pull request Aug 17, 2020

Release Notes 0.13.0 #4370

Closed

[Feature] Support InPredicate in delete statement #4006

[Feature] Support InPredicate in delete statement #4006

Uh oh!

Conversation

caiconghui commented Jul 2, 2020

Uh oh!

caiconghui commented Jul 2, 2020

Uh oh!

morningman commented Jul 2, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

morningman Jul 29, 2020

Choose a reason for hiding this comment

Uh oh!

caiconghui Jul 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

morningman Jul 29, 2020

Choose a reason for hiding this comment

Uh oh!

caiconghui Jul 29, 2020

Choose a reason for hiding this comment

Uh oh!

morningman Jul 29, 2020

Choose a reason for hiding this comment

Uh oh!

caiconghui Jul 29, 2020

Choose a reason for hiding this comment

Uh oh!

morningman left a comment

Choose a reason for hiding this comment

Uh oh!

morningman commented Aug 1, 2020

Uh oh!

Uh oh!

morningman Aug 1, 2020

Choose a reason for hiding this comment

Uh oh!

caiconghui Aug 1, 2020

Choose a reason for hiding this comment

Uh oh!

morningman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kangkaisen left a comment

Choose a reason for hiding this comment

Uh oh!

morningman left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

caiconghui Jul 29, 2020 •

edited

Loading