This is an automated email from the ASF dual-hosted git repository.
clintropolis 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 36a7d964391 minor: empty cursor holder time ordering (#19773)
36a7d964391 is described below
commit 36a7d9643918cd1e2b96732953bba49ded34dfa5
Author: Clint Wylie <[email protected]>
AuthorDate: Mon Jul 27 23:26:10 2026 -0700
minor: empty cursor holder time ordering (#19773)
---
.../apache/druid/segment/EmptyCursorHolder.java | 32 +++++++-
.../PartialQueryableIndexCursorFactory.java | 2 +-
.../druid/segment/QueryableIndexCursorFactory.java | 2 +-
.../incremental/IncrementalIndexCursorFactory.java | 2 +-
.../ClusteredSegmentTimeOrderedQueryTest.java | 40 ++++++++--
.../druid/segment/EmptyCursorHolderTest.java | 91 ++++++++++++++++++++++
6 files changed, 155 insertions(+), 14 deletions(-)
diff --git
a/processing/src/main/java/org/apache/druid/segment/EmptyCursorHolder.java
b/processing/src/main/java/org/apache/druid/segment/EmptyCursorHolder.java
index 6b2906192ad..6b9e5c92162 100644
--- a/processing/src/main/java/org/apache/druid/segment/EmptyCursorHolder.java
+++ b/processing/src/main/java/org/apache/druid/segment/EmptyCursorHolder.java
@@ -20,6 +20,7 @@
package org.apache.druid.segment;
import org.apache.druid.error.DruidException;
+import org.apache.druid.query.Order;
import org.apache.druid.query.OrderBy;
import org.apache.druid.query.QueryContexts;
import org.apache.druid.query.dimension.DimensionSpec;
@@ -42,16 +43,39 @@ import java.util.List;
* A {@link CursorHolder} that yields no rows. Its cursors are always done,
but its selector factories hand out harmless
* nil selectors and {@link #canVectorize()} reports true.
* <p>
- * Useful wherever a segment can be proven to match no rows without scanning
it.
+ * Useful wherever a segment can be proven to match no rows without scanning
it. Use {@link #forSpec(CursorBuildSpec)}
+ * to produce an empty cursor matching the preferred ordering of the {@link
CursorBuildSpec}.
*/
public final class EmptyCursorHolder implements CursorHolder
{
- public static final EmptyCursorHolder INSTANCE = new EmptyCursorHolder();
+ private static final EmptyCursorHolder NO_ORDER = new
EmptyCursorHolder(Collections.emptyList());
+ private static final EmptyCursorHolder TIME_ASCENDING = new
EmptyCursorHolder(Cursors.ascendingTimeOrder());
+ private static final EmptyCursorHolder TIME_DESCENDING = new
EmptyCursorHolder(Cursors.descendingTimeOrder());
private static final ColumnSelectorFactory NIL_SELECTOR_FACTORY = new
AllNullColumnSelectorFactory();
- private EmptyCursorHolder()
+ /**
+ * An empty holder that advertises the spec's preferred ordering. A cursor
with no rows trivially satisfies any
+ * ordering, so it can honestly claim whatever the query asked for. The two
{@code __time} directions reuse cached
+ * singletons (the common case); any other non-empty preferred ordering gets
a fresh instance, and an empty preferred
+ * ordering yields the unordered shared instance.
+ */
+ public static EmptyCursorHolder forSpec(CursorBuildSpec spec)
{
+ final List<OrderBy> preferredOrdering = spec.getPreferredOrdering();
+ final Order timeOrder = Cursors.getTimeOrdering(preferredOrdering);
+ return switch (timeOrder) {
+ case ASCENDING -> TIME_ASCENDING;
+ case DESCENDING -> TIME_DESCENDING;
+ default -> preferredOrdering.isEmpty() ? NO_ORDER : new
EmptyCursorHolder(preferredOrdering);
+ };
+ }
+
+ private final List<OrderBy> ordering;
+
+ private EmptyCursorHolder(List<OrderBy> ordering)
+ {
+ this.ordering = ordering;
}
@Override
@@ -211,6 +235,6 @@ public final class EmptyCursorHolder implements CursorHolder
@Override
public List<OrderBy> getOrdering()
{
- return Collections.emptyList();
+ return ordering;
}
}
diff --git
a/processing/src/main/java/org/apache/druid/segment/PartialQueryableIndexCursorFactory.java
b/processing/src/main/java/org/apache/druid/segment/PartialQueryableIndexCursorFactory.java
index 31edef76062..2542d80424a 100644
---
a/processing/src/main/java/org/apache/druid/segment/PartialQueryableIndexCursorFactory.java
+++
b/processing/src/main/java/org/apache/druid/segment/PartialQueryableIndexCursorFactory.java
@@ -110,7 +110,7 @@ public class PartialQueryableIndexCursorFactory implements
CursorFactory
if (plan.clusterGroupPlan() != null) {
if (plan.bundles().isEmpty()) {
// Filter rules out every cluster group: no bundle to acquire, nothing
to download.
- return AsyncCursorHolder.completed(EmptyCursorHolder.INSTANCE);
+ return AsyncCursorHolder.completed(EmptyCursorHolder.forSpec(spec));
}
// reuse the plan's cluster resolution
return buildAsyncCursorHolder(
diff --git
a/processing/src/main/java/org/apache/druid/segment/QueryableIndexCursorFactory.java
b/processing/src/main/java/org/apache/druid/segment/QueryableIndexCursorFactory.java
index 96cc9d6fb6b..f9d55fe4e54 100644
---
a/processing/src/main/java/org/apache/druid/segment/QueryableIndexCursorFactory.java
+++
b/processing/src/main/java/org/apache/druid/segment/QueryableIndexCursorFactory.java
@@ -126,7 +126,7 @@ public class QueryableIndexCursorFactory implements
ResidentCursorFactory
public CursorHolder makeClusteredCursorHolder(CursorBuildSpec spec,
ClusterGroupQueryPlan plan)
{
if (plan.survivingGroups().isEmpty()) {
- return EmptyCursorHolder.INSTANCE;
+ return EmptyCursorHolder.forSpec(spec);
}
if (plan.survivingGroups().size() == 1) {
diff --git
a/processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndexCursorFactory.java
b/processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndexCursorFactory.java
index ac90cf6fb66..1780dd8c4b9 100644
---
a/processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndexCursorFactory.java
+++
b/processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndexCursorFactory.java
@@ -149,7 +149,7 @@ public class IncrementalIndexCursorFactory implements
ResidentCursorFactory
final List<TableClusterGroupSpec> surviving = plan.survivingGroups();
if (surviving.isEmpty()) {
- return EmptyCursorHolder.INSTANCE;
+ return EmptyCursorHolder.forSpec(spec);
}
final RowSignature clusteringColumns = summary.getClusteringColumns();
diff --git
a/processing/src/test/java/org/apache/druid/segment/ClusteredSegmentTimeOrderedQueryTest.java
b/processing/src/test/java/org/apache/druid/segment/ClusteredSegmentTimeOrderedQueryTest.java
index d5fa466bf2e..61b4002bc37 100644
---
a/processing/src/test/java/org/apache/druid/segment/ClusteredSegmentTimeOrderedQueryTest.java
+++
b/processing/src/test/java/org/apache/druid/segment/ClusteredSegmentTimeOrderedQueryTest.java
@@ -38,6 +38,8 @@ import org.apache.druid.query.QueryRunnerTestHelper;
import org.apache.druid.query.Result;
import org.apache.druid.query.aggregation.LongSumAggregatorFactory;
import org.apache.druid.query.expression.TestExprMacroTable;
+import org.apache.druid.query.filter.DimFilter;
+import org.apache.druid.query.filter.EqualityFilter;
import org.apache.druid.query.scan.ScanQuery;
import org.apache.druid.query.scan.ScanQueryConfig;
import org.apache.druid.query.scan.ScanQueryEngine;
@@ -65,6 +67,7 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
+import javax.annotation.Nullable;
import java.io.File;
import java.util.ArrayList;
import java.util.Arrays;
@@ -244,6 +247,21 @@ class ClusteredSegmentTimeOrderedQueryTest extends
InitializedNullHandlingTest
Assertions.assertEquals(expected,
runScanWithShadowingTenantVc(clusteredSegment, Order.ASCENDING));
}
+ @Test
+ void testTimeOrderedScanPruningAllGroupsReturnsEmpty()
+ {
+ // A filter on the clustering column that matches no group prunes every
cluster group, so the clustered cursor
+ // factory returns an empty holder. The scan engine hard-enforces cursor
time ordering, so the empty holder must
+ // advertise the requested __time direction (asc or desc) or the scan
explodes instead of returning zero rows.
+ // Covers both the historical (QueryableIndexCursorFactory) and realtime
(IncrementalIndexCursorFactory) paths.
+ final DimFilter noGroup = new EqualityFilter("tenant", ColumnType.STRING,
"nobody", null);
+ final Segment incremental = buildClusteredIncremental(ROWS);
+ for (final Segment segment : List.of(clusteredSegment, incremental)) {
+ Assertions.assertEquals(List.of(), runScanRows(segment, Order.ASCENDING,
noGroup));
+ Assertions.assertEquals(List.of(), runScanRows(segment,
Order.DESCENDING, noGroup));
+ }
+ }
+
@Test
void testTimeOrderedQueriesOverIncrementalClusteredSegment()
{
@@ -310,13 +328,21 @@ class ClusteredSegmentTimeOrderedQueryTest extends
InitializedNullHandlingTest
private static List<List<Object>> runScanRows(Segment segment, Order order)
{
- final ScanQuery query = Druids.newScanQueryBuilder()
- .dataSource(DATA_SOURCE)
- .intervals(new
MultipleIntervalSegmentSpec(List.of(INTERVAL)))
- .columns(ColumnHolder.TIME_COLUMN_NAME,
"tenant", "m")
- .order(order)
-
.resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_LIST)
- .build();
+ return runScanRows(segment, order, null);
+ }
+
+ private static List<List<Object>> runScanRows(Segment segment, Order order,
@Nullable DimFilter filter)
+ {
+ final Druids.ScanQueryBuilder builder = Druids.newScanQueryBuilder()
+ .dataSource(DATA_SOURCE)
+ .intervals(new
MultipleIntervalSegmentSpec(List.of(INTERVAL)))
+
.columns(ColumnHolder.TIME_COLUMN_NAME, "tenant", "m")
+ .order(order)
+
.resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_LIST);
+ if (filter != null) {
+ builder.filters(filter);
+ }
+ final ScanQuery query = builder.build();
final ScanQueryRunnerFactory factory = new ScanQueryRunnerFactory(
new
ScanQueryQueryToolChest(DefaultGenericQueryMetricsFactory.instance()),
new ScanQueryEngine(),
diff --git
a/processing/src/test/java/org/apache/druid/segment/EmptyCursorHolderTest.java
b/processing/src/test/java/org/apache/druid/segment/EmptyCursorHolderTest.java
new file mode 100644
index 00000000000..7932cadcca8
--- /dev/null
+++
b/processing/src/test/java/org/apache/druid/segment/EmptyCursorHolderTest.java
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.segment;
+
+import org.apache.druid.query.OrderBy;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+class EmptyCursorHolderTest
+{
+ private static final CursorBuildSpec ASC = CursorBuildSpec.builder()
+
.setPreferredOrdering(Cursors.ascendingTimeOrder())
+ .build();
+ private static final CursorBuildSpec DESC = CursorBuildSpec.builder()
+
.setPreferredOrdering(Cursors.descendingTimeOrder())
+ .build();
+
+ @Test
+ void testForTimeOrderAscendingAdvertisesAscendingTime()
+ {
+ CursorHolder holder = EmptyCursorHolder.forSpec(ASC);
+ Assertions.assertEquals(
+ Cursors.ascendingTimeOrder(),
+ holder.getOrdering()
+ );
+ Assertions.assertSame(EmptyCursorHolder.forSpec(ASC), holder);
+ }
+
+ @Test
+ void testForTimeOrderDescendingAdvertisesDescendingTime()
+ {
+ CursorHolder holder = EmptyCursorHolder.forSpec(DESC);
+ Assertions.assertEquals(
+ Cursors.descendingTimeOrder(),
+ holder.getOrdering()
+ );
+ Assertions.assertSame(EmptyCursorHolder.forSpec(DESC), holder);
+ }
+
+ @Test
+ void testForTimeOrderNoneReturnsUnorderedInstance()
+ {
+ CursorHolder holder = EmptyCursorHolder.forSpec(CursorBuildSpec.FULL_SCAN);
+ Assertions.assertEquals(
+ List.of(),
+ holder.getOrdering()
+ );
+
Assertions.assertSame(EmptyCursorHolder.forSpec(CursorBuildSpec.FULL_SCAN),
holder);
+ }
+
+ @Test
+ void testForSpecNonTimeOrderingAdvertisedVerbatim()
+ {
+ // preferredOrdering decouples from assumed __time ordering; an empty
cursor trivially satisfies any ordering, so
+ // forSpec advertises a non-__time preferred ordering verbatim via a
fresh, uncached instance.
+ final List<OrderBy> ordering = List.of(OrderBy.ascending("dim"));
+ final CursorBuildSpec spec =
CursorBuildSpec.builder().setPreferredOrdering(ordering).build();
+ final CursorHolder holder = EmptyCursorHolder.forSpec(spec);
+ Assertions.assertEquals(ordering, holder.getOrdering());
+ // Not the unordered singleton, and arbitrary orderings are not cached (a
fresh instance each call).
+ Assertions.assertNotSame(holder,
EmptyCursorHolder.forSpec(CursorBuildSpec.FULL_SCAN));
+ Assertions.assertNotSame(holder, EmptyCursorHolder.forSpec(spec));
+ }
+
+ @Test
+ void testCursorIsAlwaysDone()
+ {
+ Assertions.assertTrue(EmptyCursorHolder.forSpec(ASC).asCursor().isDone());
+ Assertions.assertTrue(EmptyCursorHolder.forSpec(DESC).asCursor().isDone());
+
Assertions.assertTrue(EmptyCursorHolder.forSpec(CursorBuildSpec.FULL_SCAN).asCursor().isDone());
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
