Skip to content

Commit

Permalink
Enable quidem shadowing for decoupled testcases
Browse files Browse the repository at this point in the history
* Altered `QueryTestBuilder` to be able to switch to a backing quidem test
* added a small crc to ensure that the shadow testcase does not deviate from the original one
* Packaged all decoupled related things into a a single `DecoupledExtension` to reduce copy-paste
* `DecoupledTestConfig#quidemReason` must describe why its being used
* `DecoupledTestConfig#separateDefaultModeTest` can be used to make multiple case files based on `NullHandling` state
* fixed a cosmetic bug during decoupled join translation
* enhanced `!druidPlan` to report the final logical plan in non-decoupled mode as well
* add check to ensure that only supported params are present in a druidtest uri
* enabled shadow testcases for previously disabled testcases
  • Loading branch information
kgyrtkirk committed May 10, 2024
1 parent 30f3cf5 commit e13d560
Show file tree
Hide file tree
Showing 156 changed files with 17,331 additions and 84 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@
package org.apache.druid.sql.calcite.planner;

import com.fasterxml.jackson.annotation.JsonProperty;
import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.UOE;
import org.apache.druid.query.QueryContexts;
import org.joda.time.DateTimeZone;

import java.util.HashMap;
import java.util.Map;
import java.util.Objects;

Expand Down Expand Up @@ -428,4 +430,28 @@ public PlannerConfig build()
return config;
}
}

public Map<String, Object> getNonDefaultAsQueryContext()
{
Map<String, Object> overrides = new HashMap<>();
PlannerConfig def = new PlannerConfig();
if (def.useApproximateCountDistinct != useApproximateCountDistinct) {
overrides.put(
CTX_KEY_USE_APPROXIMATE_COUNT_DISTINCT,
String.valueOf(useApproximateCountDistinct)
);
}
if (def.useGroupingSetForExactDistinct != useGroupingSetForExactDistinct) {
overrides.put(
CTX_KEY_USE_GROUPING_SET_FOR_EXACT_DISTINCT,
String.valueOf(useGroupingSetForExactDistinct)
);
}

PlannerConfig newConfig = PlannerConfig.builder().withOverrides(overrides).build();
if (!equals(newConfig)) {
throw new IAE("Some configs are not handled in this method or not persistable as QueryContext keys!\nold: %s\nnew: %s", this, newConfig);
}
return overrides;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,11 @@ protected PlannerResult planWithDruidConvention() throws ValidationException
.plus(rootQueryRel.collation),
parameterized
);

handlerContext.hook().captureDruidRel(druidRel);

Hook.JAVA_PLAN.run(druidRel);

if (explain != null) {
return planExplanation(possiblyLimitedRoot, druidRel, true);
} else {
Expand Down
32 changes: 32 additions & 0 deletions sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidRel.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,19 @@

package org.apache.druid.sql.calcite.rel;

import com.google.common.collect.Iterables;
import org.apache.calcite.plan.RelOptCluster;
import org.apache.calcite.plan.RelTraitSet;
import org.apache.calcite.plan.volcano.RelSubset;
import org.apache.calcite.rel.AbstractRelNode;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.RelShuttleImpl;
import org.apache.calcite.rel.RelWriter;
import org.apache.druid.server.QueryResponse;
import org.apache.druid.sql.calcite.planner.PlannerContext;

import javax.annotation.Nullable;

import java.util.Set;

public abstract class DruidRel<T extends DruidRel<?>> extends AbstractRelNode
Expand Down Expand Up @@ -122,4 +127,31 @@ protected Object clone() throws CloneNotSupportedException
* Get the set of names of table datasources read by this DruidRel
*/
public abstract Set<String> getDataSourceNames();

public final RelNode unwrapLogicalPlan()
{
return accept(new LogicalPlanUnwrapperShuttle());
}

private static class LogicalPlanUnwrapperShuttle extends RelShuttleImpl
{
@Override
public RelNode visit(RelNode other)
{
return super.visit(visitNode(other));
}

private RelNode visitNode(RelNode other)
{
if (other instanceof RelSubset) {
final RelSubset subset = (RelSubset) other;
return visitNode(Iterables.getFirst(subset.getRels(), null));
}
if (other instanceof DruidRel<?>) {
DruidRel<?> druidRel = (DruidRel<?>) other;
return druidRel.getPartialDruidQuery().leafRel();
}
return other;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ public DruidJoinRule(Class<? extends RelNode> clazz, RelTrait in, RelTrait out,
// reject the query in case the anaysis detected any issues
throw InvalidSqlInput.exception(analysis.errorStr);
}

return new DruidJoin(
join.getCluster(),
newTrait,
Expand All @@ -67,7 +66,7 @@ public DruidJoinRule(Class<? extends RelNode> clazz, RelTrait in, RelTrait out,
join.getRight(),
DruidLogicalConvention.instance()
),
join.getCondition(),
analysis.getConditionWithUnsupportedSubConditionsIgnored(join.getCluster().getRexBuilder()),
join.getVariablesSet(),
join.getJoinType()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public interface DruidConnectionExtras
{
ObjectMapper getObjectMapper();

class DruidConnectionExtrasImpl implements DruidConnectionExtras
public class DruidConnectionExtrasImpl implements DruidConnectionExtras
{
private final ObjectMapper objectMapper;

Expand Down
41 changes: 41 additions & 0 deletions sql/src/test/java/org/apache/druid/quidem/DruidQTestInfo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.apache.druid.quidem;

import java.io.File;

public class DruidQTestInfo
{
public final File caseDir;
public final String testName;
public final String comment;

public DruidQTestInfo(File caseDir, String testName, String comment)
{
this.caseDir = caseDir;
this.testName = testName;
this.comment = comment;
}

public File getIQFile()
{
return new File(caseDir, testName + ".iq");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.apache.calcite.util.Util;
import org.apache.druid.query.Query;
import org.apache.druid.sql.calcite.BaseCalciteQueryTest;
import org.apache.druid.sql.calcite.rel.DruidRel;
import org.apache.druid.sql.calcite.util.QueryLogHook;

import java.sql.ResultSet;
Expand Down Expand Up @@ -171,6 +172,9 @@ protected final void executeExplain(Context x)
}

for (RelNode node : logged) {
if (node instanceof DruidRel<?>) {
node = ((DruidRel) node).unwrapLogicalPlan();
}
String str = RelOptUtil.dumpPlan("", node, SqlExplainFormat.TEXT, SqlExplainLevel.EXPPLAN_ATTRIBUTES);
x.echo(ImmutableList.of(str));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
import org.junit.Assert;
import org.junit.internal.matchers.ThrowableMessageMatcher;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Named;
import org.junit.jupiter.api.extension.RegisterExtension;

import javax.annotation.Nullable;
Expand Down Expand Up @@ -933,6 +934,12 @@ public Map<String, Object> baseQueryContext()
{
return baseQueryContext;
}

@Override
public SqlTestFramework queryFramework()
{
return BaseCalciteQueryTest.this.queryFramework();
}
}

public enum ResultMatchMode
Expand Down Expand Up @@ -1273,51 +1280,51 @@ public static Object[] provideQueryContexts()
{
return new Object[] {
// default behavior
QUERY_CONTEXT_DEFAULT,
Named.of("default", QUERY_CONTEXT_DEFAULT),
// all rewrites enabled
new ImmutableMap.Builder<String, Object>()
Named.of("all_enabled", new ImmutableMap.Builder<String, Object>()
.putAll(QUERY_CONTEXT_DEFAULT)
.put(QueryContexts.JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS_ENABLE_KEY, true)
.put(QueryContexts.JOIN_FILTER_REWRITE_ENABLE_KEY, true)
.put(QueryContexts.REWRITE_JOIN_TO_FILTER_ENABLE_KEY, true)
.build(),
.build()),
// filter-on-value-column rewrites disabled, everything else enabled
new ImmutableMap.Builder<String, Object>()
Named.of("filter-on-value-column_disabled", new ImmutableMap.Builder<String, Object>()
.putAll(QUERY_CONTEXT_DEFAULT)
.put(QueryContexts.JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS_ENABLE_KEY, false)
.put(QueryContexts.JOIN_FILTER_REWRITE_ENABLE_KEY, true)
.put(QueryContexts.REWRITE_JOIN_TO_FILTER_ENABLE_KEY, true)
.build(),
.build()),
// filter rewrites fully disabled, join-to-filter enabled
new ImmutableMap.Builder<String, Object>()
Named.of("join-to-filter", new ImmutableMap.Builder<String, Object>()
.putAll(QUERY_CONTEXT_DEFAULT)
.put(QueryContexts.JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS_ENABLE_KEY, false)
.put(QueryContexts.JOIN_FILTER_REWRITE_ENABLE_KEY, false)
.put(QueryContexts.REWRITE_JOIN_TO_FILTER_ENABLE_KEY, true)
.build(),
.build()),
// filter rewrites disabled, but value column filters still set to true
// (it should be ignored and this should
// behave the same as the previous context)
new ImmutableMap.Builder<String, Object>()
Named.of("filter-rewrites-disabled", new ImmutableMap.Builder<String, Object>()
.putAll(QUERY_CONTEXT_DEFAULT)
.put(QueryContexts.JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS_ENABLE_KEY, true)
.put(QueryContexts.JOIN_FILTER_REWRITE_ENABLE_KEY, false)
.put(QueryContexts.REWRITE_JOIN_TO_FILTER_ENABLE_KEY, true)
.build(),
.build()),
// filter rewrites fully enabled, join-to-filter disabled
new ImmutableMap.Builder<String, Object>()
Named.of("filter-rewrites", new ImmutableMap.Builder<String, Object>()
.putAll(QUERY_CONTEXT_DEFAULT)
.put(QueryContexts.JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS_ENABLE_KEY, true)
.put(QueryContexts.JOIN_FILTER_REWRITE_ENABLE_KEY, true)
.put(QueryContexts.REWRITE_JOIN_TO_FILTER_ENABLE_KEY, false)
.build(),
.build()),
// all rewrites disabled
new ImmutableMap.Builder<String, Object>()
Named.of("all_disabled", new ImmutableMap.Builder<String, Object>()
.putAll(QUERY_CONTEXT_DEFAULT)
.put(QueryContexts.JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS_ENABLE_KEY, false)
.put(QueryContexts.JOIN_FILTER_REWRITE_ENABLE_KEY, false)
.put(QueryContexts.REWRITE_JOIN_TO_FILTER_ENABLE_KEY, false)
.build(),
.build()),
};
}

Expand Down
Loading

0 comments on commit e13d560

Please sign in to comment.