DRILL-4232: Support for EXCEPT and INTERSECT set operator #2599

Leon-WTF · 2022-07-17T12:01:35Z

DRILL-4232: Support for EXCEPT and INTERSECT set operator

Description

Can have hash set operator and sorted set operator, only implement hash version in this PR.
Compute number of left-input duplicates(numLeft, probe side) and number of right-input duplicates(numRight, build side) for each same tuple:
INTERSECT: if numRight > 0 and numLeft > 0, output one tuple
INTERSECT ALL: if numRight > 0 and numLeft > 0, output min(numLeft,numRight) tuples
EXCEPT: if numRight = 0 and numLeft > 0, output one tuple
EXCEPT ALL: if numLeft>=numRight, output numLeft - numRight tuples

Documentation

TODO

Testing

See TestSetOp

cgivre · 2022-07-20T15:14:48Z

@Leon-WTF You might want to wait until #2602 is merged before you continue this.

Leon-WTF · 2022-07-24T07:54:51Z

@Leon-WTF You might want to wait until #2602 is merged before you continue this.

@cgivre Thanks for the info, #2602 is very needed, I will focus on implementing the operator firstly, and adapt to the new Calcite when it's merged.

cgivre · 2022-08-28T01:28:12Z

@Leon-WTF Is this ready for review?

Leon-WTF · 2022-08-28T03:18:31Z

@Leon-WTF Is this ready for review?

@cgivre Not yet, I'm handling the EXCEPT case, it needs to remove the duplicate records for probe side(for other three cases, it's like semi-join, I just added the records num to the hash map), I'm trying to add an Agg phase after setop phase. Any suggestion on this?

cgivre · 2022-09-22T19:29:49Z

Hi @Leon-WTF how is this coming?

cgivre · 2022-10-02T03:03:33Z

@Leon-WTF I don't know if you saw this or not, but #2602 has been merged. We are actually running on Calcite 1.32 now!

Leon-WTF · 2022-10-16T08:34:58Z

@Leon-WTF I don't know if you saw this or not, but #2602 has been merged. We are actually running on Calcite 1.32 now!

@cgivre Sorry for the late response. Yes, I have merged that in. I'm almost done and I'm adding ut.

cgivre · 2022-10-16T16:24:03Z

HI @Leon-WTF
I saw that you added unit tests. Is this PR ready for review? If not, what is remaining?

Leon-WTF · 2022-10-17T02:30:21Z

HI @Leon-WTF I saw that you added unit tests. Is this PR ready for review? If not, what is remaining?

@cgivre I think it's just still need more ut to test more completely and fix bugs while adding ut. By the way, do we have any plan to release drill 2.0? I will try to catch that plan.

cgivre · 2022-10-24T14:54:14Z

HI @Leon-WTF I saw that you added unit tests. Is this PR ready for review? If not, what is remaining?

@cgivre I think it's just still need more ut to test more completely and fix bugs while adding ut. By the way, do we have any plan to release drill 2.0? I will try to catch that plan.

We are getting ready to release Drill 1.20.3. Once that's done, I'd like to start discussions around Drill 2.0. There have been a lot of major work in Drill and I'd like to see that getting used.

vvysotskyi

Hi @Leon-WTF, thanks for adding this functionality! I have added several code review comments to address before we can merge it.

exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java

exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java

exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillSetOpRel.java

exec/java-exec/src/test/java/org/apache/drill/TestSetOp.java

exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillSetOpRel.java

exec/java-exec/src/test/java/org/apache/drill/TestSetOp.java

...ava-exec/src/main/java/org/apache/drill/exec/physical/impl/setop/HashSetOpProbeTemplate.java

Leon-WTF · 2023-02-03T09:58:26Z

@vvysotskyi Hi, any more comments on this PR?

vvysotskyi

Thanks for refactoring and addressing code review comments. I have found several places to improve, and it will be ready to go.

.../java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillAddAggForExceptRule.java

vvysotskyi · 2023-02-04T20:27:28Z

.../java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillAddAggForExceptRule.java

+        ImmutableBitSet.range(0, drillExceptRel.getInput(0).getRowType().getFieldList().size()), ImmutableList.of(), ImmutableList.of());
+      call.transformTo(drillExceptRel.copy(ImmutableList.of(aggNode, drillExceptRel.getInput(1)), true));
+    } else {
+      call.transformTo(new DrillAggregateRel(drillExceptRel.getCluster(), drillExceptRel.getTraitSet(), drillExceptRel.copy(true),


Do we need to add aggregate on top of except?

@vvysotskyi It's for performance, if the data cardinality is high, aggregate before except may not reduce many data, if the data after except left are few, aggregate after except will only handle few data which is faster than before except. This may be choosen by statistics info + CBO automaticlly in the future.

But output values should be already distinct after the execution of except operator, so the aggregate will do nothing.

It's not distinct for left table after except operator. I choosed to reuse an aggregate operator to do the distinct.

Oh, ok, if it is specific to our implementation of except operator, aggregation added here possibly could be removed by other Calcite rules which assume that results would be already distinct.

I think it would be better to add an aggregation when converting it to physical rel nodes.

oh, yes, I will refactor that.

@vvysotskyi One more question about moving converting rule to physical phase, I need to add physical agg rel node, so needs to add both hash(distribute by all keys/single key) and stream agg, right?

Drill doesn't use streaming aggregate for distinct calls, so only hash agg should be enough.

Drill doesn't use streaming aggregate for distinct calls, so only hash agg should be enough.

@vvysotskyi I see it checks aggregate.containsDistinctCall() in StreamAggPrule, but It will generate steam agg for sql like "select a,b,c from foo group by a,b,c".

exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillSetOpRule.java

logical/src/main/java/org/apache/drill/common/logical/data/visitors/LogicalVisitor.java

exec/java-exec/src/test/java/org/apache/drill/TestSetOp.java

cgivre · 2023-02-09T12:56:59Z

Hey @Leon-WTF Any chance you could address @vvysotskyi 's comments soon. This is one of the last PRs slated to be merged for the next release.

Leon-WTF · 2023-02-10T05:46:06Z

Hey @Leon-WTF Any chance you could address @vvysotskyi 's comments soon. This is one of the last PRs slated to be merged for the next release.

I will do it by this weekend, is that ok?

cgivre · 2023-02-10T12:43:59Z

@Leon-WTF This weekend would be great! Very excited to get this merged.

vvysotskyi

Looks good, +1

cgivre · 2023-02-12T20:04:09Z

@Leon-WTF Thanks for this. @vvysotskyi Thanks for the review. Merging now!

Leon-WTF marked this pull request as draft July 17, 2022 12:01

Leon-WTF requested review from luocooong, vvysotskyi and paul-rogers July 17, 2022 12:02

Leon-WTF requested a review from cgivre July 24, 2022 07:55

Leon-WTF force-pushed the set_operation branch 2 times, most recently from 3a94b32 to 4966db9 Compare August 27, 2022 09:18

Leon-WTF force-pushed the set_operation branch from 7b8d435 to c3b1fa5 Compare September 4, 2022 13:36

cgivre added enhancement PRs that add a new functionality to Drill doc-impacting PRs that affect the documentation labels Oct 2, 2022

cgivre assigned Leon-WTF Oct 2, 2022

Leon-WTF force-pushed the set_operation branch from eb54353 to c95c854 Compare October 16, 2022 10:37

Leon-WTF force-pushed the set_operation branch from c95c854 to b5fc5b9 Compare October 23, 2022 13:26

Leon-WTF force-pushed the set_operation branch from cc8a921 to 89199f7 Compare November 27, 2022 13:38

Leon-WTF force-pushed the set_operation branch from 370d5e1 to 7d02615 Compare December 11, 2022 10:00

Leon-WTF marked this pull request as ready for review December 17, 2022 02:51

vvysotskyi requested changes Dec 26, 2022

View reviewed changes

Leon-WTF pushed a commit to Leon-WTF/drill that referenced this pull request Jan 15, 2023

DRILL-4232: Support for EXCEPT and INTERSECT set operator (apache#2599)

45a44d7

Leon-WTF force-pushed the set_operation branch from db66964 to 45a44d7 Compare January 15, 2023 07:31

Leon-WTF changed the title ~~DRILL-4232 Support for EXCEPT and INTERSECT set operator~~ DRILL-4232: Support for EXCEPT and INTERSECT set operator Jan 15, 2023

vvysotskyi reviewed Jan 22, 2023

View reviewed changes

Leon-WTF force-pushed the set_operation branch 4 times, most recently from 49a9307 to 7317fad Compare January 29, 2023 04:21

vvysotskyi reviewed Feb 4, 2023

View reviewed changes

feiteng.wtf added 4 commits February 12, 2023 16:02

DRILL-4232: Support for EXCEPT and INTERSECT set operator (apache#2599)

cdd3c84

Refactor drill set op logical rel and SetOpPrule

193da1f

Refactor set op record batch and probe template

bdb1a90

Move DrillAddAggForExceptRule into SetOpPrule

d25c3e4

Leon-WTF force-pushed the set_operation branch from 3923425 to d25c3e4 Compare February 12, 2023 08:04

fix ut

ce2906d

vvysotskyi approved these changes Feb 12, 2023

View reviewed changes

cgivre merged commit c38091f into apache:master Feb 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRILL-4232: Support for EXCEPT and INTERSECT set operator #2599

DRILL-4232: Support for EXCEPT and INTERSECT set operator #2599

Leon-WTF commented Jul 17, 2022 •

edited

cgivre commented Jul 20, 2022

Leon-WTF commented Jul 24, 2022 •

edited

cgivre commented Aug 28, 2022

Leon-WTF commented Aug 28, 2022 •

edited

cgivre commented Sep 22, 2022

cgivre commented Oct 2, 2022

Leon-WTF commented Oct 16, 2022

cgivre commented Oct 16, 2022

Leon-WTF commented Oct 17, 2022

cgivre commented Oct 24, 2022

vvysotskyi left a comment

Leon-WTF commented Feb 3, 2023

vvysotskyi left a comment

vvysotskyi Feb 4, 2023

Leon-WTF Feb 6, 2023 •

edited

vvysotskyi Feb 6, 2023

Leon-WTF Feb 6, 2023 •

edited

vvysotskyi Feb 6, 2023

Leon-WTF Feb 6, 2023

Leon-WTF Feb 10, 2023 •

edited

vvysotskyi Feb 11, 2023

Leon-WTF Feb 12, 2023 •

edited

cgivre commented Feb 9, 2023

Leon-WTF commented Feb 10, 2023

cgivre commented Feb 10, 2023

vvysotskyi left a comment

cgivre commented Feb 12, 2023

DRILL-4232: Support for EXCEPT and INTERSECT set operator #2599

DRILL-4232: Support for EXCEPT and INTERSECT set operator #2599

Conversation

Leon-WTF commented Jul 17, 2022 • edited

DRILL-4232: Support for EXCEPT and INTERSECT set operator

Description

Documentation

Testing

cgivre commented Jul 20, 2022

Leon-WTF commented Jul 24, 2022 • edited

cgivre commented Aug 28, 2022

Leon-WTF commented Aug 28, 2022 • edited

cgivre commented Sep 22, 2022

cgivre commented Oct 2, 2022

Leon-WTF commented Oct 16, 2022

cgivre commented Oct 16, 2022

Leon-WTF commented Oct 17, 2022

cgivre commented Oct 24, 2022

vvysotskyi left a comment

Choose a reason for hiding this comment

Leon-WTF commented Feb 3, 2023

vvysotskyi left a comment

Choose a reason for hiding this comment

vvysotskyi Feb 4, 2023

Choose a reason for hiding this comment

Leon-WTF Feb 6, 2023 • edited

Choose a reason for hiding this comment

vvysotskyi Feb 6, 2023

Choose a reason for hiding this comment

Leon-WTF Feb 6, 2023 • edited

Choose a reason for hiding this comment

vvysotskyi Feb 6, 2023

Choose a reason for hiding this comment

Leon-WTF Feb 6, 2023

Choose a reason for hiding this comment

Leon-WTF Feb 10, 2023 • edited

Choose a reason for hiding this comment

vvysotskyi Feb 11, 2023

Choose a reason for hiding this comment

Leon-WTF Feb 12, 2023 • edited

Choose a reason for hiding this comment

cgivre commented Feb 9, 2023

Leon-WTF commented Feb 10, 2023

cgivre commented Feb 10, 2023

vvysotskyi left a comment

Choose a reason for hiding this comment

cgivre commented Feb 12, 2023

Leon-WTF commented Jul 17, 2022 •

edited

Leon-WTF commented Jul 24, 2022 •

edited

Leon-WTF commented Aug 28, 2022 •

edited

Leon-WTF Feb 6, 2023 •

edited

Leon-WTF Feb 6, 2023 •

edited

Leon-WTF Feb 10, 2023 •

edited

Leon-WTF Feb 12, 2023 •

edited