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

Try converting all inner joins to filters #13201

Merged
merged 3 commits into from
Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -79,34 +79,40 @@ public static Pair<List<Filter>, List<JoinableClause>> convertJoinsToFilters(
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ public void test_convertJoinsToFilters_dontConvertJoinsDependedOnByLaterJoins()

Assert.assertEquals(
Pair.of(
ImmutableList.of(),
ImmutableList.of(new InDimFilter("x", TEST_LOOKUP_KEYS)),
clauses
),
conversion
Expand Down Expand Up @@ -590,4 +590,39 @@ public void test_convertJoinsToFilters_partialConvertJoinsDependedOnByLaterJoins
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
);
}
}