Try converting all inner joins to filters (#13201)

This commit is contained in:
Rohan Garg 2022-11-07 23:19:18 +05:30 committed by GitHub
parent a738ac9ad7
commit a9b39fc29d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 68 additions and 27 deletions

View File

@ -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);
}
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 {
// 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);
// add back the required columns by this join since it wasn't converted fully
for (String column : clause.getCondition().getRequiredColumns()) {
columnsRequiredByJoinClauses.add(column, 1);
}
}
}

View File

@ -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
);
}
}