ccaominh commented on a change in pull request #9923:
URL: https://github.com/apache/druid/pull/9923#discussion_r430742917
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -13917,4 +14073,38 @@ public void
testRepeatedIdenticalVirtualExpressionGrouping() throws Exception
)
);
}
+
+ /**
+ * This is a provider of query contexts that should be used by join tests.
+ * It tests various configs that can be passed to join queries. All the
configs provided by this provider should
+ * have the join query engine return the same results.
+ */
+ public static class QueryContextForJoinProvider
+ {
+ @UsedByJUnitParamsRunner
+ public static Object[] provideQueryContexts()
+ {
+ return new Object[] {
+ // default behavior
+ QUERY_CONTEXT_DEFAULT,
+ // filter value re-writes enabled
+ new ImmutableMap.Builder<String, Object>()
+ .putAll(QUERY_CONTEXT_DEFAULT)
+
.put(QueryContexts.JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS_ENABLE_KEY, true)
+ .build(),
Review comment:
May be worth explicitly setting JOIN_FILTER_REWRITE_ENABLE_KEY to true
here in case the default value changes in the future.
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -8803,6 +8841,7 @@ public void
testInnerJoinTwoLookupsToTableUsingNumericColumn() throws Exception
+ "FROM foo\n"
+ "INNER JOIN lookup.lookyloo l1 ON l1.k = foo.m1\n"
+ "INNER JOIN lookup.lookyloo l2 ON l2.k = l1.k",
+ queryContext,
Review comment:
Should line 8863 use `queryContext` instead of `QUERY_CONTEXT_DEFAULT`?
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -9279,22 +9330,24 @@ public void testInnerJoinCastLeft() throws Exception
)
.intervals(querySegmentSpec(Filtration.eternity()))
.columns("j0.k", "j0.v", "m1")
- .context(QUERY_CONTEXT_DEFAULT)
+ .context(queryContext)
Review comment:
Revert extra indent
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -9152,6 +9198,7 @@ public void
testCommaJoinTableLookupTableMismatchedTypes() throws Exception
"SELECT COUNT(*)\n"
+ "FROM foo, lookup.lookyloo l, numfoo\n"
+ "WHERE foo.cnt = l.k AND l.k = numfoo.cnt\n",
+ queryContext,
Review comment:
Should line 9219 use queryContext instead of QUERY_CONTEXT_DEFAULT?
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -9152,6 +9198,7 @@ public void
testCommaJoinTableLookupTableMismatchedTypes() throws Exception
"SELECT COUNT(*)\n"
+ "FROM foo, lookup.lookyloo l, numfoo\n"
+ "WHERE foo.cnt = l.k AND l.k = numfoo.cnt\n",
+ queryContext,
Review comment:
Yeah, doesn't need to be addressed by this PR
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
