Skip to content
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

executor: make IndexHashJoin support keeping the outer order #12349

Merged
merged 26 commits into from Oct 17, 2019

Conversation

XuHuaiyu
Copy link
Contributor

@XuHuaiyu XuHuaiyu commented Sep 24, 2019

NOTE:
For testing the correctness, I change the execution logic that IndexJoin and IndexMergeJoin both execute IndexHashJoin actually.

What problem does this PR solve?

Make the IndexHashJoin support keeping the result as the outer order.

What is changed and how it works?

Add a keepOuterOrder attribute for IndexHashJoin, outerWorker, and IndexHashJoinTask, when keepOuterOrder is true, a task will be sent to the channels maintained in the main thread and inner workers separately. And the result will be fetched from task.resultCh one by one.

Check List

Tests

Exists tests.

Code changes

  • Has exported function/method change
  • Has exported variable/fields change

Side effects

N/A

Related changes

N/A

Release note

N/A

@XuHuaiyu
Copy link
Contributor Author

/rebuild

@codecov
Copy link

codecov bot commented Sep 24, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@abadb0b). Click here to learn what that means.
The diff coverage is 66.6666%.

@@             Coverage Diff             @@
##             master     #12349   +/-   ##
===========================================
  Coverage          ?   80.0544%           
===========================================
  Files             ?        462           
  Lines             ?     105753           
  Branches          ?          0           
===========================================
  Hits              ?      84660           
  Misses            ?      14780           
  Partials          ?       6313

@XuHuaiyu
Copy link
Contributor Author

/rebuild

@XuHuaiyu
Copy link
Contributor Author

/run-all-tests

@XuHuaiyu
Copy link
Contributor Author

XuHuaiyu commented Oct 8, 2019

PTAL @SunRunAway @qw4990

1 similar comment
@XuHuaiyu
Copy link
Contributor Author

XuHuaiyu commented Oct 9, 2019

PTAL @SunRunAway @qw4990

@XuHuaiyu
Copy link
Contributor Author

/run-all-tests

@@ -482,3 +614,77 @@ func (iw *indexHashJoinInnerWorker) joinMatchedInnerRow2Chunk(ctx context.Contex
}
return true, joinResult
}

func (iw *indexHashJoinInnerWorker) collectMatchedInnerPtrs4OuterRows(ctx context.Context, innerRow chunk.Row, innerRowPtr chunk.RowPtr, task *indexHashJoinTask,
joinResult *indexHashJoinResult, h hash.Hash64, buf []byte) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

joinResult is not used in this function.

Copy link
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

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

LGTM

@qw4990 qw4990 added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 17, 2019
@XuHuaiyu
Copy link
Contributor Author

/run-all-tests

Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@SunRunAway SunRunAway added status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 17, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Oct 17, 2019

Your auto merge job has been accepted, waiting for 12773

@sre-bot
Copy link
Contributor

sre-bot commented Oct 17, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Oct 17, 2019

@XuHuaiyu merge failed.

@XuHuaiyu
Copy link
Contributor Author

/run-integration-common-test
/run-unit-test

@XuHuaiyu XuHuaiyu merged commit e1ba309 into pingcap:master Oct 17, 2019
@XuHuaiyu XuHuaiyu deleted the indexhashjoin_order branch October 17, 2019 06:48
XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants