clintropolis commented on code in PR #18262:
URL: https://github.com/apache/druid/pull/18262#discussion_r2220034984


##########
processing/src/test/java/org/apache/druid/segment/CursorFactoryProjectionTest.java:
##########
@@ -1343,6 +1351,61 @@ public void 
testProjectionSelectionMissingColumnOnBaseTableToo()
     Assert.assertArrayEquals(new Object[]{"b", null, 12L, 13.2}, 
results.get(1).getArray());
   }
 
+  @Test
+  public void testTimeseriesQueryAllGranularityCanMatchNonTimeDimProjection()
+  {
+    final TimeseriesQuery query = Druids.newTimeseriesQueryBuilder()
+                                        .dataSource("test")
+                                        
.intervals(ImmutableList.of(Intervals.ETERNITY))
+                                        .granularity(Granularities.ALL)
+                                        .aggregators(new 
LongSumAggregatorFactory("c_sum", "c"))
+                                        
.context(ImmutableMap.of(QueryContexts.USE_PROJECTION, "b_c_sum"))
+                                        .build();
+
+    final CursorBuildSpec buildSpec = 
TimeseriesQueryEngine.makeCursorBuildSpec(query, null);
+    try (final CursorHolder cursorHolder = 
projectionsCursorFactory.makeCursorHolder(buildSpec)) {
+      final Cursor cursor = cursorHolder.asCursor();
+      int rowCount = 0;
+      while (!cursor.isDone()) {
+        rowCount++;
+        cursor.advance();
+      }
+      Assert.assertEquals(4, rowCount);
+    }
+
+    final Sequence<Result<TimeseriesResultValue>> resultRows = 
timeseriesEngine.process(
+        query,
+        projectionsCursorFactory,
+        projectionsTimeBoundaryInspector,
+        null
+    );
+
+    final List<Result<TimeseriesResultValue>> results = resultRows.toList();
+    Assert.assertEquals(1, results.size());
+    final RowSignature querySignature = 
query.getResultRowSignature(RowSignature.Finalization.YES);
+    Assert.assertArrayEquals(new Object[]{TIMESTAMP, 19L}, 
getResultArray(results.get(0), querySignature));
+  }
+
+  @Test
+  public void testTimeseriesQueryOrderByNotCompatibleWithProjection()
+  {
+    final TimeseriesQuery query = Druids.newTimeseriesQueryBuilder()
+                                        .dataSource("test")
+                                        
.intervals(ImmutableList.of(Intervals.ETERNITY))
+                                        .granularity(Granularities.DAY)
+                                        .descending(true)
+                                        .aggregators(new 
LongSumAggregatorFactory("c_sum", "c"))
+                                        
.context(ImmutableMap.of(QueryContexts.USE_PROJECTION, "c_sum_daily"))
+                                        .build();
+
+    final CursorBuildSpec buildSpec = 
TimeseriesQueryEngine.makeCursorBuildSpec(query, null);
+    DruidException e = Assert.assertThrows(
+        DruidException.class,
+        () -> projectionsCursorFactory.makeCursorHolder(buildSpec));
+    Assert.assertEquals(DruidException.Category.INVALID_INPUT, 
e.getCategory());
+    Assert.assertEquals("Projection[c_sum_daily] specified, but does not 
satisfy query", e.getMessage());

Review Comment:
   oh actually c_sum_daily has a day granularity time column, not missing time 
column, so is different from the change in `AggregateProjectionSpec` isn't it. 
Still though, why doesn't this match, is it the direction? since it is ordered 
by __time it could still work with a descending cursor, maybe the match code 
needs to allow descending time order if the projection is still ordered by time?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to