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

Change Null-safe equal operator from cross join to hash join #2156

Merged
merged 7 commits into from Nov 8, 2019

Conversation

EmmyMiao87
Copy link
Contributor

ISSUE-2136

This commit change the join method from cross join to hash join when the equal operator is Null-safe '<=>'.
It will improve the speed of query which has the Null-safe equal operator.
The finds_nulls field is used to save if there is Null-safe operator.
The finds_nulls[i] is true means that the i-th equal operator is Null-safe.
The equal function in hash table will return true, if both val and loc are NULL when finds_nulls[i] is true.

@EmmyMiao87
Copy link
Contributor Author

#2155

@EmmyMiao87
Copy link
Contributor Author

#2136

@EmmyMiao87 EmmyMiao87 mentioned this pull request Nov 7, 2019
be/src/exec/hash_join_node.cpp Show resolved Hide resolved
@@ -42,12 +42,18 @@ const char* HashTable::_s_llvm_class_name = "class.doris::HashTable";

HashTable::HashTable(const vector<ExprContext*>& build_expr_ctxs,
const vector<ExprContext*>& probe_expr_ctxs,
int num_build_tuples, bool stores_nulls, int32_t initial_seed,
int num_build_tuples, bool null_preserved,
const std::vector<bool> finds_nulls,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const std::vector<bool> finds_nulls,
const std::vector<bool>& finds_nulls,

@@ -98,7 +98,9 @@ class HashTable {
HashTable(
const std::vector<ExprContext*>& build_exprs,
const std::vector<ExprContext*>& probe_exprs,
int num_build_tuples, bool stores_nulls, int32_t initial_seed,
int num_build_tuples, bool stores_nulls,
const std::vector<bool> finds_nulls,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const std::vector<bool> finds_nulls,
const std::vector<bool>& finds_nulls,

@@ -194,8 +194,10 @@ Status OlapScanNode::open(RuntimeState* state) {
RETURN_IF_CANCELLED(state);
RETURN_IF_ERROR(ExecNode::open(state));

LOG(INFO) << "olap scan node conjunct in open " << Expr::debug_string(_conjunct_ctxs);
Copy link
Contributor

Choose a reason for hiding this comment

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

should be VLOG

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove it.

@@ -53,8 +54,8 @@

private final TableRef innerRef;
private final JoinOperator joinOp;
// conjuncts of the form "<lhs> = <rhs>", recorded as Pair(<lhs>, <rhs>)
private List<Pair<Expr, Expr>> eqJoinConjuncts;
// predicates of the form "<lhs> = <rhs>", recorded as Pair(<lhs>, <rhs>)
Copy link
Contributor

Choose a reason for hiding this comment

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

why you change conjuncts to predicates?
Better to keep it conjuncts, becuase it means and and same with otherJoinConjuncts

// the row in hash table is preserved such as RIGHT_OUTER_JOIN
const bool _null_preserved;
// true: the null value should be keep in
// false: the null value should be filter
Copy link
Contributor

Choose a reason for hiding this comment

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

what keep in means? and what filter means?

@@ -384,13 +384,13 @@ private PlanFragment createHashJoinFragment(HashJoinNode node, PlanFragment righ
// TODO: create equivalence classes based on equality predicates

// first, extract join exprs
List<Pair<Expr, Expr>> eqJoinConjuncts = node.getEqJoinConjuncts();
List<BinaryPredicate> eqJoinPredicates = node.getEqJoinConjuncts();
Copy link
Contributor

Choose a reason for hiding this comment

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

name?

morningman
morningman previously approved these changes Nov 7, 2019
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

ISSUE-2136

This commit change the join method from cross join to hash join when the equal operator is Null-safe '<=>'.
It will improve the speed of query which has the Null-safe equal operator.
The finds_nulls field is used to save if there is Null-safe operator.
The finds_nulls[i] is true means that the i-th equal operator is Null-safe.
The equal function in hash table will return true, if both val and loc are NULL when finds_nulls[i] is true.
@EmmyMiao87 EmmyMiao87 added the need more test Add more test label Nov 8, 2019
@EmmyMiao87 EmmyMiao87 added this to the 0.12.0 milestone Nov 8, 2019
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

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

@morningman morningman merged commit 42395d2 into apache:master Nov 8, 2019
morningman pushed a commit to morningman/doris that referenced this pull request Nov 8, 2019
…2156)

* Change Null-safe equal operator from cross join to hash join
ISSUE-2136

This commit change the join method from cross join to hash join when the equal operator is Null-safe '<=>'.
It will improve the speed of query which has the Null-safe equal operator.
The finds_nulls field is used to save if there is Null-safe operator.
The finds_nulls[i] is true means that the i-th equal operator is Null-safe.
The equal function in hash table will return true, if both val and loc are NULL when finds_nulls[i] is true.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need more test Add more test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants