review stuffs

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<typeName>` (e.g. `COMPLEX<hyperUnique>`).
+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<typeName>`, e.g. `COMPLEX<hyperUnique>`.
 
-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.

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<SegmentAn
   private static final TypeReference<SegmentAnalysis> TYPE_REFERENCE = new TypeReference<SegmentAnalysis>()
   {
   };
-  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<SegmentAnalysis, SegmentAnalysis> MERGE_TRANSFORM_FN = new Function<SegmentAnalysis, SegmentAnalysis>()
-  {
-    @Override
-    public SegmentAnalysis apply(SegmentAnalysis analysis)
-    {
-      return finalizeAnalysis(analysis);
-    }
-  };
+  private static final Function<SegmentAnalysis, SegmentAnalysis> 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<SegmentAnalysis> getCacheObjectClazz()
       @Override
       public Function<SegmentAnalysis, SegmentAnalysis> prepareForCache(boolean isResultLevelCache)
       {
-        return new Function<SegmentAnalysis, SegmentAnalysis>()
-        {
-          @Override
-          public SegmentAnalysis apply(@Nullable SegmentAnalysis input)
-          {
-            return input;
-          }
-        };
+        return input -> input;
       }
 
       @Override
       public Function<SegmentAnalysis, SegmentAnalysis> pullFromCache(boolean isResultLevelCache)
       {
-        return new Function<SegmentAnalysis, SegmentAnalysis>()
-        {
-          @Override
-          public SegmentAnalysis apply(@Nullable SegmentAnalysis input)
-          {
-            return input;
-          }
-        };
+        return input -> input;
       }
     };
   }
@@ -266,14 +239,7 @@ public <T extends LogicalSegment> List<T> filterSegments(SegmentMetadataQuery qu
     return Lists.newArrayList(
         Iterables.filter(
             segments,
-            new Predicate<T>()
-            {
-              @Override
-              public boolean apply(T input)
-              {
-                return (input.getInterval().overlaps(targetInterval));
-              }
-            }
+            input -> (input.getInterval().overlaps(targetInterval))
         )
     );
   }

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 extends Comparable> 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
+    );
   }
 }

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();
 }

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,24 +38,16 @@
 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;
 import java.util.Objects;
 
 public class SegmentMetadataQuery extends BaseQuery<SegmentAnalysis>
 {
-  /**
-   * 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<byte[]> 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<SegmentAnalysis> withOverriddenContext(Map<String, Object> contextOverride)
   {

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);