From be1c2ba7e23b039fa055e08d1a45770632b45342 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Mon, 8 Nov 2021 18:25:03 -0800 Subject: [PATCH] review stuffs --- docs/querying/segmentmetadataquery.md | 6 +-- .../SegmentMetadataQueryQueryToolChest.java | 52 ++++--------------- .../metadata/metadata/ColumnAnalysis.java | 32 ++++++++---- .../metadata/metadata/ColumnIncluderator.java | 4 +- .../metadata/SegmentMetadataQuery.java | 32 ++---------- ...egmentMetadataQueryQueryToolChestTest.java | 2 +- 6 files changed, 39 insertions(+), 89 deletions(-) diff --git a/docs/querying/segmentmetadataquery.md b/docs/querying/segmentmetadataquery.md index db3e873ff0d9..4bdbd8f4e8d0 100644 --- a/docs/querying/segmentmetadataquery.md +++ b/docs/querying/segmentmetadataquery.md @@ -87,11 +87,11 @@ The format of the result is: } ] ``` -All columns will contain a `typeSignature` which is the Druid internal representation of the type information for this column, is what is show in [`INFORMATION_SCHEMA.COLUMNS`](../querying/sql.md#columns-table) table in SQL, and is typically the value used to supply Druid with JSON type information at query or ingest time. This value will be `STRING`, `FLOAT`, `DOUBLE`, `LONG`, or `COMPLEX` (e.g. `COMPLEX`). +All columns contain a `typeSignature` that Druid uses to represent the column type information internally. The `typeSignature` is typically the same value used to identify the JSON type information at query or ingest time. One of: STRING`, `FLOAT`, `DOUBLE`, `LONG`, or `COMPLEX`, e.g. `COMPLEX`. -Additionally, columns will have a legacy friendly `type` name. This might match `typeSignature` for some column types (`STRING`, `FLOAT`, `DOUBLE`, or `LONG`) but for COMPLEX columns will only contain the name of the underlying complex type such as `hyperUnique`. +Columns will also have a legacy `type` name. For some column types, the value may match the `typeSignature` (`STRING`, `FLOAT`, `DOUBLE`, or `LONG`). For `COMPLEX` columns, the `type` only contains the name of the underlying complex type such as `hyperUnique`. -The timestamp column will always have `typeSignature` and `type` as `LONG`. +New applications should use `typeSignature`, not `type`. If the `errorMessage` field is non-null, you should not trust the other fields in the response. Their contents are undefined. diff --git a/processing/src/main/java/org/apache/druid/query/metadata/SegmentMetadataQueryQueryToolChest.java b/processing/src/main/java/org/apache/druid/query/metadata/SegmentMetadataQueryQueryToolChest.java index a40bd69ee55c..1bb24ef2e716 100644 --- a/processing/src/main/java/org/apache/druid/query/metadata/SegmentMetadataQueryQueryToolChest.java +++ b/processing/src/main/java/org/apache/druid/query/metadata/SegmentMetadataQueryQueryToolChest.java @@ -23,7 +23,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Functions; -import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; @@ -58,8 +57,6 @@ import org.joda.time.DateTime; import org.joda.time.Interval; -import javax.annotation.Nullable; -import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; import java.util.Comparator; @@ -75,16 +72,10 @@ public class SegmentMetadataQueryQueryToolChest extends QueryToolChest TYPE_REFERENCE = new TypeReference() { }; - private static final byte[] SEGMENT_METADATA_CACHE_PREFIX = new byte[]{0x4}; + private static final byte SEGMENT_METADATA_CACHE_PREFIX = 0x4; private static final byte SEGMENT_METADATA_QUERY = 0x16; - private static final Function MERGE_TRANSFORM_FN = new Function() - { - @Override - public SegmentAnalysis apply(SegmentAnalysis analysis) - { - return finalizeAnalysis(analysis); - } - }; + private static final Function MERGE_TRANSFORM_FN = + SegmentMetadataQueryQueryToolChest::finalizeAnalysis; private final SegmentMetadataQueryConfig config; private final GenericQueryMetricsFactory queryMetricsFactory; @@ -195,13 +186,9 @@ public boolean isCacheable(SegmentMetadataQuery query, boolean willMergeRunners) public byte[] computeCacheKey(SegmentMetadataQuery query) { SegmentMetadataQuery updatedQuery = query.withFinalizedAnalysisTypes(config); - byte[] includerBytes = updatedQuery.getToInclude().getCacheKey(); - byte[] analysisTypesBytes = updatedQuery.getAnalysisTypesCacheKey(); - return ByteBuffer.allocate(1 + includerBytes.length + analysisTypesBytes.length) - .put(SEGMENT_METADATA_CACHE_PREFIX) - .put(includerBytes) - .put(analysisTypesBytes) - .array(); + return new CacheKeyBuilder(SEGMENT_METADATA_CACHE_PREFIX).appendCacheable(updatedQuery.getToInclude()) + .appendCacheables(updatedQuery.getAnalysisTypes()) + .build(); } @Override @@ -223,27 +210,13 @@ public TypeReference getCacheObjectClazz() @Override public Function prepareForCache(boolean isResultLevelCache) { - return new Function() - { - @Override - public SegmentAnalysis apply(@Nullable SegmentAnalysis input) - { - return input; - } - }; + return input -> input; } @Override public Function pullFromCache(boolean isResultLevelCache) { - return new Function() - { - @Override - public SegmentAnalysis apply(@Nullable SegmentAnalysis input) - { - return input; - } - }; + return input -> input; } }; } @@ -266,14 +239,7 @@ public List filterSegments(SegmentMetadataQuery qu return Lists.newArrayList( Iterables.filter( segments, - new Predicate() - { - @Override - public boolean apply(T input) - { - return (input.getInterval().overlaps(targetInterval)); - } - } + input -> (input.getInterval().overlaps(targetInterval)) ) ); } diff --git a/processing/src/main/java/org/apache/druid/query/metadata/metadata/ColumnAnalysis.java b/processing/src/main/java/org/apache/druid/query/metadata/metadata/ColumnAnalysis.java index 87de1d56fb88..4d68cfa32d48 100644 --- a/processing/src/main/java/org/apache/druid/query/metadata/metadata/ColumnAnalysis.java +++ b/processing/src/main/java/org/apache/druid/query/metadata/metadata/ColumnAnalysis.java @@ -73,15 +73,16 @@ public ColumnAnalysis( } @JsonProperty - public String getType() + public ColumnType getTypeSignature() { - return type; + return typeSignature; } @JsonProperty - public ColumnType getTypeSignature() + @Deprecated + public String getType() { - return typeSignature; + return type; } @JsonProperty @@ -146,13 +147,13 @@ public ColumnAnalysis fold(ColumnAnalysis rhs) return rhs; } - if (!type.equals(rhs.getType())) { + if (!Objects.equals(type, rhs.getType())) { return ColumnAnalysis.error( StringUtils.format("cannot_merge_diff_types: [%s] and [%s]", type, rhs.getType()) ); } - if (!typeSignature.equals(rhs.getTypeSignature())) { + if (!Objects.equals(typeSignature, rhs.getTypeSignature())) { return ColumnAnalysis.error( StringUtils.format( "cannot_merge_diff_types: [%s] and [%s]", @@ -204,8 +205,8 @@ private T choose(T obj1, T obj2, boolean max) public String toString() { return "ColumnAnalysis{" + - "type='" + type + '\'' + - ", columnType=" + typeSignature + + "typeSignature='" + typeSignature + '\'' + + ", type=" + type + ", hasMultipleValues=" + hasMultipleValues + ", hasNulls=" + hasNulls + ", size=" + size + @@ -229,8 +230,8 @@ public boolean equals(Object o) return hasMultipleValues == that.hasMultipleValues && hasNulls == that.hasNulls && size == that.size && - Objects.equals(type, that.type) && Objects.equals(typeSignature, that.typeSignature) && + Objects.equals(type, that.type) && Objects.equals(cardinality, that.cardinality) && Objects.equals(minValue, that.minValue) && Objects.equals(maxValue, that.maxValue) && @@ -240,7 +241,16 @@ public boolean equals(Object o) @Override public int hashCode() { - return Objects.hash(type, - typeSignature, hasMultipleValues, hasNulls, size, cardinality, minValue, maxValue, errorMessage); + return Objects.hash( + typeSignature, + type, + hasMultipleValues, + hasNulls, + size, + cardinality, + minValue, + maxValue, + errorMessage + ); } } diff --git a/processing/src/main/java/org/apache/druid/query/metadata/metadata/ColumnIncluderator.java b/processing/src/main/java/org/apache/druid/query/metadata/metadata/ColumnIncluderator.java index 600b2773d2ed..cf092f458d7f 100644 --- a/processing/src/main/java/org/apache/druid/query/metadata/metadata/ColumnIncluderator.java +++ b/processing/src/main/java/org/apache/druid/query/metadata/metadata/ColumnIncluderator.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; +import org.apache.druid.java.util.common.Cacheable; /** */ @@ -30,8 +31,7 @@ @JsonSubTypes.Type(name = "all", value = AllColumnIncluderator.class), @JsonSubTypes.Type(name = "list", value = ListColumnIncluderator.class) }) -public interface ColumnIncluderator +public interface ColumnIncluderator extends Cacheable { boolean include(String columnName); - byte[] getCacheKey(); } diff --git a/processing/src/main/java/org/apache/druid/query/metadata/metadata/SegmentMetadataQuery.java b/processing/src/main/java/org/apache/druid/query/metadata/metadata/SegmentMetadataQuery.java index 3888f8c989bb..746dc4f224b8 100644 --- a/processing/src/main/java/org/apache/druid/query/metadata/metadata/SegmentMetadataQuery.java +++ b/processing/src/main/java/org/apache/druid/query/metadata/metadata/SegmentMetadataQuery.java @@ -23,7 +23,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonValue; import com.google.common.base.Preconditions; -import com.google.common.collect.Lists; +import org.apache.druid.java.util.common.Cacheable; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.query.BaseQuery; @@ -38,7 +38,6 @@ import org.apache.druid.query.spec.QuerySegmentSpec; import org.joda.time.Interval; -import java.nio.ByteBuffer; import java.util.EnumSet; import java.util.List; import java.util.Map; @@ -46,16 +45,9 @@ public class SegmentMetadataQuery extends BaseQuery { - /** - * The SegmentMetadataQuery cache key may contain UTF-8 column name strings. - * Prepend 0xFF before the analysisTypes as a separator to avoid - * any potential confusion with string values. - */ - public static final byte[] ANALYSIS_TYPES_CACHE_PREFIX = new byte[] {(byte) 0xFF}; - private static final QuerySegmentSpec DEFAULT_SEGMENT_SPEC = new MultipleIntervalSegmentSpec(Intervals.ONLY_ETERNITY); - public enum AnalysisType + public enum AnalysisType implements Cacheable { CARDINALITY, SIZE, @@ -79,6 +71,7 @@ public static AnalysisType fromString(String name) return valueOf(StringUtils.toUpperCase(name)); } + @Override public byte[] getCacheKey() { return new byte[] {(byte) this.ordinal()}; @@ -193,25 +186,6 @@ public boolean hasRollup() return analysisTypes.contains(AnalysisType.ROLLUP); } - public byte[] getAnalysisTypesCacheKey() - { - int size = 1; - List typeBytesList = Lists.newArrayListWithExpectedSize(analysisTypes.size()); - for (AnalysisType analysisType : analysisTypes) { - final byte[] bytes = analysisType.getCacheKey(); - typeBytesList.add(bytes); - size += bytes.length; - } - - final ByteBuffer bytes = ByteBuffer.allocate(size); - bytes.put(ANALYSIS_TYPES_CACHE_PREFIX); - for (byte[] typeBytes : typeBytesList) { - bytes.put(typeBytes); - } - - return bytes.array(); - } - @Override public Query withOverriddenContext(Map contextOverride) { diff --git a/processing/src/test/java/org/apache/druid/query/metadata/SegmentMetadataQueryQueryToolChestTest.java b/processing/src/test/java/org/apache/druid/query/metadata/SegmentMetadataQueryQueryToolChestTest.java index 934b433e52f5..b93c160b2906 100644 --- a/processing/src/test/java/org/apache/druid/query/metadata/SegmentMetadataQueryQueryToolChestTest.java +++ b/processing/src/test/java/org/apache/druid/query/metadata/SegmentMetadataQueryQueryToolChestTest.java @@ -70,7 +70,7 @@ public void testCacheStrategy() throws Exception new SegmentMetadataQueryQueryToolChest(new SegmentMetadataQueryConfig()).getCacheStrategy(query); // Test cache key generation - byte[] expectedKey = {0x04, 0x01, (byte) 0xFF, 0x00, 0x02, 0x04}; + byte[] expectedKey = {0x04, 0x09, 0x01, 0x0A, 0x00, 0x00, 0x00, 0x03, 0x00, 0x02, 0x04}; byte[] actualKey = strategy.computeCacheKey(query); Assert.assertArrayEquals(expectedKey, actualKey);