This is an automated email from the ASF dual-hosted git repository.
rohangarg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new a9b39fc29d Try converting all inner joins to filters (#13201)
a9b39fc29d is described below
commit a9b39fc29ddb34b3ac32bc9198b52ba86d8de2b8
Author: Rohan Garg <[email protected]>
AuthorDate: Mon Nov 7 23:19:18 2022 +0530
Try converting all inner joins to filters (#13201)
---
.../druid/segment/join/JoinableFactoryWrapper.java | 56 ++++++++++++----------
.../segment/join/JoinableFactoryWrapperTest.java | 37 +++++++++++++-
2 files changed, 67 insertions(+), 26 deletions(-)
diff --git
a/processing/src/main/java/org/apache/druid/segment/join/JoinableFactoryWrapper.java
b/processing/src/main/java/org/apache/druid/segment/join/JoinableFactoryWrapper.java
index 020d23317f..134c9da48d 100644
---
a/processing/src/main/java/org/apache/druid/segment/join/JoinableFactoryWrapper.java
+++
b/processing/src/main/java/org/apache/druid/segment/join/JoinableFactoryWrapper.java
@@ -79,34 +79,40 @@ public class JoinableFactoryWrapper
}
Set<String> rightPrefixes =
clauses.stream().map(JoinableClause::getPrefix).collect(Collectors.toSet());
- // Walk through the list of clauses, picking off any from the start of the
list that can be converted to filters.
- boolean atStart = true;
+ boolean isRightyJoinSeen = false;
for (JoinableClause clause : clauses) {
- if (atStart) {
- // Remove this clause from columnsRequiredByJoinClauses. It's ok if it
relies on itself.
- for (String column : clause.getCondition().getRequiredColumns()) {
- columnsRequiredByJoinClauses.remove(column, 1);
- }
+ // Incase we find a RIGHT/OUTER join, we shouldn't convert join
conditions to left column filters for any join
+ // afterwards because the values of left colmun might change to NULL
after the RIGHT/OUTER join. We should only
+ // consider cases where the values of the filter columns do not change
after the join.
+ isRightyJoinSeen = isRightyJoinSeen || clause.getJoinType().isRighty();
+ if (isRightyJoinSeen) {
+ clausesToUse.add(clause);
+ continue;
+ }
+ // Remove this clause from columnsRequiredByJoinClauses. It's ok if it
relies on itself.
+ for (String column : clause.getCondition().getRequiredColumns()) {
+ columnsRequiredByJoinClauses.remove(column, 1);
+ }
- final JoinClauseToFilterConversion joinClauseToFilterConversion =
- convertJoinToFilter(
- clause,
- Sets.union(requiredColumns,
columnsRequiredByJoinClauses.elementSet()),
- maxNumFilterValues,
- rightPrefixes
- );
-
- // add the converted filter to the filter list
- if (joinClauseToFilterConversion.getConvertedFilter() != null) {
- filterList.add(joinClauseToFilterConversion.getConvertedFilter());
- }
- // if the converted filter is partial, keep the join clause too
- if (!joinClauseToFilterConversion.isJoinClauseFullyConverted()) {
- clausesToUse.add(clause);
- atStart = false;
- }
- } else {
+ final JoinClauseToFilterConversion joinClauseToFilterConversion =
+ convertJoinToFilter(
+ clause,
+ Sets.union(requiredColumns,
columnsRequiredByJoinClauses.elementSet()),
+ maxNumFilterValues,
+ rightPrefixes
+ );
+
+ // add the converted filter to the filter list
+ if (joinClauseToFilterConversion.getConvertedFilter() != null) {
+ filterList.add(joinClauseToFilterConversion.getConvertedFilter());
+ }
+ // if the converted filter is partial, keep the join clause too
+ if (!joinClauseToFilterConversion.isJoinClauseFullyConverted()) {
clausesToUse.add(clause);
+ // add back the required columns by this join since it wasn't
converted fully
+ for (String column : clause.getCondition().getRequiredColumns()) {
+ columnsRequiredByJoinClauses.add(column, 1);
+ }
}
}
diff --git
a/processing/src/test/java/org/apache/druid/segment/join/JoinableFactoryWrapperTest.java
b/processing/src/test/java/org/apache/druid/segment/join/JoinableFactoryWrapperTest.java
index 3d8a01785a..1ff3664aba 100644
---
a/processing/src/test/java/org/apache/druid/segment/join/JoinableFactoryWrapperTest.java
+++
b/processing/src/test/java/org/apache/druid/segment/join/JoinableFactoryWrapperTest.java
@@ -506,7 +506,7 @@ public class JoinableFactoryWrapperTest extends
NullHandlingTest
Assert.assertEquals(
Pair.of(
- ImmutableList.of(),
+ ImmutableList.of(new InDimFilter("x", TEST_LOOKUP_KEYS)),
clauses
),
conversion
@@ -590,4 +590,39 @@ public class JoinableFactoryWrapperTest extends
NullHandlingTest
conversion
);
}
+
+ @Test
+ public void
test_convertJoinsToFilters_dontConvertJoinsDependedOnByPreviousJoins()
+ {
+ // upon discovering a RIGHT/OUTER join, all conversions for subsequent
joins should stop since the output of left
+ // table columns might change to NULL after the RIGHT/OUTER join.
+ final ImmutableList<JoinableClause> clauses = ImmutableList.of(
+ new JoinableClause(
+ "j.",
+ LookupJoinable.wrap(new MapLookupExtractor(TEST_LOOKUP, false)),
+ JoinType.RIGHT,
+ JoinConditionAnalysis.forExpression("x == \"j.k\"", "j.",
ExprMacroTable.nil())
+ ),
+ new JoinableClause(
+ "_j.",
+ LookupJoinable.wrap(new MapLookupExtractor(TEST_LOOKUP, false)),
+ JoinType.INNER,
+ JoinConditionAnalysis.forExpression("x == \"_j.k\"", "_j.",
ExprMacroTable.nil())
+ )
+ );
+
+ final Pair<List<Filter>, List<JoinableClause>> conversion =
JoinableFactoryWrapper.convertJoinsToFilters(
+ clauses,
+ ImmutableSet.of("x"),
+ Integer.MAX_VALUE
+ );
+
+ Assert.assertEquals(
+ Pair.of(
+ ImmutableList.of(),
+ clauses
+ ),
+ conversion
+ );
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
