This is an automated email from the ASF dual-hosted git repository.
yqm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new c4ead9b2301 Allow time series query to return empty preferred ordering
for All Granularity (#18262)
c4ead9b2301 is described below
commit c4ead9b23010326999c65c566adc601d4b5533c9
Author: Cece Mei <[email protected]>
AuthorDate: Wed Jul 23 09:16:25 2025 -0700
Allow time series query to return empty preferred ordering for All
Granularity (#18262)
* allow time series query to return empty preferred ordering for
`Granularities.ALL`
* minor change in equal
* some updates for docker in IT
* only to cleanup
* some docker image change
* revert IT changes
* some test
* unit test
* more test
* modify test case
---
.../data/input/impl/AggregateProjectionSpec.java | 5 +-
.../query/timeseries/TimeseriesQueryEngine.java | 4 +-
.../druid/segment/projections/Projections.java | 23 ++++-
.../input/impl/AggregateProjectionSpecTest.java | 15 +++
.../query/timeseries/TimeseriesQueryTest.java | 5 +-
.../segment/AggregateProjectionMetadataTest.java | 40 ++++++++
.../druid/segment/CursorFactoryProjectionTest.java | 101 +++++++++++++++++++++
7 files changed, 183 insertions(+), 10 deletions(-)
diff --git
a/processing/src/main/java/org/apache/druid/data/input/impl/AggregateProjectionSpec.java
b/processing/src/main/java/org/apache/druid/data/input/impl/AggregateProjectionSpec.java
index 0fd4e614002..925fd26f4fe 100644
---
a/processing/src/main/java/org/apache/druid/data/input/impl/AggregateProjectionSpec.java
+++
b/processing/src/main/java/org/apache/druid/data/input/impl/AggregateProjectionSpec.java
@@ -32,7 +32,6 @@ import
org.apache.druid.java.util.common.granularity.Granularity;
import org.apache.druid.query.OrderBy;
import org.apache.druid.query.aggregation.AggregatorFactory;
import org.apache.druid.segment.AggregateProjectionMetadata;
-import org.apache.druid.segment.Cursors;
import org.apache.druid.segment.VirtualColumn;
import org.apache.druid.segment.VirtualColumns;
import org.apache.druid.segment.column.ColumnHolder;
@@ -175,8 +174,8 @@ public class AggregateProjectionSpec
private static ProjectionOrdering computeOrdering(VirtualColumns
virtualColumns, List<DimensionSchema> groupingColumns)
{
if (groupingColumns.isEmpty()) {
- // call it time ordered, there is no grouping columns so there is only 1
row for this projection
- return new ProjectionOrdering(Cursors.ascendingTimeOrder(), null);
+ // no ordering since there is only 1 row for this projection
+ return new ProjectionOrdering(List.of(), null);
}
final List<OrderBy> ordering =
Lists.newArrayListWithCapacity(groupingColumns.size());
diff --git
a/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQueryEngine.java
b/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQueryEngine.java
index 45872514b2e..02b9b883501 100644
---
a/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQueryEngine.java
+++
b/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQueryEngine.java
@@ -337,7 +337,9 @@ public class TimeseriesQueryEngine
.setAggregators(query.getAggregatorSpecs())
.setQueryContext(query.context())
.setPreferredOrdering(
- query.isDescending() ?
Cursors.descendingTimeOrder() : Cursors.ascendingTimeOrder()
+ query.getGranularity().equals(Granularities.ALL)
+ ? List.of()
+ : query.isDescending() ?
Cursors.descendingTimeOrder() : Cursors.ascendingTimeOrder()
)
.setQueryMetrics(queryMetrics)
.build()
diff --git
a/processing/src/main/java/org/apache/druid/segment/projections/Projections.java
b/processing/src/main/java/org/apache/druid/segment/projections/Projections.java
index ce26f322cf4..2a633aadf9d 100644
---
a/processing/src/main/java/org/apache/druid/segment/projections/Projections.java
+++
b/processing/src/main/java/org/apache/druid/segment/projections/Projections.java
@@ -50,6 +50,7 @@ import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import java.util.SortedSet;
import java.util.function.Function;
@@ -124,7 +125,7 @@ public class Projections
/**
* Returns true if column is defined in {@link
AggregateProjectionSpec#getGroupingColumns()} OR if the column does not
* exist in the base table. Part of determining if a projection can be used
for a given {@link CursorBuildSpec},
- *
+ *
* @see AggregateProjectionMetadata.Schema#matches(CursorBuildSpec,
PhysicalColumnChecker)
*/
@FunctionalInterface
@@ -153,6 +154,25 @@ public class Projections
{
return remapColumns;
}
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o) {
+ return true;
+ }
+ if (!(o instanceof ProjectionMatch)) {
+ return false;
+ }
+ ProjectionMatch that = (ProjectionMatch) o;
+ return Objects.equals(cursorBuildSpec, that.cursorBuildSpec) &&
Objects.equals(remapColumns, that.remapColumns);
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hash(cursorBuildSpec, remapColumns);
+ }
}
public static final class ProjectionMatchBuilder
@@ -208,7 +228,6 @@ public class Projections
/**
* Add a query {@link
AggregatorFactory#substituteCombiningFactory(AggregatorFactory)} which can
combine the inputs
* of a selector created by a projection {@link AggregatorFactory}
- *
*/
public ProjectionMatchBuilder addPreAggregatedAggregator(AggregatorFactory
aggregator)
{
diff --git
a/processing/src/test/java/org/apache/druid/data/input/impl/AggregateProjectionSpecTest.java
b/processing/src/test/java/org/apache/druid/data/input/impl/AggregateProjectionSpecTest.java
index 2e15cdec490..dcb47416321 100644
---
a/processing/src/test/java/org/apache/druid/data/input/impl/AggregateProjectionSpecTest.java
+++
b/processing/src/test/java/org/apache/druid/data/input/impl/AggregateProjectionSpecTest.java
@@ -64,6 +64,21 @@ class AggregateProjectionSpecTest extends
InitializedNullHandlingTest
Assertions.assertEquals(spec,
JSON_MAPPER.readValue(JSON_MAPPER.writeValueAsString(spec),
AggregateProjectionSpec.class));
}
+ @Test
+ void testComputeOrdering_noOrdering()
+ {
+ AggregateProjectionSpec spec = new AggregateProjectionSpec(
+ "some_projection",
+ VirtualColumns.EMPTY,
+ List.of(),
+ new AggregatorFactory[] {
+ new CountAggregatorFactory("count"),
+ new LongSumAggregatorFactory("e", "e")
+ }
+ );
+ Assertions.assertTrue(spec.getOrdering().isEmpty());
+ }
+
@Test
void testMissingName()
{
diff --git
a/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryTest.java
b/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryTest.java
index 9834e221cd5..2565a7467e2 100644
---
a/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryTest.java
+++
b/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryTest.java
@@ -150,10 +150,7 @@ public class TimeseriesQueryTest extends
InitializedNullHandlingTest
buildSpec.getAggregators()
);
Assert.assertEquals(virtualColumns, buildSpec.getVirtualColumns());
- Assert.assertEquals(
- descending ? Cursors.descendingTimeOrder() :
Cursors.ascendingTimeOrder(),
- buildSpec.getPreferredOrdering()
- );
+ Assert.assertTrue(buildSpec.getPreferredOrdering().isEmpty());
}
@Test
diff --git
a/processing/src/test/java/org/apache/druid/segment/AggregateProjectionMetadataTest.java
b/processing/src/test/java/org/apache/druid/segment/AggregateProjectionMetadataTest.java
index 1c7d1bfe24c..8e025600686 100644
---
a/processing/src/test/java/org/apache/druid/segment/AggregateProjectionMetadataTest.java
+++
b/processing/src/test/java/org/apache/druid/segment/AggregateProjectionMetadataTest.java
@@ -21,6 +21,9 @@ package org.apache.druid.segment;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import it.unimi.dsi.fastutil.objects.ObjectAVLTreeSet;
import nl.jqno.equalsverifier.EqualsVerifier;
import org.apache.druid.error.DruidException;
@@ -30,6 +33,7 @@ import org.apache.druid.query.aggregation.AggregatorFactory;
import org.apache.druid.query.aggregation.CountAggregatorFactory;
import org.apache.druid.query.aggregation.LongSumAggregatorFactory;
import org.apache.druid.segment.column.ColumnHolder;
+import org.apache.druid.segment.projections.Projections;
import org.apache.druid.testing.InitializedNullHandlingTest;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -293,4 +297,40 @@ class AggregateProjectionMetadataTest extends
InitializedNullHandlingTest
.usingGetClass()
.verify();
}
+
+ @Test
+ public void testSchemaMatchSimple()
+ {
+ // arrange
+ AggregateProjectionMetadata spec = new AggregateProjectionMetadata(
+ new AggregateProjectionMetadata.Schema(
+ "some_projection",
+ null,
+ VirtualColumns.EMPTY,
+ Arrays.asList("a", "b"),
+ new AggregatorFactory[]{new
LongSumAggregatorFactory("a_projection", "a")},
+ Arrays.asList(OrderBy.ascending("a"), OrderBy.ascending("b"))
+ ),
+ 12345
+ );
+ CursorBuildSpec cursorBuildSpec = CursorBuildSpec.builder()
+
.setPreferredOrdering(ImmutableList.of())
+
.setAggregators(ImmutableList.of(new LongSumAggregatorFactory(
+ "a",
+ "a"
+ )))
+ .build();
+ // act & assert
+ Projections.ProjectionMatch projectionMatch = spec.getSchema()
+
.matches(cursorBuildSpec, (projectionName, columnName) -> true);
+ Projections.ProjectionMatch expected = new Projections.ProjectionMatch(
+ CursorBuildSpec.builder()
+ .setAggregators(ImmutableList.of(new
LongSumAggregatorFactory("a", "a")))
+ .setPhysicalColumns(ImmutableSet.of("a_projection"))
+ .setPreferredOrdering(ImmutableList.of())
+ .build(),
+ ImmutableMap.of("a", "a_projection")
+ );
+ Assertions.assertEquals(expected, projectionMatch);
+ }
}
diff --git
a/processing/src/test/java/org/apache/druid/segment/CursorFactoryProjectionTest.java
b/processing/src/test/java/org/apache/druid/segment/CursorFactoryProjectionTest.java
index 7936b06329c..32f695b76b1 100644
---
a/processing/src/test/java/org/apache/druid/segment/CursorFactoryProjectionTest.java
+++
b/processing/src/test/java/org/apache/druid/segment/CursorFactoryProjectionTest.java
@@ -226,6 +226,14 @@ public class CursorFactoryProjectionTest extends
InitializedNullHandlingTest
new LongSumAggregatorFactory("_c_sum", "c")
}
),
+ new AggregateProjectionSpec(
+ "b_c_sum",
+ VirtualColumns.EMPTY,
+ List.of(new StringDimensionSchema("b")),
+ new AggregatorFactory[]{
+ new LongSumAggregatorFactory("_c_sum", "c")
+ }
+ ),
new AggregateProjectionSpec(
"ab",
null,
@@ -1343,6 +1351,99 @@ public class CursorFactoryProjectionTest extends
InitializedNullHandlingTest
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 testTimeseriesQueryAllGranularitiesAlwaysRuns()
+ {
+ // Same test as
testTimeseriesQueryAllGranularityCanMatchNonTimeDimProjection, but no
projection used.
+ // Query can run with segment time ordering on/off.
+ 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.NO_PROJECTIONS, true))
+ .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(8, 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()
+ {
+ // Query has `__time ASC` ordering, but projection has `b ASC` ordering,
so no projection can be used.
+ // This is not ideal, since we know query has Granularities.DAY, and
segment also has Granularities.DAY, so the __time ordering does not matter.
+ final TimeseriesQuery query = Druids.newTimeseriesQueryBuilder()
+ .dataSource("test")
+
.intervals(ImmutableList.of(Intervals.ETERNITY))
+ .granularity(Granularities.DAY)
+ .aggregators(new
LongSumAggregatorFactory("c_sum", "c"))
+
.context(ImmutableMap.of(QueryContexts.USE_PROJECTION, "b_c_sum"))
+ .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[b_c_sum] specified, but does not satisfy
query", e.getMessage());
+ }
+
@Test
public void testTimeseriesQueryGranularityFitsProjectionGranularity()
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
