SQL: Plan non-equijoin conditions as cross join followed by filter #15302

abhishekagarwal87 · 2023-11-01T11:28:18Z

This PR revives #14978 with a few more bells and whistles. Instead of an unconditional cross-join, we will now split the join condition such that some conditions are now evaluated post-join. To decide what sub-condition goes where, I have refactored DruidJoinRule class to extract unsupported sub-conditions. We build a postJoinFilter out of these unsupported sub-conditions and push to the join.

Release notes
Druid can support arbitrary join conditions for INNER join. For INNER joins, druid will look at the join condition, and any sub-conditions that cannot be evaluated efficiently as part of the join, will be converted to a post-join filter. With this feature, you can do in-equality joins that were not possible before.

This PR has:

sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java

+                                .build()
+                        ),
+                        "j0.",
+                        equalsCondition(makeColumnExpression("m1"), makeColumnExpression("j0.m1")),


sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java

+                                .build()
+                        ),
+                        "j0.",
+                        equalsCondition(makeColumnExpression("m1"), makeColumnExpression("j0.m1")),


sql/src/main/java/org/apache/druid/sql/calcite/rule/JoinRules.java

soumyava · 2023-11-14T16:32:30Z

extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQSelectTest.java

@@ -772,7 +772,7 @@ public void testJoinWithLookup()
                                               DruidExpression.ofColumn(ColumnType.STRING, "dim2"),
                                               DruidExpression.ofColumn(ColumnType.STRING, "j0.k")
                                           ),
-                                           JoinType.LEFT
+                                           JoinType.INNER


The initial query has a left join here and we end up planning it as in inner. Can there be a case where the user does not expect the results which have a pair on both left and right ? Also that seems to be case where the result changed for the not sql compatible mode

soumyava · 2023-11-14T18:48:43Z

sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java

@@ -723,7 +723,7 @@ public void testFilterAndGroupByLookupUsingJoinOperatorBackwards(Map<String, Obj
                                ),
                                "j0.",
                                equalsCondition(makeColumnExpression("k"), makeColumnExpression("j0.dim2")),
-                                JoinType.RIGHT
+                                JoinType.INNER


Same comment as before, we are converting a right join to an inner join here and that is changing the expected results

soumyava · 2023-11-14T18:49:08Z

sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java

@@ -768,7 +762,7 @@ public void testFilterAndGroupByLookupUsingJoinOperatorWithNotFilter(Map<String,
                                new LookupDataSource("lookyloo"),
                                "j0.",
                                equalsCondition(makeColumnExpression("dim2"), makeColumnExpression("j0.k")),
-                                JoinType.LEFT
+                                JoinType.INNER


Same as before

There is a post-join filter that will reject nulls on the right side. Hence the SQL query is equivalent to INNER join with post-join filter moved into the join condition itself. That's also the same reason why some other join queries have become INNER join queries. I have also now tweaked the rule a bit so that we still reject non-equi conditions for non-sql compatible mode. With that, results in non-sql compatible mode won't change.

soumyava · 2023-11-14T18:49:40Z

sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java

@@ -821,7 +809,7 @@ public void testJoinUnionTablesOnLookup(Map<String, Object> queryContext)
                                new LookupDataSource("lookyloo"),
                                "j0.",
                                equalsCondition(makeColumnExpression("dim2"), makeColumnExpression("j0.k")),
-                                JoinType.LEFT
+                                JoinType.INNER


Similar comment as the previous one

sql/src/test/java/org/apache/druid/sql/calcite/DecoupledPlanningCalciteQueryTest.java

sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java

-                          "1",
+                          NullHandling.sqlCompatible() ?
+                          equalsCondition(
+                              DruidExpression.fromExpression("CAST(\"j0.k\", 'LONG')"),


This reverts commit 2b5026e.

abhishekagarwal87 · 2023-11-26T15:57:32Z

sql/src/test/java/org/apache/druid/sql/calcite/CalciteJoinQueryTest.java

@@ -740,7 +740,6 @@ public void testFilterAndGroupByLookupUsingJoinOperatorBackwards(Map<String, Obj
            new Object[]{"xabc", 1L}
        )
        : ImmutableList.of(
-            new Object[]{null, 5L},


This result changed because there is a bug in splitting the join filter on leaf nodes. A filter that can reject null cannot be pushed into the left table if the join is right. But that's what https://github.com/apache/druid/blob/master/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterAnalyzer.java#L198 will do right now. I will fix this in another PR.

clintropolis

👍

docs/querying/datasource.md

sql/src/test/java/org/apache/druid/sql/calcite/rule/DruidJoinRuleTest.java

sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidJoinRule.java

clintropolis · 2023-11-28T10:05:56Z

sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidJoinRule.java

+      // I am not sure in what case, the list of system fields will be not empty. I have just picked up this logic
+      // directly from https://github.com/apache/calcite/blob/calcite-1.35.0/core/src/main/java/org/apache/calcite/rel/rules/AbstractJoinExtractFilterRule.java#L58


what is systemFieldList? i dug around a bit and isnt super obvious to me 🙃

* @param systemFieldList List of system fields that will be prefixed to * output row type; typically empty but must not be * null

or Returns a list of system fields that will be prefixed to output row type. are all that i can find but i don't quite fully understand what it means

I am not sure either. I had asked @kgyrtkirk, and he didn't have any answer either. Maybe @gianm knows since he was exchanging emails with calcite community recently over other kind of system fields (like file names in input format).
In any case, I don't think this list will ever be non-empty based on its usages I found in calcite.

abhishekagarwal87 · 2023-11-29T08:16:23Z

Thank you @clintropolis and @soumyava for review.

…pache#15302) This PR revives apache#14978 with a few more bells and whistles. Instead of an unconditional cross-join, we will now split the join condition such that some conditions are now evaluated post-join. To decide what sub-condition goes where, I have refactored DruidJoinRule class to extract unsupported sub-conditions. We build a postJoinFilter out of these unsupported sub-conditions and push to the join.

…lter rule (#15511) FILTER_INTO_JOIN is mainly run along with the other rules with the Volcano planner; however if the query starts highly underdefined (join conditions in the where clauses) that generic query could give a lot of room for the other rules to play around with only enabled it for when the join uses subqueries for its inputs. PROJECT_FILTER rule is not that useful. and could increase planning times by providing new plans. This problem worsened after we started supporting inner joins with arbitrary join conditions in #15302

gianm and others added 2 commits November 1, 2023 16:48

Inequality join

f26dc2e

Merge with master

44ff897

github-actions bot added Area - Documentation Area - Batch Ingestion Area - Querying Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels Nov 1, 2023

github-advanced-security bot found potential problems Nov 1, 2023

View reviewed changes

abhishekagarwal87 added 6 commits November 2, 2023 17:07

fix static rules

808b608

Merge branch 'master' of github.com:apache/druid into inequality_join

f23915f

Merge branch 'master' of github.com:apache/druid into inequality_join

4bd1801

Pull condition handling into DruidJoinRule

d9ca64f

some changes

1aa076f

Some more fixes

259ef9b

abhishekagarwal87 marked this pull request as ready for review November 8, 2023 09:40

abhishekagarwal87 added the Release Notes label Nov 8, 2023

abhishekagarwal87 added 2 commits November 8, 2023 17:57

Fix tests

5e9e33e

try again

8f75c45

soumyava reviewed Nov 14, 2023

View reviewed changes

Fix spelling

6f89b2d

github-advanced-security bot found potential problems Nov 24, 2023

View reviewed changes

abhishekagarwal87 added 4 commits November 25, 2023 18:45

Fix other tests

7a29c85

Fix again

d1ede2f

Fix MSQSelectTest

b868756

Fix CalciteSelectQueryMSQTest

2b5026e

github-actions bot added the Area - Segment Format and Ser/De label Nov 26, 2023

abhishekagarwal87 added 2 commits November 26, 2023 22:00

Revert "Fix CalciteSelectQueryMSQTest"

14d5a5e

This reverts commit 2b5026e.

Add the right change

b422611

github-actions bot removed the Area - Segment Format and Ser/De label Nov 26, 2023

abhishekagarwal87 commented Nov 27, 2023

View reviewed changes

clintropolis reviewed Nov 28, 2023

View reviewed changes

clintropolis approved these changes Nov 28, 2023

View reviewed changes

Minor refactoring

e1fa913

abhishekagarwal87 merged commit 0a56c87 into apache:master Nov 29, 2023
83 checks passed

LakshSingla added this to the 29.0.0 milestone Jan 29, 2024

LakshSingla mentioned this pull request Feb 13, 2024

[DRAFT] 29.0.0 release notes #15896

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SQL: Plan non-equijoin conditions as cross join followed by filter #15302

SQL: Plan non-equijoin conditions as cross join followed by filter #15302

abhishekagarwal87 commented Nov 1, 2023 •

edited

soumyava Nov 14, 2023

soumyava Nov 14, 2023 •

edited

soumyava Nov 14, 2023

abhishekagarwal87 Nov 27, 2023

soumyava Nov 14, 2023

abhishekagarwal87 Nov 26, 2023

clintropolis left a comment

clintropolis Nov 28, 2023

abhishekagarwal87 Nov 28, 2023

abhishekagarwal87 commented Nov 29, 2023

		// I am not sure in what case, the list of system fields will be not empty. I have just picked up this logic
		// directly from https://github.com/apache/calcite/blob/calcite-1.35.0/core/src/main/java/org/apache/calcite/rel/rules/AbstractJoinExtractFilterRule.java#L58

SQL: Plan non-equijoin conditions as cross join followed by filter #15302

SQL: Plan non-equijoin conditions as cross join followed by filter #15302

Conversation

abhishekagarwal87 commented Nov 1, 2023 • edited

soumyava Nov 14, 2023

Choose a reason for hiding this comment

soumyava Nov 14, 2023 • edited

Choose a reason for hiding this comment

soumyava Nov 14, 2023

Choose a reason for hiding this comment

abhishekagarwal87 Nov 27, 2023

Choose a reason for hiding this comment

soumyava Nov 14, 2023

Choose a reason for hiding this comment

abhishekagarwal87 Nov 26, 2023

Choose a reason for hiding this comment

clintropolis left a comment

Choose a reason for hiding this comment

clintropolis Nov 28, 2023

Choose a reason for hiding this comment

abhishekagarwal87 Nov 28, 2023

Choose a reason for hiding this comment

abhishekagarwal87 commented Nov 29, 2023

abhishekagarwal87 commented Nov 1, 2023 •

edited

soumyava Nov 14, 2023 •

edited