imply-cheddar commented on code in PR #17483:
URL: https://github.com/apache/druid/pull/17483#discussion_r1858960397
##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/test/CalciteUnionQueryMSQTest.java:
##########
@@ -62,13 +62,12 @@ protected QueryTestBuilder testBuilder()
*/
@Test
@Override
- public void testUnionIsUnplannable()
+ public void testUnionIsUnplannableInNative()
Review Comment:
Aside from the name change, why does this evaluate native-only?
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/PlannerContext.java:
##########
@@ -508,6 +509,9 @@ public String getPlanningError()
*/
public void setPlanningError(String formatText, Object... arguments)
{
+ if (queryContext().isDecoupledMode()) {
+ throw InvalidSqlInput.exception(formatText, arguments);
+ }
Review Comment:
Can we not have a "DecoupledPlannerContext" or something that just overrides
this method and throws the exception instead of checks the context? I don't
actually know where the PlannerContext is created, so no clue how hard/easy it
is to do it, but this check feels ugly.
##########
processing/src/test/java/org/apache/druid/query/QueryContextTest.java:
##########
@@ -372,6 +372,20 @@ public void testDefaultEnableQueryDebugging()
assertTrue(QueryContext.of(ImmutableMap.of(QueryContexts.ENABLE_DEBUG,
true)).isDebug());
}
+ @Test
+ public void testDefaultIsDecoupled()
Review Comment:
Perhaps also test what happens when a completely different, random value is
set. Like "bilyBob"
##########
sql/src/main/java/org/apache/druid/sql/calcite/rel/logical/DruidUnion.java:
##########
@@ -53,6 +53,9 @@ public DruidUnion(
boolean all)
{
super(cluster, traits, hints, inputs, all);
+ if (!all) {
+ throw InvalidInput.exception("SQL requires 'UNION' but only 'UNION ALL'
is supported.");
Review Comment:
Nit: `InvalidSqlInput`
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
