[incubator-druid] branch 0.16.0-incubating updated: fix caching bug with multi-column group-by (#8535) (#8542)

[cwylie]

This is an automated email from the ASF dual-hosted git repository.

cwylie pushed a commit to branch 0.16.0-incubating
in repository https://gitbox.apache.org/repos/asf/incubator-druid.git


The following commit(s) were added to refs/heads/0.16.0-incubating by this push:
     new b495630  fix caching bug with multi-column group-by (#8535) (#8542)
b495630 is described below

commit b4956308f1b4259c36f08ca32d2ec6a6bb9c5b7f
Author: Clint Wylie <cwy...@apache.org>
AuthorDate: Fri Sep 13 20:12:54 2019 -0700

    fix caching bug with multi-column group-by (#8535) (#8542)
    
    * fix caching bug with multi-column group-by
    
    * review
---
 .../query/groupby/GroupByQueryQueryToolChest.java  |  3 +-
 .../groupby/GroupByQueryQueryToolChestTest.java    | 48 ++++++++++++++++++++++
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git 
a/processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChest.java
 
b/processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChest.java
index b942660..5b6bb6d 100644
--- 
a/processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChest.java
+++ 
b/processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChest.java
@@ -633,8 +633,7 @@ public class GroupByQueryQueryToolChest extends 
QueryToolChest<ResultRow, GroupB
             }
 
             final Iterator<DimensionSpec> dimsIter = dims.iterator();
-            int dimPos = 0;
-            while (dimsIter.hasNext() && results.hasNext()) {
+            for (int dimPos = 0; dimsIter.hasNext() && results.hasNext(); 
dimPos++) {
               final DimensionSpec dimensionSpec = dimsIter.next();
 
               // Must convert generic Jackson-deserialized type into the 
proper type.
diff --git 
a/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChestTest.java
 
b/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChestTest.java
index d843060..cf59d7e 100644
--- 
a/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChestTest.java
+++ 
b/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChestTest.java
@@ -59,6 +59,7 @@ import 
org.apache.druid.query.groupby.orderby.OrderByColumnSpec;
 import org.apache.druid.query.ordering.StringComparators;
 import org.apache.druid.segment.TestHelper;
 import org.apache.druid.segment.column.ValueType;
+import org.apache.druid.segment.virtual.ExpressionVirtualColumn;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -508,6 +509,53 @@ public class GroupByQueryQueryToolChestTest
   }
 
   @Test
+  public void testMultiColumnCacheStrategy() throws Exception
+  {
+    final GroupByQuery query1 = GroupByQuery
+        .builder()
+        .setDataSource(QueryRunnerTestHelper.DATA_SOURCE)
+        .setQuerySegmentSpec(QueryRunnerTestHelper.FIRST_TO_THIRD)
+        .setDimensions(ImmutableList.of(
+            new DefaultDimensionSpec("test", "test", ValueType.STRING),
+            new DefaultDimensionSpec("v0", "v0", ValueType.STRING)
+        ))
+        .setVirtualColumns(
+            new ExpressionVirtualColumn("v0", "concat('foo', test)", 
ValueType.STRING, TestExprMacroTable.INSTANCE)
+        )
+        .setAggregatorSpecs(
+            Arrays.asList(
+                QueryRunnerTestHelper.ROWS_COUNT,
+                getComplexAggregatorFactoryForValueType(ValueType.STRING)
+            )
+        )
+        .setPostAggregatorSpecs(
+            ImmutableList.of(new ConstantPostAggregator("post", 10))
+        )
+        .setGranularity(QueryRunnerTestHelper.DAY_GRAN)
+        .build();
+
+    CacheStrategy<ResultRow, Object, GroupByQuery> strategy =
+        new GroupByQueryQueryToolChest(null, null).getCacheStrategy(
+            query1
+        );
+
+    // test timestamps that result in integer size millis
+    final ResultRow result1 = ResultRow.of(123L, "val1", "fooval1", 1, 
getIntermediateComplexValue(ValueType.STRING, "val1"));
+
+    Object preparedValue = 
strategy.prepareForSegmentLevelCache().apply(result1);
+
+    ObjectMapper objectMapper = TestHelper.makeJsonMapper();
+    Object fromCacheValue = objectMapper.readValue(
+        objectMapper.writeValueAsBytes(preparedValue),
+        strategy.getCacheObjectClazz()
+    );
+
+    ResultRow fromCacheResult = 
strategy.pullFromSegmentLevelCache().apply(fromCacheValue);
+
+    Assert.assertEquals(result1, fromCacheResult);
+  }
+
+  @Test
   public void testResultSerde() throws Exception
   {
     final GroupByQuery query = GroupByQuery


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org