add missing json type for ListFilteredVirtualColumn (#11887)

* add missing json type for ListFilteredVirtualColumn, and tests to try to avoid this happening again

* fixes

* ugly, but maybe this

* oops

* too many mappers
This commit is contained in:
Clint Wylie 2021-11-09 17:25:12 -08:00 committed by GitHub
parent a36a41da73
commit a8805ab60d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 165 additions and 34 deletions

View File

@ -22,6 +22,7 @@ package org.apache.druid.query.aggregation.tdigestsketch.sql;
import com.fasterxml.jackson.databind.Module;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.java.util.common.granularity.Granularities;
import org.apache.druid.math.expr.ExprMacroTable;
@ -65,13 +66,16 @@ public class TDigestSketchSqlAggregatorTest extends BaseCalciteQueryTest
ImmutableSet.of()
);
@Override
public Iterable<? extends Module> getJacksonModules()
{
return Iterables.concat(super.getJacksonModules(), new TDigestSketchModule().getJacksonModules());
}
@Override
public SpecificSegmentsQuerySegmentWalker createQuerySegmentWalker() throws IOException
{
TDigestSketchModule.registerSerde();
for (Module mod : new TDigestSketchModule().getJacksonModules()) {
CalciteTests.getJsonMapper().registerModule(mod);
}
final QueryableIndex index =
IndexBuilder.create(CalciteTests.getJsonMapper())

View File

@ -23,6 +23,7 @@ import com.fasterxml.jackson.databind.Module;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.granularity.Granularities;
@ -51,7 +52,6 @@ import org.apache.druid.query.spec.MultipleIntervalSegmentSpec;
import org.apache.druid.query.timeseries.TimeseriesQuery;
import org.apache.druid.segment.IndexBuilder;
import org.apache.druid.segment.QueryableIndex;
import org.apache.druid.segment.TestHelper;
import org.apache.druid.segment.column.ColumnType;
import org.apache.druid.segment.incremental.IncrementalIndexSchema;
import org.apache.druid.segment.virtual.ExpressionVirtualColumn;
@ -78,14 +78,16 @@ public class HllSketchSqlAggregatorTest extends BaseCalciteQueryTest
{
private static final boolean ROUND = true;
@Override
public Iterable<? extends Module> getJacksonModules()
{
return Iterables.concat(super.getJacksonModules(), new HllSketchModule().getJacksonModules());
}
@Override
public SpecificSegmentsQuerySegmentWalker createQuerySegmentWalker() throws IOException
{
HllSketchModule.registerSerde();
for (Module mod : new HllSketchModule().getJacksonModules()) {
CalciteTests.getJsonMapper().registerModule(mod);
TestHelper.JSON_MAPPER.registerModule(mod);
}
final QueryableIndex index = IndexBuilder.create()
.tmpDir(temporaryFolder.newFolder())
.segmentWriteOutMediumFactory(OffHeapMemorySegmentWriteOutMediumFactory.instance())

View File

@ -22,6 +22,7 @@ package org.apache.druid.query.aggregation.datasketches.quantiles.sql;
import com.fasterxml.jackson.databind.Module;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.java.util.common.granularity.Granularities;
import org.apache.druid.math.expr.ExprMacroTable;
@ -88,13 +89,16 @@ public class DoublesSketchSqlAggregatorTest extends BaseCalciteQueryTest
)
);
@Override
public Iterable<? extends Module> getJacksonModules()
{
return Iterables.concat(super.getJacksonModules(), new DoublesSketchModule().getJacksonModules());
}
@Override
public SpecificSegmentsQuerySegmentWalker createQuerySegmentWalker() throws IOException
{
DoublesSketchModule.registerSerde();
for (Module mod : new DoublesSketchModule().getJacksonModules()) {
CalciteTests.getJsonMapper().registerModule(mod);
}
final QueryableIndex index =
IndexBuilder.create(CalciteTests.getJsonMapper())

View File

@ -22,6 +22,7 @@ package org.apache.druid.query.aggregation.datasketches.theta.sql;
import com.fasterxml.jackson.databind.Module;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.granularity.Granularities;
@ -46,7 +47,6 @@ import org.apache.druid.query.ordering.StringComparators;
import org.apache.druid.query.spec.MultipleIntervalSegmentSpec;
import org.apache.druid.segment.IndexBuilder;
import org.apache.druid.segment.QueryableIndex;
import org.apache.druid.segment.TestHelper;
import org.apache.druid.segment.column.ColumnType;
import org.apache.druid.segment.incremental.IncrementalIndexSchema;
import org.apache.druid.segment.virtual.ExpressionVirtualColumn;
@ -73,14 +73,16 @@ public class ThetaSketchSqlAggregatorTest extends BaseCalciteQueryTest
{
private static final String DATA_SOURCE = "foo";
@Override
public Iterable<? extends Module> getJacksonModules()
{
return Iterables.concat(super.getJacksonModules(), new SketchModule().getJacksonModules());
}
@Override
public SpecificSegmentsQuerySegmentWalker createQuerySegmentWalker() throws IOException
{
SketchModule.registerSerde();
for (Module mod : new SketchModule().getJacksonModules()) {
CalciteTests.getJsonMapper().registerModule(mod);
TestHelper.JSON_MAPPER.registerModule(mod);
}
final QueryableIndex index = IndexBuilder.create()
.tmpDir(temporaryFolder.newFolder())

View File

@ -19,8 +19,10 @@
package org.apache.druid.query.aggregation.bloom.sql;
import com.fasterxml.jackson.databind.Module;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.data.input.InputRow;
import org.apache.druid.data.input.impl.DimensionSchema;
@ -32,7 +34,7 @@ import org.apache.druid.data.input.impl.LongDimensionSchema;
import org.apache.druid.data.input.impl.MapInputRowParser;
import org.apache.druid.data.input.impl.TimeAndDimsParseSpec;
import org.apache.druid.data.input.impl.TimestampSpec;
import org.apache.druid.guice.BloomFilterSerializersModule;
import org.apache.druid.guice.BloomFilterExtensionModule;
import org.apache.druid.java.util.common.granularity.Granularities;
import org.apache.druid.query.Druids;
import org.apache.druid.query.aggregation.CountAggregatorFactory;
@ -76,10 +78,15 @@ public class BloomFilterSqlAggregatorTest extends BaseCalciteQueryTest
ImmutableSet.of()
);
@Override
public Iterable<? extends Module> getJacksonModules()
{
return Iterables.concat(super.getJacksonModules(), new BloomFilterExtensionModule().getJacksonModules());
}
@Override
public SpecificSegmentsQuerySegmentWalker createQuerySegmentWalker() throws IOException
{
CalciteTests.getJsonMapper().registerModule(new BloomFilterSerializersModule());
InputRowParser parser = new MapInputRowParser(
new TimeAndDimsParseSpec(
new TimestampSpec("t", "iso", null),

View File

@ -19,10 +19,13 @@
package org.apache.druid.query.filter.sql;
import com.fasterxml.jackson.databind.Module;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import org.apache.calcite.avatica.SqlType;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.guice.BloomFilterExtensionModule;
import org.apache.druid.guice.BloomFilterSerializersModule;
import org.apache.druid.guice.ExpressionModule;
import org.apache.druid.java.util.common.StringUtils;
@ -78,6 +81,12 @@ public class BloomDimFilterSqlTest extends BaseCalciteQueryTest
return new ExprMacroTable(exprMacros);
}
@Override
public Iterable<? extends Module> getJacksonModules()
{
return Iterables.concat(super.getJacksonModules(), new BloomFilterExtensionModule().getJacksonModules());
}
@Test
public void testBloomFilter() throws Exception
{

View File

@ -23,6 +23,7 @@ import com.fasterxml.jackson.databind.Module;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.java.util.common.granularity.Granularities;
import org.apache.druid.query.Druids;
@ -69,13 +70,16 @@ public class FixedBucketsHistogramQuantileSqlAggregatorTest extends BaseCalciteQ
ImmutableSet.of()
);
@Override
public Iterable<? extends Module> getJacksonModules()
{
return Iterables.concat(super.getJacksonModules(), new ApproximateHistogramDruidModule().getJacksonModules());
}
@Override
public SpecificSegmentsQuerySegmentWalker createQuerySegmentWalker() throws IOException
{
ApproximateHistogramDruidModule.registerSerde();
for (Module mod : new ApproximateHistogramDruidModule().getJacksonModules()) {
CalciteTests.getJsonMapper().registerModule(mod);
}
final QueryableIndex index = IndexBuilder.create(CalciteTests.getJsonMapper())
.tmpDir(temporaryFolder.newFolder())

View File

@ -22,6 +22,7 @@ package org.apache.druid.query.aggregation.histogram.sql;
import com.fasterxml.jackson.databind.Module;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.java.util.common.granularity.Granularities;
import org.apache.druid.math.expr.ExprMacroTable;
@ -68,13 +69,16 @@ public class QuantileSqlAggregatorTest extends BaseCalciteQueryTest
ImmutableSet.of()
);
@Override
public Iterable<? extends Module> getJacksonModules()
{
return Iterables.concat(super.getJacksonModules(), new ApproximateHistogramDruidModule().getJacksonModules());
}
@Override
public SpecificSegmentsQuerySegmentWalker createQuerySegmentWalker() throws IOException
{
ApproximateHistogramDruidModule.registerSerde();
for (Module mod : new ApproximateHistogramDruidModule().getJacksonModules()) {
CalciteTests.getJsonMapper().registerModule(mod);
}
final QueryableIndex index = IndexBuilder.create(CalciteTests.getJsonMapper())
.tmpDir(temporaryFolder.newFolder())

View File

@ -19,8 +19,10 @@
package org.apache.druid.query.aggregation.variance.sql;
import com.fasterxml.jackson.databind.Module;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.data.input.InputRow;
import org.apache.druid.data.input.impl.DimensionSchema;
@ -34,6 +36,7 @@ import org.apache.druid.query.Druids;
import org.apache.druid.query.aggregation.CountAggregatorFactory;
import org.apache.druid.query.aggregation.DoubleSumAggregatorFactory;
import org.apache.druid.query.aggregation.FilteredAggregatorFactory;
import org.apache.druid.query.aggregation.stats.DruidStatsModule;
import org.apache.druid.query.aggregation.variance.StandardDeviationPostAggregator;
import org.apache.druid.query.aggregation.variance.VarianceAggregatorCollector;
import org.apache.druid.query.aggregation.variance.VarianceAggregatorFactory;
@ -76,6 +79,12 @@ public class VarianceSqlAggregatorTest extends BaseCalciteQueryTest
ImmutableSet.of()
);
@Override
public Iterable<? extends Module> getJacksonModules()
{
return Iterables.concat(super.getJacksonModules(), new DruidStatsModule().getJacksonModules());
}
@Override
public SpecificSegmentsQuerySegmentWalker createQuerySegmentWalker() throws IOException
{

View File

@ -33,6 +33,7 @@ import org.apache.druid.segment.vector.VectorColumnSelectorFactory;
import org.apache.druid.segment.vector.VectorObjectSelector;
import org.apache.druid.segment.vector.VectorValueSelector;
import org.apache.druid.segment.virtual.ExpressionVirtualColumn;
import org.apache.druid.segment.virtual.ListFilteredVirtualColumn;
import javax.annotation.Nullable;
import java.util.List;
@ -47,7 +48,8 @@ import java.util.List;
*/
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type")
@JsonSubTypes(value = {
@JsonSubTypes.Type(name = "expression", value = ExpressionVirtualColumn.class)
@JsonSubTypes.Type(name = "expression", value = ExpressionVirtualColumn.class),
@JsonSubTypes.Type(name = "mv-filtered", value = ListFilteredVirtualColumn.class)
})
public interface VirtualColumn extends Cacheable
{

View File

@ -19,6 +19,9 @@
package org.apache.druid.sql.calcite;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.InjectableValues;
import com.fasterxml.jackson.databind.Module;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@ -26,6 +29,7 @@ import org.apache.calcite.plan.RelOptPlanner;
import org.apache.druid.annotations.UsedByJUnitParamsRunner;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.hll.VersionOneHyperLogLogCollector;
import org.apache.druid.jackson.DefaultObjectMapper;
import org.apache.druid.java.util.common.DateTimes;
import org.apache.druid.java.util.common.Intervals;
import org.apache.druid.java.util.common.StringUtils;
@ -55,6 +59,8 @@ import org.apache.druid.query.filter.OrDimFilter;
import org.apache.druid.query.filter.SelectorDimFilter;
import org.apache.druid.query.groupby.GroupByQuery;
import org.apache.druid.query.groupby.having.DimFilterHavingSpec;
import org.apache.druid.query.lookup.LookupExtractorFactoryContainerProvider;
import org.apache.druid.query.lookup.LookupSerdeModule;
import org.apache.druid.query.ordering.StringComparator;
import org.apache.druid.query.ordering.StringComparators;
import org.apache.druid.query.scan.ScanQuery;
@ -86,6 +92,7 @@ import org.apache.druid.sql.calcite.util.QueryLogHook;
import org.apache.druid.sql.calcite.util.SpecificSegmentsQuerySegmentWalker;
import org.apache.druid.sql.calcite.view.InProcessViewManager;
import org.apache.druid.sql.http.SqlParameter;
import org.apache.druid.timeline.DataSegment;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.Interval;
@ -121,8 +128,6 @@ public class BaseCalciteQueryTest extends CalciteTestBase
public static Long NULL_LONG;
public static final String HLLC_STRING = VersionOneHyperLogLogCollector.class.getName();
final boolean useDefault = NullHandling.replaceWithDefault();
@BeforeClass
public static void setupNullValues()
{
@ -262,15 +267,19 @@ public class BaseCalciteQueryTest extends CalciteTestBase
public static Closer resourceCloser;
public static int minTopNThreshold = TopNQueryConfig.DEFAULT_MIN_TOPN_THRESHOLD;
final boolean useDefault = NullHandling.replaceWithDefault();
@Rule
public ExpectedException expectedException = ExpectedException.none();
@Rule
public TemporaryFolder temporaryFolder = new TemporaryFolder();
public boolean cannotVectorize = false;
public boolean skipVectorize = false;
public ObjectMapper queryJsonMapper;
public SpecificSegmentsQuerySegmentWalker walker = null;
public QueryLogHook queryLogHook;
@ -469,13 +478,19 @@ public class BaseCalciteQueryTest extends CalciteTestBase
@Rule
public QueryLogHook getQueryLogHook()
{
return queryLogHook = QueryLogHook.create();
return queryLogHook = QueryLogHook.create(queryJsonMapper);
}
@Before
public void setUp() throws Exception
{
queryJsonMapper = createQueryJsonMapper();
walker = createQuerySegmentWalker();
// also register the static injected mapper, though across multiple test runs
ObjectMapper mapper = CalciteTests.getJsonMapper();
mapper.registerModules(getJacksonModules());
setMapperInjectableValues(mapper, getJacksonInjectables());
}
@After
@ -493,6 +508,16 @@ public class BaseCalciteQueryTest extends CalciteTestBase
);
}
public ObjectMapper createQueryJsonMapper()
{
// ugly workaround, for some reason the static mapper from Calcite.INJECTOR seems to get messed up over
// multiple test runs, resulting in missing subtypes that cause this JSON serde round-trip test
// this should be nearly identical to CalciteTests.getJsonMapper()
ObjectMapper mapper = new DefaultObjectMapper().registerModules(getJacksonModules());
setMapperInjectableValues(mapper, getJacksonInjectables());
return mapper;
}
public DruidOperatorTable createOperatorTable()
{
return CalciteTests.createOperatorTable();
@ -503,6 +528,36 @@ public class BaseCalciteQueryTest extends CalciteTestBase
return CalciteTests.createExprMacroTable();
}
public Map<String, Object> getJacksonInjectables()
{
return new HashMap<>();
}
public final void setMapperInjectableValues(ObjectMapper mapper, Map<String, Object> injectables)
{
// duplicate the injectable values from CalciteTests.INJECTOR initialization, mainly to update the injectable
// macro table, or whatever else you feel like injecting to a mapper
LookupExtractorFactoryContainerProvider lookupProvider =
CalciteTests.INJECTOR.getInstance(LookupExtractorFactoryContainerProvider.class);
mapper.setInjectableValues(new InjectableValues.Std(injectables)
.addValue(ExprMacroTable.class.getName(), createMacroTable())
.addValue(ObjectMapper.class.getName(), mapper)
.addValue(
DataSegment.PruneSpecsHolder.class,
DataSegment.PruneSpecsHolder.DEFAULT
)
.addValue(
LookupExtractorFactoryContainerProvider.class.getName(),
lookupProvider
)
);
}
public Iterable<? extends Module> getJacksonModules()
{
return new LookupSerdeModule().getJacksonModules();
}
public void assertQueryIsUnplannable(final String sql)
{
assertQueryIsUnplannable(PLANNER_CONFIG_DEFAULT, sql);
@ -730,7 +785,7 @@ public class BaseCalciteQueryTest extends CalciteTestBase
createOperatorTable(),
createMacroTable(),
CalciteTests.TEST_AUTHORIZER_MAPPER,
CalciteTests.getJsonMapper()
queryJsonMapper
);
}
@ -793,6 +848,20 @@ public class BaseCalciteQueryTest extends CalciteTestBase
expectedQueries.get(i),
recordedQueries.get(i)
);
try {
// go through some JSON serde and back, round tripping both queries and comparing them to each other, because
// Assert.assertEquals(recordedQueries.get(i), stringAndBack) is a failure due to a sorted map being present
// in the recorded queries, but it is a regular map after deserialization
final String recordedString = queryJsonMapper.writeValueAsString(recordedQueries.get(i));
final Query<?> stringAndBack = queryJsonMapper.readValue(recordedString, Query.class);
final String expectedString = queryJsonMapper.writeValueAsString(expectedQueries.get(i));
final Query<?> expectedStringAndBack = queryJsonMapper.readValue(expectedString, Query.class);
Assert.assertEquals(expectedStringAndBack, stringAndBack);
}
catch (JsonProcessingException e) {
Assert.fail(e.getMessage());
}
}
}
}
@ -887,7 +956,7 @@ public class BaseCalciteQueryTest extends CalciteTestBase
createOperatorTable(),
createMacroTable(),
CalciteTests.TEST_AUTHORIZER_MAPPER,
CalciteTests.getJsonMapper()
queryJsonMapper
);
SqlLifecycle lifecycle = lifecycleFactory.factorize();

View File

@ -19,6 +19,7 @@
package org.apache.druid.sql.calcite.util;
import com.fasterxml.jackson.databind.InjectableValues;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableList;
@ -78,7 +79,9 @@ import org.apache.druid.query.aggregation.LongSumAggregatorFactory;
import org.apache.druid.query.aggregation.hyperloglog.HyperUniquesAggregatorFactory;
import org.apache.druid.query.expression.LookupEnabledTestExprMacroTable;
import org.apache.druid.query.expression.LookupExprMacro;
import org.apache.druid.query.expression.TestExprMacroTable;
import org.apache.druid.query.lookup.LookupExtractorFactoryContainerProvider;
import org.apache.druid.query.lookup.LookupSerdeModule;
import org.apache.druid.segment.IndexBuilder;
import org.apache.druid.segment.QueryableIndex;
import org.apache.druid.segment.TestHelper;
@ -242,10 +245,6 @@ public class CalciteTests
public static final Injector INJECTOR = Guice.createInjector(
binder -> {
binder.bind(Key.get(ObjectMapper.class, Json.class)).toInstance(TestHelper.makeJsonMapper());
// This Module is just to get a LookupExtractorFactoryContainerProvider with a usable "lookyloo" lookup.
final LookupExtractorFactoryContainerProvider lookupProvider =
LookupEnabledTestExprMacroTable.createTestLookupProvider(
ImmutableMap.of(
@ -255,6 +254,21 @@ public class CalciteTests
"6", "x6"
)
);
ObjectMapper mapper = TestHelper.makeJsonMapper().registerModules(
new LookupSerdeModule().getJacksonModules()
);
mapper.setInjectableValues(
new InjectableValues.Std().addValue(ExprMacroTable.class.getName(), TestExprMacroTable.INSTANCE)
.addValue(ObjectMapper.class.getName(), mapper)
.addValue(DataSegment.PruneSpecsHolder.class, DataSegment.PruneSpecsHolder.DEFAULT)
.addValue(LookupExtractorFactoryContainerProvider.class.getName(), lookupProvider)
);
binder.bind(Key.get(ObjectMapper.class, Json.class)).toInstance(
mapper
);
// This Module is just to get a LookupExtractorFactoryContainerProvider with a usable "lookyloo" lookup.
binder.bind(LookupExtractorFactoryContainerProvider.class).toInstance(lookupProvider);
SqlBindings.addOperatorConversion(binder, QueryLookupOperatorConversion.class);
},
@ -1021,6 +1035,7 @@ public class CalciteTests
exprMacros.add(INJECTOR.getInstance(clazz));
}
exprMacros.add(INJECTOR.getInstance(LookupExprMacro.class));
return new ExprMacroTable(exprMacros);
}