fix bug with string first/last aggs when druid.generic.useDefaultValu…

…eForNull=false
clintropolis · Aug 15, 2023 · 9fb5451 · 9fb5451
1 parent 6b42d2e
commit 9fb5451
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 14 deletions.
diff --git a/...on-tests-ex/cases/src/test/resources/multi-stage-query/wikipedia_merge_index_queries.json b/...on-tests-ex/cases/src/test/resources/multi-stage-query/wikipedia_merge_index_queries.json
@@ -34,8 +34,8 @@
       "timestamp" : "2013-08-31T00:00:00.000Z",
       "event" : {
         "continent":"Asia",
-        "earliest_user":null,
-        "latest_user":null
+        "earliest_user":"masterYi",
+        "latest_user":"stringer"
       }
     } ]
   }

diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/first/StringFirstAggregator.java b/processing/src/main/java/org/apache/druid/query/aggregation/first/StringFirstAggregator.java
@@ -56,9 +56,6 @@ public StringFirstAggregator(
   @Override
   public void aggregate()
   {
-    if (timeSelector.isNull()) {
-      return;
-    }
     if (needsFoldCheck) {
       // Less efficient code path when folding is a possibility (we must read the value selector first just in case
       // it's a foldable object).
@@ -72,6 +69,9 @@ public void aggregate()
         firstValue = StringUtils.fastLooseChop(inPair.rhs, maxStringBytes);
       }
     } else {
+      if (timeSelector.isNull()) {
+        return;
+      }
       final long time = timeSelector.getLong();
 
       if (time < firstTime) {

diff --git a/...g/src/main/java/org/apache/druid/query/aggregation/first/StringFirstBufferAggregator.java b/...g/src/main/java/org/apache/druid/query/aggregation/first/StringFirstBufferAggregator.java
@@ -63,9 +63,6 @@ public void init(ByteBuffer buf, int position)
   @Override
   public void aggregate(ByteBuffer buf, int position)
   {
-    if (timeSelector.isNull()) {
-      return;
-    }
     if (needsFoldCheck) {
       // Less efficient code path when folding is a possibility (we must read the value selector first just in case
       // it's a foldable object).
@@ -86,6 +83,9 @@ public void aggregate(ByteBuffer buf, int position)
         }
       }
     } else {
+      if (timeSelector.isNull()) {
+        return;
+      }
       final long time = timeSelector.getLong();
       final long firstTime = buf.getLong(position);
 

diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/first/StringFirstLastUtils.java b/processing/src/main/java/org/apache/druid/query/aggregation/first/StringFirstLastUtils.java
@@ -120,6 +120,9 @@ public static SerializablePairLongString readPairFromSelectors(
       time = pair.lhs;
       string = pair.rhs;
     } else if (object != null) {
+      if (timeSelector.isNull()) {
+        return null;
+      }
       time = timeSelector.getLong();
       string = DimensionHandlerUtils.convertObjectToString(object);
     } else {

diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/last/StringLastAggregator.java b/processing/src/main/java/org/apache/druid/query/aggregation/last/StringLastAggregator.java
@@ -57,9 +57,6 @@ public StringLastAggregator(
   @Override
   public void aggregate()
   {
-    if (timeSelector.isNull()) {
-      return;
-    }
     if (needsFoldCheck) {
       // Less efficient code path when folding is a possibility (we must read the value selector first just in case
       // it's a foldable object).
@@ -73,6 +70,9 @@ public void aggregate()
         lastValue = StringUtils.fastLooseChop(inPair.rhs, maxStringBytes);
       }
     } else {
+      if (timeSelector.isNull()) {
+        return;
+      }
       final long time = timeSelector.getLong();
 
       if (time >= lastTime) {

diff --git a/...ing/src/main/java/org/apache/druid/query/aggregation/last/StringLastBufferAggregator.java b/...ing/src/main/java/org/apache/druid/query/aggregation/last/StringLastBufferAggregator.java
@@ -64,9 +64,6 @@ public void init(ByteBuffer buf, int position)
   @Override
   public void aggregate(ByteBuffer buf, int position)
   {
-    if (timeSelector.isNull()) {
-      return;
-    }
     if (needsFoldCheck) {
       // Less efficient code path when folding is a possibility (we must read the value selector first just in case
       // it's a foldable object).
@@ -87,6 +84,9 @@ public void aggregate(ByteBuffer buf, int position)
         }
       }
     } else {
+      if (timeSelector.isNull()) {
+        return;
+      }
       final long time = timeSelector.getLong();
       final long lastTime = buf.getLong(position);