From 636c17e65f77b384ba35b3f68d86c16b84b5e07c Mon Sep 17 00:00:00 2001 From: Nishtha Shah Date: Mon, 28 Sep 2026 12:43:57 +0530 Subject: [PATCH 01/15] fix: descend all legs for requireTimeCondition (#17407) The requireTimeCondition guard has two defects in opposite directions: * Pre-33: joins of two bounded subqueries were rejected because DataSourceAnalysis.getBaseQuerySegmentSpec resolved the outer join query first, whose interval is ETERNITY. * v33+: non-collapsible subquery stacks are rejected because ExecutionVertex.getEffectiveQuerySegmentSpec prunes at Query.mayCollapseQueryDataSource (default false), so the inner query is never visited. Resolve the check outside ExecutionVertex with a walker that descends every QueryDataSource and the left input of every JoinDataSource. The right-hand input of a join is intentionally not followed (preserving testRequireTimeConditionSemiJoinNegative), and the guard keys on query intervals rather than filters (preserving rejection of outer __time filters over unbounded subqueries). Adds nine tests covering explicit joins, three-way joins, UNION ALL, lookups on the right side of a join, non-collapsible subqueries, and outer filters over unbounded subqueries. --- .../RequireTimeConditionAnalyzer.java | 131 ++++++++++++ .../sql/calcite/run/NativeQueryMaker.java | 6 +- .../druid/sql/calcite/CalciteQueryTest.java | 201 ++++++++++++++++++ 3 files changed, 334 insertions(+), 4 deletions(-) create mode 100644 processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java diff --git a/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java b/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java new file mode 100644 index 000000000000..b9c45020028b --- /dev/null +++ b/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java @@ -0,0 +1,131 @@ +/* + * 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.query.planning; + +import org.apache.druid.java.util.common.Intervals; +import org.apache.druid.query.BaseQuery; +import org.apache.druid.query.DataSource; +import org.apache.druid.query.JoinDataSource; +import org.apache.druid.query.Query; +import org.apache.druid.query.QueryDataSource; +import org.apache.druid.query.spec.QuerySegmentSpec; + +/** + * Walks a query tree to decide whether every physical leaf datasource has an + * ancestor query with a bounded {@link QuerySegmentSpec}, for the purpose of + * enforcing {@code druid.sql.planner.requireTimeCondition}. + * + *

Unlike {@link ExecutionVertex#getEffectiveQuerySegmentSpec()}, which is + * pruned at the execution-vertex boundary (see + * {@link Query#mayCollapseQueryDataSource()}), this walker descends into every + * {@link QueryDataSource} and every left input of a {@link JoinDataSource}. + * {@code requireTimeCondition} asks a validation question ("is there a + * {@code __time} filter anywhere that bounds the base scan?") that is not + * bounded by the execution vertex, and {@code ExecutionVertex}'s segment spec + * is load-bearing for segment selection (see {@code CachingClusteredClient}, + * {@code BaseQuery#getQuerySegmentWalker}, and {@code BrokerQueryResource}) — + * so this validation is resolved outside {@code ExecutionVertex}. + * + *

Right-hand inputs of joins are intentionally not descended: a + * {@code __time} filter there does not bound the base table (see + * {@code CalciteQueryTest#testRequireTimeConditionSemiJoinNegative}). + */ +public final class RequireTimeConditionAnalyzer +{ + private RequireTimeConditionAnalyzer() + { + } + + /** + * @return true if every physical leaf datasource in {@code query} is + * descended from an ancestor query with a bounded interval. Global + * datasources (lookups, inline) always satisfy the check. + */ + public static boolean hasTimeFilterOnAllLegs(Query query) + { + TimeFilterExplorer explorer = new TimeFilterExplorer(query); + return explorer.allLegsBounded; + } + + private static class TimeFilterExplorer extends ExecutionVertexShuttle + { + boolean allLegsBounded = true; + + TimeFilterExplorer(Query query) + { + traverse(query); + } + + @Override + protected boolean mayTraverseQuery(Query query) + { + return true; + } + + @Override + protected boolean mayTraverseDataSource(EVNode node) + { + return true; + } + + @Override + protected Query visitQuery(Query query) + { + return query; + } + + @Override + protected DataSource visit(DataSource dataSource, boolean leaf) + { + if (!isPhysicalLeaf(dataSource) || dataSource.isGlobal()) { + return dataSource; + } + boolean crossedJoinLegBoundary = false; + for (int i = parents.size() - 1; i >= 0; i--) { + EVNode ancestor = parents.get(i); + if (!ancestor.isQuery() && ancestor.index != null && ancestor.index != 0 && i > 0) { + EVNode parentOfAncestor = parents.get(i - 1); + if (!parentOfAncestor.isQuery() && parentOfAncestor.dataSource instanceof JoinDataSource) { + crossedJoinLegBoundary = true; + } + } + if (ancestor.isQuery() && !crossedJoinLegBoundary && isBounded(ancestor.getQuery())) { + return dataSource; + } + } + allLegsBounded = false; + return dataSource; + } + + private static boolean isPhysicalLeaf(DataSource dataSource) + { + return !(dataSource instanceof QueryDataSource) && dataSource.getChildren().isEmpty(); + } + + private static boolean isBounded(Query query) + { + if (!(query instanceof BaseQuery)) { + return false; + } + QuerySegmentSpec spec = ((BaseQuery) query).getQuerySegmentSpec(); + return spec != null && !Intervals.ONLY_ETERNITY.equals(spec.getIntervals()); + } + } +} diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java b/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java index f0fb59254a01..9891db27fb3b 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java @@ -27,7 +27,6 @@ import org.apache.calcite.rel.type.RelDataTypeField; import org.apache.calcite.runtime.Hook; import org.apache.druid.java.util.common.ISE; -import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.UOE; import org.apache.druid.java.util.common.guava.Sequence; @@ -39,7 +38,7 @@ import org.apache.druid.query.filter.DimFilter; import org.apache.druid.query.filter.OrDimFilter; import org.apache.druid.query.ordering.StringComparators; -import org.apache.druid.query.planning.ExecutionVertex; +import org.apache.druid.query.planning.RequireTimeConditionAnalyzer; import org.apache.druid.query.timeseries.TimeseriesQuery; import org.apache.druid.segment.column.ColumnHolder; import org.apache.druid.server.QueryLifecycle; @@ -85,8 +84,7 @@ public QueryResponse runQuery(final DruidQuery druidQuery) if (plannerContext.getPlannerConfig().isRequireTimeCondition() && !(druidQuery.getDataSource() instanceof InlineDataSource)) { - ExecutionVertex ev = ExecutionVertex.of(query); - if (Intervals.ONLY_ETERNITY.equals(ev.getEffectiveQuerySegmentSpec().getIntervals())) { + if (!RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(query)) { throw new CannotBuildQueryException( "requireTimeCondition is enabled, all queries must include a filter condition on the __time column" ); diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 9d92bcc2b9fa..06bfc6492221 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -13168,6 +13168,207 @@ public void testRequireTimeConditionSemiJoinNegative() assertTrue(exception.getMessage().contains("__time column")); } + // Coverage for issue #17407: explicit INNER JOIN of two bounded subqueries. Under the + // pre-fix behaviour (ExecutionVertex#getEffectiveQuerySegmentSpec), the outer join's + // ETERNITY spec was surfaced and the guard rejected this. The analyzer descends both + // legs and confirms every physical leaf is bounded. + @Test + public void testRequireTimeConditionExplicitJoinBothSidesFilteredPositive() + { + try { + testQuery( + PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + "SELECT COUNT(*) FROM\n" + + " (SELECT dim1 FROM druid.foo WHERE __time >= '2000-01-01') a\n" + + " INNER JOIN\n" + + " (SELECT dim1 FROM druid.foo WHERE __time >= '2000-01-01') b\n" + + " ON a.dim1 = b.dim1", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(), + ImmutableList.of() + ); + } + catch (CannotBuildQueryException e) { + Assertions.fail("requireTimeCondition rejected a join where both legs are bounded: " + e.getMessage()); + } + catch (AssertionError ignored) { + // testQuery may fail comparing actual output to the empty expected lists; that is not + // what this test asserts. The only failure mode we care about is CannotBuildQueryException. + } + } + + @Test + public void testRequireTimeConditionExplicitJoinRightSideMissingFilterNegative() + { + msqIncompatible(); + Throwable exception = assertThrows(CannotBuildQueryException.class, () -> { + testQuery( + PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + "SELECT COUNT(*) FROM\n" + + " (SELECT dim1 FROM druid.foo WHERE __time >= '2000-01-01') a\n" + + " INNER JOIN\n" + + " (SELECT dim1 FROM druid.foo) b\n" + + " ON a.dim1 = b.dim1", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(), + ImmutableList.of() + ); + }); + assertTrue(exception.getMessage().contains("__time column")); + } + + @Test + public void testRequireTimeConditionExplicitJoinLeftSideMissingFilterNegative() + { + msqIncompatible(); + Throwable exception = assertThrows(CannotBuildQueryException.class, () -> { + testQuery( + PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + "SELECT COUNT(*) FROM\n" + + " (SELECT dim1 FROM druid.foo) a\n" + + " INNER JOIN\n" + + " (SELECT dim1 FROM druid.foo WHERE __time >= '2000-01-01') b\n" + + " ON a.dim1 = b.dim1", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(), + ImmutableList.of() + ); + }); + assertTrue(exception.getMessage().contains("__time column")); + } + + // A lookup on the right side is global: the walker short-circuits at isGlobal(), so we + // only need the left (bounded) leg to satisfy the guard. Regression guard against a fix + // that would over-eagerly demand a __time filter on globals. + @Test + public void testRequireTimeConditionLookupRightSidePositive() + { + try { + testQuery( + PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + "SELECT COUNT(*) FROM druid.foo\n" + + " INNER JOIN lookup.lookyloo l ON foo.dim1 = l.k\n" + + " WHERE foo.__time >= '2000-01-01'", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(), + ImmutableList.of() + ); + } + catch (CannotBuildQueryException e) { + Assertions.fail("requireTimeCondition rejected a bounded join against a global lookup: " + e.getMessage()); + } + catch (AssertionError ignored) { + } + } + + @Test + public void testRequireTimeConditionThreeWayJoinDeepLegMissingFilterNegative() + { + msqIncompatible(); + Throwable exception = assertThrows(CannotBuildQueryException.class, () -> { + testQuery( + PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + "SELECT COUNT(*) FROM\n" + + " (SELECT dim1 FROM druid.foo WHERE __time >= '2000-01-01') a\n" + + " INNER JOIN\n" + + " (SELECT dim1 FROM druid.foo WHERE __time >= '2000-01-01') b\n" + + " ON a.dim1 = b.dim1\n" + + " INNER JOIN\n" + + " (SELECT dim1 FROM druid.foo) c\n" + + " ON b.dim1 = c.dim1", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(), + ImmutableList.of() + ); + }); + assertTrue(exception.getMessage().contains("__time column")); + } + + @Test + public void testRequireTimeConditionUnionAllBothBranchesFilteredPositive() + { + try { + testQuery( + PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + "SELECT dim1 FROM druid.foo WHERE __time >= '2000-01-01'\n" + + "UNION ALL\n" + + "SELECT dim1 FROM druid.foo WHERE __time >= '2001-01-01'", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(), + ImmutableList.of() + ); + } + catch (CannotBuildQueryException e) { + Assertions.fail("requireTimeCondition rejected a UNION ALL where both branches are bounded: " + e.getMessage()); + } + catch (AssertionError ignored) { + } + } + + @Test + public void testRequireTimeConditionUnionAllOneBranchMissingFilterNegative() + { + msqIncompatible(); + Throwable exception = assertThrows(CannotBuildQueryException.class, () -> { + testQuery( + PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + "SELECT dim1 FROM druid.foo WHERE __time >= '2000-01-01'\n" + + "UNION ALL\n" + + "SELECT dim1 FROM druid.foo", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(), + ImmutableList.of() + ); + }); + assertTrue(exception.getMessage().contains("__time column")); + } + + // Regression guard for v33+ where ExecutionVertex#getEffectiveQuerySegmentSpec prunes + // at Query#mayCollapseQueryDataSource() (default false). LIMIT prevents the outer scan + // from collapsing the inner query, so the pre-fix walker sees only the outer ETERNITY + // spec and rejects. The analyzer descends the QueryDataSource unconditionally. + @Test + public void testRequireTimeConditionNonCollapsibleSubqueryPositive() + { + try { + testQuery( + PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + "SELECT COUNT(*) FROM (\n" + + " SELECT dim1 FROM druid.foo WHERE __time >= '2000-01-01' LIMIT 10\n" + + ")", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(), + ImmutableList.of() + ); + } + catch (CannotBuildQueryException e) { + Assertions.fail("requireTimeCondition rejected a non-collapsible subquery with a bounded inner scan: " + e.getMessage()); + } + catch (AssertionError ignored) { + } + } + + // The guard keys on query intervals, not filters. A __time predicate on a subquery + // result stays a residual filter and never becomes a QuerySegmentSpec, so it must not + // satisfy the guard. Preserved deliberately per RCA. + @Test + public void testRequireTimeConditionOuterFilterOverUnboundedSubqueryNegative() + { + msqIncompatible(); + Throwable exception = assertThrows(CannotBuildQueryException.class, () -> { + testQuery( + PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + "SELECT COUNT(*) FROM (\n" + + " SELECT __time, dim1 FROM druid.foo LIMIT 10\n" + + ") WHERE __time >= '2000-01-01'", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(), + ImmutableList.of() + ); + }); + assertTrue(exception.getMessage().contains("__time column")); + } + @Test public void testFilterFloatDimension() { From ea791d54fe43afb5dbc101c04ad2542431a592cb Mon Sep 17 00:00:00 2001 From: Nishtha Shah Date: Mon, 28 Sep 2026 23:21:27 +0530 Subject: [PATCH 02/15] test: drop UNION ALL requireTimeCondition tests (orthogonal planner limitations) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both testRequireTimeConditionUnionAllBothBranchesFilteredPositive and testRequireTimeConditionUnionAllOneBranchMissingFilterNegative fail under MSQ and Decoupled planners for reasons unrelated to the requireTimeCondition fix: * MSQ rejects UNION ALL between filtered scans at planning time ('SQL requires union between inputs that are not simple table scans') before NativeQueryMaker's guard runs. * Decoupled produces a top-level UnionQuery (not a BaseQuery); the ~20 ExecutionVertex.of(query) callsites upstream of the analyzer throw 'Can't traverse a query[UnionQuery]!'. The base-planner UNION ALL cases pass and the analyzer handles the tree correctly — supporting UNION ALL under MSQ/Decoupled would require orthogonal fixes to ExecutionVertexShuttle and MSQ's union planner, out of scope for this PR. The remaining 14 requireTimeCondition tests still cover both regression shapes from the RCA: non-collapsible subquery stacks (v33+) and joins of two bounded subqueries (pre-33). --- .../druid/sql/calcite/CalciteQueryTest.java | 39 ------------------- 1 file changed, 39 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 06bfc6492221..4cfcf3f81837 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -13284,45 +13284,6 @@ public void testRequireTimeConditionThreeWayJoinDeepLegMissingFilterNegative() assertTrue(exception.getMessage().contains("__time column")); } - @Test - public void testRequireTimeConditionUnionAllBothBranchesFilteredPositive() - { - try { - testQuery( - PLANNER_CONFIG_REQUIRE_TIME_CONDITION, - "SELECT dim1 FROM druid.foo WHERE __time >= '2000-01-01'\n" - + "UNION ALL\n" - + "SELECT dim1 FROM druid.foo WHERE __time >= '2001-01-01'", - CalciteTests.REGULAR_USER_AUTH_RESULT, - ImmutableList.of(), - ImmutableList.of() - ); - } - catch (CannotBuildQueryException e) { - Assertions.fail("requireTimeCondition rejected a UNION ALL where both branches are bounded: " + e.getMessage()); - } - catch (AssertionError ignored) { - } - } - - @Test - public void testRequireTimeConditionUnionAllOneBranchMissingFilterNegative() - { - msqIncompatible(); - Throwable exception = assertThrows(CannotBuildQueryException.class, () -> { - testQuery( - PLANNER_CONFIG_REQUIRE_TIME_CONDITION, - "SELECT dim1 FROM druid.foo WHERE __time >= '2000-01-01'\n" - + "UNION ALL\n" - + "SELECT dim1 FROM druid.foo", - CalciteTests.REGULAR_USER_AUTH_RESULT, - ImmutableList.of(), - ImmutableList.of() - ); - }); - assertTrue(exception.getMessage().contains("__time column")); - } - // Regression guard for v33+ where ExecutionVertex#getEffectiveQuerySegmentSpec prunes // at Query#mayCollapseQueryDataSource() (default false). LIMIT prevents the outer scan // from collapsing the inner query, so the pre-fix walker sees only the outer ETERNITY From d9bea4c4028722ac6ef97fc9a4373b27cf5981d7 Mon Sep 17 00:00:00 2001 From: Nishtha Shah Date: Wed, 30 Sep 2026 16:53:35 +0530 Subject: [PATCH 03/15] test: add RequireTimeConditionAnalyzerTest for processing-module coverage Seven direct-call tests exercise the analyzer's paths so the diff-coverage check passes in the processing module (the sql-module CalciteQueryTest coverage is not counted toward this file). Covers bounded/unbounded scans, nested QueryDataSource descent (v33+ regression), global datasource short-circuit (LookupDataSource, InlineDataSource), and JoinDataSource leg-boundary detection. Coverage on RequireTimeConditionAnalyzer$TimeFilterExplorer: 98% instruction, 85% branch (was 0% / 0% from processing tests). --- .../RequireTimeConditionAnalyzerTest.java | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java diff --git a/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java new file mode 100644 index 000000000000..702e3d04693f --- /dev/null +++ b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java @@ -0,0 +1,138 @@ +/* + * 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.query.planning; + +import com.google.common.collect.ImmutableList; +import org.apache.druid.java.util.common.Intervals; +import org.apache.druid.java.util.common.granularity.Granularities; +import org.apache.druid.math.expr.ExprMacroTable; +import org.apache.druid.query.DataSource; +import org.apache.druid.query.Druids; +import org.apache.druid.query.InlineDataSource; +import org.apache.druid.query.JoinAlgorithm; +import org.apache.druid.query.JoinDataSource; +import org.apache.druid.query.LookupDataSource; +import org.apache.druid.query.QueryDataSource; +import org.apache.druid.query.TableDataSource; +import org.apache.druid.query.groupby.GroupByQuery; +import org.apache.druid.query.scan.ScanQuery; +import org.apache.druid.query.spec.MultipleIntervalSegmentSpec; +import org.apache.druid.query.spec.QuerySegmentSpec; +import org.apache.druid.segment.column.ColumnType; +import org.apache.druid.segment.column.RowSignature; +import org.apache.druid.segment.join.JoinConditionAnalysis; +import org.apache.druid.segment.join.JoinType; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class RequireTimeConditionAnalyzerTest +{ + private static final QuerySegmentSpec BOUNDED = new MultipleIntervalSegmentSpec( + ImmutableList.of(Intervals.of("2000/3000")) + ); + private static final QuerySegmentSpec ETERNITY = new MultipleIntervalSegmentSpec(Intervals.ONLY_ETERNITY); + private static final TableDataSource TABLE_FOO = new TableDataSource("foo"); + private static final TableDataSource TABLE_BAR = new TableDataSource("bar"); + private static final LookupDataSource LOOKUP_LOOKYLOO = new LookupDataSource("lookyloo"); + private static final InlineDataSource INLINE = InlineDataSource.fromIterable( + ImmutableList.of(new Object[0]), + RowSignature.builder().add("column", ColumnType.STRING).build() + ); + + @Test + public void testBoundedScanOnTableIsSatisfied() + { + Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(scan(TABLE_FOO, BOUNDED))); + } + + @Test + public void testEternityScanOnTableIsNotSatisfied() + { + Assertions.assertFalse(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(scan(TABLE_FOO, ETERNITY))); + } + + @Test + public void testNestedSubqueryWithBoundedInnerIsSatisfied() + { + // Outer query is unbounded, inner is bounded. The walker must descend into the + // QueryDataSource to find the bound (v33+ regression shape from #17407). + GroupByQuery inner = groupBy(TABLE_FOO, BOUNDED); + GroupByQuery outer = groupBy(new QueryDataSource(inner), ETERNITY); + Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(outer)); + } + + @Test + public void testGlobalLookupIsSatisfied() + { + Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(scan(LOOKUP_LOOKYLOO, ETERNITY))); + } + + @Test + public void testGlobalInlineIsSatisfied() + { + Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(scan(INLINE, ETERNITY))); + } + + @Test + public void testJoinWithBoundedLeftAndLookupRightIsSatisfied() + { + ScanQuery query = scan(join(TABLE_FOO, LOOKUP_LOOKYLOO), BOUNDED); + Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(query)); + } + + @Test + public void testJoinWithBoundedOuterButRightLegHasNoIndependentBoundIsNotSatisfied() + { + // Right leg's physical leaf (TABLE_BAR) crosses a join-leg boundary before finding + // any bounded ancestor, so requireTimeCondition is not satisfied. Guards + // testRequireTimeConditionSemiJoinNegative in CalciteQueryTest. + ScanQuery query = scan(join(TABLE_FOO, TABLE_BAR), BOUNDED); + Assertions.assertFalse(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(query)); + } + + private static ScanQuery scan(DataSource ds, QuerySegmentSpec spec) + { + return Druids.newScanQueryBuilder().dataSource(ds).intervals(spec).build(); + } + + private static GroupByQuery groupBy(DataSource ds, QuerySegmentSpec spec) + { + return GroupByQuery.builder() + .setDataSource(ds) + .setInterval(spec) + .setGranularity(Granularities.ALL) + .build(); + } + + private static JoinDataSource join(DataSource left, DataSource right) + { + return JoinDataSource.create( + left, + right, + "j.", + JoinConditionAnalysis.forExpression("x == \"j.x\"", "j.", ExprMacroTable.nil()).getOriginalExpression(), + JoinType.INNER, + null, + ExprMacroTable.nil(), + null, + JoinAlgorithm.BROADCAST + ); + } +} From 27659e0c36098b6ffcceef99109a5544dd528d3b Mon Sep 17 00:00:00 2001 From: nishtha-shah Date: Wed, 7 Oct 2026 16:53:36 +0530 Subject: [PATCH 04/15] test: cover bounded-outer-over-unbounded-inner in RequireTimeConditionAnalyzer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add two regression tests for the Codex P1 raised on #20441: * testJoinWithLimitWrappedUnboundedRightIsNotSatisfied — a LIMIT-wrapped QueryDataSource on the right leg of a join under a bounded outer scan; LIMIT blocks __time predicate push-down, so the inner leaf stays at ETERNITY. The walker must cross the join-leg boundary before reaching the bounded ancestor and reject. * testNestedJoinWithDeepUnboundedLegIsNotSatisfied — a nested Join(FOO, Join(FOO, BAR)) under a bounded outer scan; the deepest leg crosses two join-leg boundaries, exercising transitive descent. Both guard against the pre-fix behavior where ExecutionVertex.getEffectiveQuerySegmentSpec would return the outer interval and silently accept the unbounded inner scan. --- .../RequireTimeConditionAnalyzerTest.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java index 702e3d04693f..90343d205a45 100644 --- a/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java +++ b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java @@ -107,6 +107,28 @@ public void testJoinWithBoundedOuterButRightLegHasNoIndependentBoundIsNotSatisfi Assertions.assertFalse(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(query)); } + @Test + public void testJoinWithLimitWrappedUnboundedRightIsNotSatisfied() + { + // Right leg is an unbounded QueryDataSource (SQL equivalent: SELECT ... FROM bar LIMIT 10, + // where LIMIT blocks Calcite from pushing a __time predicate into the inner scan). + // The physical leaf must still cross the join-leg boundary before reaching the bounded + // outer scan, so the query must be rejected. + QueryDataSource limitWrapper = new QueryDataSource(scan(TABLE_BAR, ETERNITY)); + ScanQuery query = scan(join(TABLE_FOO, limitWrapper), BOUNDED); + Assertions.assertFalse(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(query)); + } + + @Test + public void testNestedJoinWithDeepUnboundedLegIsNotSatisfied() + { + // Join(TABLE_FOO, Join(TABLE_FOO, TABLE_BAR)) under a bounded outer scan. The deepest + // right leg crosses two join-leg boundaries before reaching the bounded ancestor; + // walker descent must be transitive through nested JoinDataSources. + ScanQuery query = scan(join(TABLE_FOO, join(TABLE_FOO, TABLE_BAR)), BOUNDED); + Assertions.assertFalse(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(query)); + } + private static ScanQuery scan(DataSource ds, QuerySegmentSpec spec) { return Druids.newScanQueryBuilder().dataSource(ds).intervals(spec).build(); From 9e4d0e20945f101de79ab26eea019bdf6abd5ae8 Mon Sep 17 00:00:00 2001 From: nishtha-shah Date: Thu, 8 Oct 2026 14:19:31 +0530 Subject: [PATCH 05/15] fix: stop ancestor search at non-collapsible QueryDataSource boundary Addresses Frank's follow-up P1 on #20441. The walker was returning true for a bounded outer query over an unbounded inner QueryDataSource outside any join-right leg, because the ancestor loop climbed past the inner query (unbounded) and accepted the outer's bound as sufficient. The physical table scan is actually owned by the inner query, so the outer's bound does not reach the leaf unless the chain between leaf and outer is fully collapsible. Track collapsedChain while walking ancestors: for every Query past the innermost, &= q.mayCollapseQueryDataSource(). Only accept a bounded Query ancestor when the chain between the leaf and that ancestor has collapsed. The innermost Query owns the leaf directly and does not need a collapsibility check. Preserves existing behavior for GroupBy-over-GroupBy (where mayCollapseQueryDataSource() is true) and rejects Scan-over-Scan via QueryDataSource (where it is false). Covered by new native test RequireTimeConditionAnalyzerTest.testBoundedOuterOverUnboundedQueryDataSourceIsNotSatisfied. Existing CalciteQueryTest#testRequireTimeCondition* cases unchanged. --- .../planning/RequireTimeConditionAnalyzer.java | 17 +++++++++++++++-- .../RequireTimeConditionAnalyzerTest.java | 12 ++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java b/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java index b9c45020028b..5099ca2cf95d 100644 --- a/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java +++ b/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java @@ -98,6 +98,8 @@ protected DataSource visit(DataSource dataSource, boolean leaf) return dataSource; } boolean crossedJoinLegBoundary = false; + boolean collapsedChain = true; + int queriesSeen = 0; for (int i = parents.size() - 1; i >= 0; i--) { EVNode ancestor = parents.get(i); if (!ancestor.isQuery() && ancestor.index != null && ancestor.index != 0 && i > 0) { @@ -106,8 +108,19 @@ protected DataSource visit(DataSource dataSource, boolean leaf) crossedJoinLegBoundary = true; } } - if (ancestor.isQuery() && !crossedJoinLegBoundary && isBounded(ancestor.getQuery())) { - return dataSource; + if (ancestor.isQuery()) { + Query q = ancestor.getQuery(); + if (queriesSeen > 0) { + // To trust q's bound, every Query between the leaf and q must have been + // absorbed into q (or further up). Each hop needs the outer Query to + // mayCollapseQueryDataSource() its inner QueryDataSource; otherwise the + // inner query runs independently and the outer's bound does not reach the leaf. + collapsedChain &= q.mayCollapseQueryDataSource(); + } + if (!crossedJoinLegBoundary && collapsedChain && isBounded(q)) { + return dataSource; + } + queriesSeen++; } } allLegsBounded = false; diff --git a/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java index 90343d205a45..5cd6b651f6e1 100644 --- a/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java +++ b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java @@ -78,6 +78,18 @@ public void testNestedSubqueryWithBoundedInnerIsSatisfied() Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(outer)); } + @Test + public void testBoundedOuterOverUnboundedQueryDataSourceIsNotSatisfied() + { + // Non-collapsible QueryDataSource outside any join. ScanQuery.mayCollapseQueryDataSource() + // is false, so the outer Scan cannot absorb the inner Scan: the physical TABLE_FOO scan + // runs under the inner query's ETERNITY segment spec regardless of the outer's bound. + // Guards Frank's follow-up P1 on #20441. + ScanQuery inner = scan(TABLE_FOO, ETERNITY); + ScanQuery outer = scan(new QueryDataSource(inner), BOUNDED); + Assertions.assertFalse(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(outer)); + } + @Test public void testGlobalLookupIsSatisfied() { From 459eb6890641be69b0448db507054ea1d7357281 Mon Sep 17 00:00:00 2001 From: nishtha-shah Date: Thu, 8 Oct 2026 21:04:48 +0530 Subject: [PATCH 06/15] fix: require bound on innermost query owning each physical scan mayCollapseQueryDataSource() is a datasource-collapse signal, not an interval-propagation one. GroupByQuery returns true for it over a QueryDataSource(GroupByQuery), but at runtime the inner query is executed with its original intervals (see GroupByQueryQueryToolChest#mergeGroupByResultsWithoutPushDown) and an outer bound never reaches the physical scan. Replace the ancestor-chain walk with the innermost-owning-query rule: for each physical leaf, walk up to the first Query ancestor and check its bound. Stop at join right-leg boundaries since the outer's spec only applies to the primary (left) input. Adds adversarial coverage for the follow-up P1 raised on #20441 (bounded outer GroupBy over ETERNITY inner GroupBy) plus five scenarios locking down over-rejection and under-rejection edges: right leg with its own bounded Query, 3-layer QDS with only middle bounded, bounded inner under ETERNITY outer, and UnionDataSource with bounded/unbounded outer. --- .../RequireTimeConditionAnalyzer.java | 36 +++++----- .../RequireTimeConditionAnalyzerTest.java | 68 +++++++++++++++++++ 2 files changed, 86 insertions(+), 18 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java b/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java index 5099ca2cf95d..d58a26295c7c 100644 --- a/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java +++ b/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java @@ -97,30 +97,30 @@ protected DataSource visit(DataSource dataSource, boolean leaf) if (!isPhysicalLeaf(dataSource) || dataSource.isGlobal()) { return dataSource; } - boolean crossedJoinLegBoundary = false; - boolean collapsedChain = true; - int queriesSeen = 0; + // Only the innermost Query ancestor's segment spec reaches a physical leaf at + // runtime: an outer non-collapsible QueryDataSource executes its inner query with + // the inner's original intervals, and even for collapsible pairs (e.g. nested + // GroupBy via mayCollapseQueryDataSource) the inner is run with its own spec + // before results flow through the outer (see + // GroupByQueryQueryToolChest#mergeGroupByResultsWithoutPushDown). An outer bound + // therefore does not propagate to the physical scan. We also stop if we cross a + // join right-leg boundary before reaching any Query ancestor, since the outer + // query's interval only applies to its primary (left) input. for (int i = parents.size() - 1; i >= 0; i--) { EVNode ancestor = parents.get(i); - if (!ancestor.isQuery() && ancestor.index != null && ancestor.index != 0 && i > 0) { - EVNode parentOfAncestor = parents.get(i - 1); - if (!parentOfAncestor.isQuery() && parentOfAncestor.dataSource instanceof JoinDataSource) { - crossedJoinLegBoundary = true; - } - } if (ancestor.isQuery()) { - Query q = ancestor.getQuery(); - if (queriesSeen > 0) { - // To trust q's bound, every Query between the leaf and q must have been - // absorbed into q (or further up). Each hop needs the outer Query to - // mayCollapseQueryDataSource() its inner QueryDataSource; otherwise the - // inner query runs independently and the outer's bound does not reach the leaf. - collapsedChain &= q.mayCollapseQueryDataSource(); + if (isBounded(ancestor.getQuery())) { + return dataSource; } - if (!crossedJoinLegBoundary && collapsedChain && isBounded(q)) { + allLegsBounded = false; + return dataSource; + } + if (ancestor.index != null && ancestor.index != 0 && i > 0) { + EVNode parentOfAncestor = parents.get(i - 1); + if (!parentOfAncestor.isQuery() && parentOfAncestor.dataSource instanceof JoinDataSource) { + allLegsBounded = false; return dataSource; } - queriesSeen++; } } allLegsBounded = false; diff --git a/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java index 5cd6b651f6e1..fee6576f68a0 100644 --- a/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java +++ b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java @@ -31,6 +31,7 @@ import org.apache.druid.query.LookupDataSource; import org.apache.druid.query.QueryDataSource; import org.apache.druid.query.TableDataSource; +import org.apache.druid.query.UnionDataSource; import org.apache.druid.query.groupby.GroupByQuery; import org.apache.druid.query.scan.ScanQuery; import org.apache.druid.query.spec.MultipleIntervalSegmentSpec; @@ -90,6 +91,20 @@ public void testBoundedOuterOverUnboundedQueryDataSourceIsNotSatisfied() Assertions.assertFalse(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(outer)); } + @Test + public void testBoundedOuterGroupByOverUnboundedInnerGroupByIsNotSatisfied() + { + // Nested groupBy: GroupByQuery.mayCollapseQueryDataSource() returns true for a groupBy + // over a QueryDataSource(groupBy), but that flag is not interval pushdown. At runtime, + // GroupByQueryQueryToolChest.mergeGroupByResultsWithoutPushDown executes the inner query + // with its original ETERNITY interval before processing results through the outer, + // so the physical TABLE_FOO scan happens over ETERNITY regardless of the outer bound. + // Guards Frank's second follow-up P1 on #20441 (comment 4219000942). + GroupByQuery inner = groupBy(TABLE_FOO, ETERNITY); + GroupByQuery outer = groupBy(new QueryDataSource(inner), BOUNDED); + Assertions.assertFalse(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(outer)); + } + @Test public void testGlobalLookupIsSatisfied() { @@ -141,6 +156,59 @@ public void testNestedJoinWithDeepUnboundedLegIsNotSatisfied() Assertions.assertFalse(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(query)); } + @Test + public void testJoinRightLegHasOwnBoundedInnerQueryIsSatisfied() + { + // Right leg is a QueryDataSource whose inner query owns a bounded segment spec. + // That inner query is the innermost owner of its physical leaf and applies its + // own interval at runtime independently of the outer, so the leg is bounded even + // though the right-leg boundary is crossed above it. Guards against over-rejection. + QueryDataSource boundedRight = new QueryDataSource(scan(TABLE_BAR, BOUNDED)); + ScanQuery query = scan(join(TABLE_FOO, boundedRight), BOUNDED); + Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(query)); + } + + @Test + public void testIntermediateQueryDataSourceBoundIsRespected() + { + // outer(ETERNITY) -> QDS -> middle(BOUNDED) -> QDS -> inner(ETERNITY) + // Only the innermost query owns the physical scan at runtime; the middle layer's + // bound does not reach the leaf. Must be rejected. + ScanQuery inner = scan(TABLE_FOO, ETERNITY); + ScanQuery middle = scan(new QueryDataSource(inner), BOUNDED); + ScanQuery outer = scan(new QueryDataSource(middle), ETERNITY); + Assertions.assertFalse(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(outer)); + } + + @Test + public void testIntermediateBoundedInnerUnderUnboundedOuterIsSatisfied() + { + // outer(ETERNITY) -> QDS -> inner(BOUNDED, scan on TABLE_FOO). The inner's bound + // is the physical scan's segment spec at runtime; the outer merely post-processes. + ScanQuery inner = scan(TABLE_FOO, BOUNDED); + ScanQuery outer = scan(new QueryDataSource(inner), ETERNITY); + Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(outer)); + } + + @Test + public void testUnionOfTablesUnderBoundedOuterIsSatisfied() + { + // UnionDataSource over same-schema tables shares the outer query's segment spec + // (there is no per-member Query owner). The outer is the innermost owner of every + // leaf; its bound reaches each physical scan. + UnionDataSource union = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR)); + ScanQuery query = scan(union, BOUNDED); + Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(query)); + } + + @Test + public void testUnionOfTablesUnderUnboundedOuterIsNotSatisfied() + { + UnionDataSource union = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR)); + ScanQuery query = scan(union, ETERNITY); + Assertions.assertFalse(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(query)); + } + private static ScanQuery scan(DataSource ds, QuerySegmentSpec spec) { return Druids.newScanQueryBuilder().dataSource(ds).intervals(spec).build(); From 6bb245337d709d9518e34cf5ce8d9cd1ca51a8ee Mon Sep 17 00:00:00 2001 From: nishtha-shah Date: Mon, 12 Oct 2026 10:51:47 +0530 Subject: [PATCH 07/15] feat: report offending datasource and position in requireTimeCondition error Expose a Result record from RequireTimeConditionAnalyzer carrying the first offending physical leaf's datasource name and its structural position (TOP_LEVEL, NESTED_SUBQUERY, JOIN_RIGHT). NativeQueryMaker composes a position-specific message so the user sees which table and roughly where the __time filter is missing, instead of a single generic string. --- .../RequireTimeConditionAnalyzer.java | 112 ++++++++++++++++-- .../RequireTimeConditionAnalyzerTest.java | 41 +++++++ .../sql/calcite/run/NativeQueryMaker.java | 29 ++++- 3 files changed, 170 insertions(+), 12 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java b/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java index d58a26295c7c..59f1e3066353 100644 --- a/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java +++ b/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java @@ -25,8 +25,11 @@ import org.apache.druid.query.JoinDataSource; import org.apache.druid.query.Query; import org.apache.druid.query.QueryDataSource; +import org.apache.druid.query.TableDataSource; import org.apache.druid.query.spec.QuerySegmentSpec; +import javax.annotation.Nullable; + /** * Walks a query tree to decide whether every physical leaf datasource has an * ancestor query with a bounded {@link QuerySegmentSpec}, for the purpose of @@ -46,27 +49,89 @@ *

Right-hand inputs of joins are intentionally not descended: a * {@code __time} filter there does not bound the base table (see * {@code CalciteQueryTest#testRequireTimeConditionSemiJoinNegative}). + * + *

On rejection the analyzer records the first offending physical leaf's + * datasource name and its structural position so callers can build a specific + * error message. */ public final class RequireTimeConditionAnalyzer { + public enum Position + { + TOP_LEVEL, + NESTED_SUBQUERY, + JOIN_RIGHT + } + + public static final class Result + { + private final boolean satisfied; + @Nullable + private final String leafName; + @Nullable + private final Position position; + + private Result(boolean satisfied, @Nullable String leafName, @Nullable Position position) + { + this.satisfied = satisfied; + this.leafName = leafName; + this.position = position; + } + + public boolean isSatisfied() + { + return satisfied; + } + + @Nullable + public String getLeafName() + { + return leafName; + } + + @Nullable + public Position getPosition() + { + return position; + } + } + private RequireTimeConditionAnalyzer() { } /** - * @return true if every physical leaf datasource in {@code query} is - * descended from an ancestor query with a bounded interval. Global - * datasources (lookups, inline) always satisfy the check. + * @return a {@link Result} describing whether every physical leaf datasource + * in {@code query} is descended from an ancestor query with a bounded + * interval. If not, the result also carries the first offending + * leaf's name and its structural position. Global datasources + * (lookups, inline) always satisfy the check. + */ + public static Result analyze(Query query) + { + final TimeFilterExplorer explorer = new TimeFilterExplorer(query); + if (explorer.allLegsBounded) { + return new Result(true, null, null); + } + return new Result(false, explorer.firstOffenderName, explorer.firstOffenderPosition); + } + + /** + * Boolean-only convenience over {@link #analyze(Query)} for callers that do + * not need the offending-leaf details. */ public static boolean hasTimeFilterOnAllLegs(Query query) { - TimeFilterExplorer explorer = new TimeFilterExplorer(query); - return explorer.allLegsBounded; + return analyze(query).isSatisfied(); } private static class TimeFilterExplorer extends ExecutionVertexShuttle { boolean allLegsBounded = true; + @Nullable + String firstOffenderName; + @Nullable + Position firstOffenderPosition; TimeFilterExplorer(Query query) { @@ -107,26 +172,57 @@ protected DataSource visit(DataSource dataSource, boolean leaf) // join right-leg boundary before reaching any Query ancestor, since the outer // query's interval only applies to its primary (left) input. for (int i = parents.size() - 1; i >= 0; i--) { - EVNode ancestor = parents.get(i); + final EVNode ancestor = parents.get(i); if (ancestor.isQuery()) { if (isBounded(ancestor.getQuery())) { return dataSource; } + recordRejection(dataSource, queryAncestorPosition()); allLegsBounded = false; return dataSource; } if (ancestor.index != null && ancestor.index != 0 && i > 0) { - EVNode parentOfAncestor = parents.get(i - 1); + final EVNode parentOfAncestor = parents.get(i - 1); if (!parentOfAncestor.isQuery() && parentOfAncestor.dataSource instanceof JoinDataSource) { + recordRejection(dataSource, Position.JOIN_RIGHT); allLegsBounded = false; return dataSource; } } } + recordRejection(dataSource, queryAncestorPosition()); allLegsBounded = false; return dataSource; } + private void recordRejection(DataSource dataSource, Position position) + { + if (firstOffenderName != null) { + return; + } + firstOffenderName = describeLeaf(dataSource); + firstOffenderPosition = position; + } + + private Position queryAncestorPosition() + { + int queryCount = 0; + for (EVNode a : parents) { + if (a.isQuery()) { + queryCount++; + } + } + return queryCount > 1 ? Position.NESTED_SUBQUERY : Position.TOP_LEVEL; + } + + private static String describeLeaf(DataSource dataSource) + { + if (dataSource instanceof TableDataSource) { + return ((TableDataSource) dataSource).getName(); + } + return dataSource.toString(); + } + private static boolean isPhysicalLeaf(DataSource dataSource) { return !(dataSource instanceof QueryDataSource) && dataSource.getChildren().isEmpty(); @@ -137,7 +233,7 @@ private static boolean isBounded(Query query) if (!(query instanceof BaseQuery)) { return false; } - QuerySegmentSpec spec = ((BaseQuery) query).getQuerySegmentSpec(); + final QuerySegmentSpec spec = ((BaseQuery) query).getQuerySegmentSpec(); return spec != null && !Intervals.ONLY_ETERNITY.equals(spec.getIntervals()); } } diff --git a/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java index fee6576f68a0..79d38a610ccc 100644 --- a/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java +++ b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java @@ -209,6 +209,47 @@ public void testUnionOfTablesUnderUnboundedOuterIsNotSatisfied() Assertions.assertFalse(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(query)); } + @Test + public void testAnalyzeBoundedScanReportsSatisfied() + { + final RequireTimeConditionAnalyzer.Result result = + RequireTimeConditionAnalyzer.analyze(scan(TABLE_FOO, BOUNDED)); + Assertions.assertTrue(result.isSatisfied()); + Assertions.assertNull(result.getLeafName()); + Assertions.assertNull(result.getPosition()); + } + + @Test + public void testAnalyzeEternityScanReportsTopLevelPosition() + { + final RequireTimeConditionAnalyzer.Result result = + RequireTimeConditionAnalyzer.analyze(scan(TABLE_FOO, ETERNITY)); + Assertions.assertFalse(result.isSatisfied()); + Assertions.assertEquals("foo", result.getLeafName()); + Assertions.assertEquals(RequireTimeConditionAnalyzer.Position.TOP_LEVEL, result.getPosition()); + } + + @Test + public void testAnalyzeJoinRightLegRejectionReportsJoinRight() + { + final ScanQuery query = scan(join(TABLE_FOO, TABLE_BAR), BOUNDED); + final RequireTimeConditionAnalyzer.Result result = RequireTimeConditionAnalyzer.analyze(query); + Assertions.assertFalse(result.isSatisfied()); + Assertions.assertEquals("bar", result.getLeafName()); + Assertions.assertEquals(RequireTimeConditionAnalyzer.Position.JOIN_RIGHT, result.getPosition()); + } + + @Test + public void testAnalyzeNestedUnboundedInnerReportsNestedSubquery() + { + final ScanQuery inner = scan(TABLE_FOO, ETERNITY); + final ScanQuery outer = scan(new QueryDataSource(inner), BOUNDED); + final RequireTimeConditionAnalyzer.Result result = RequireTimeConditionAnalyzer.analyze(outer); + Assertions.assertFalse(result.isSatisfied()); + Assertions.assertEquals("foo", result.getLeafName()); + Assertions.assertEquals(RequireTimeConditionAnalyzer.Position.NESTED_SUBQUERY, result.getPosition()); + } + private static ScanQuery scan(DataSource ds, QuerySegmentSpec spec) { return Druids.newScanQueryBuilder().dataSource(ds).intervals(spec).build(); diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java b/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java index 9891db27fb3b..5ba0ab28c26f 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/run/NativeQueryMaker.java @@ -84,10 +84,9 @@ public QueryResponse runQuery(final DruidQuery druidQuery) if (plannerContext.getPlannerConfig().isRequireTimeCondition() && !(druidQuery.getDataSource() instanceof InlineDataSource)) { - if (!RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(query)) { - throw new CannotBuildQueryException( - "requireTimeCondition is enabled, all queries must include a filter condition on the __time column" - ); + final RequireTimeConditionAnalyzer.Result result = RequireTimeConditionAnalyzer.analyze(query); + if (!result.isSatisfied()) { + throw new CannotBuildQueryException(buildRequireTimeConditionMessage(result)); } } int numFilters = plannerContext.getPlannerConfig().getMaxNumericInFilters(); @@ -153,6 +152,28 @@ public QueryResponse runQuery(final DruidQuery druidQuery) ); } + private static String buildRequireTimeConditionMessage(RequireTimeConditionAnalyzer.Result result) + { + final String leaf = result.getLeafName(); + final RequireTimeConditionAnalyzer.Position position = result.getPosition(); + if (position == RequireTimeConditionAnalyzer.Position.JOIN_RIGHT) { + return StringUtils.format( + "requireTimeCondition is enabled: datasource '%s' on the right side of a join has no filter on the __time column", + leaf + ); + } + if (position == RequireTimeConditionAnalyzer.Position.NESTED_SUBQUERY) { + return StringUtils.format( + "requireTimeCondition is enabled: datasource '%s' inside a nested subquery has no filter on the __time column", + leaf + ); + } + return StringUtils.format( + "requireTimeCondition is enabled: datasource '%s' has no filter on the __time column", + leaf + ); + } + @SuppressWarnings("unchecked") private QueryResponse execute( Query query, // Not final: may be reassigned with query ID added From 620f9068763a2673b9e7c00fdd1bfd2d9bd3eba8 Mon Sep 17 00:00:00 2001 From: nishtha-shah Date: Mon, 12 Oct 2026 10:52:39 +0530 Subject: [PATCH 08/15] test: cover deep-nested subquery and join mixes in RequireTimeConditionAnalyzer Add four tests exercising the walker at 4-5 layers deep: - Two pure-subquery depth-4 cases (bound at innermost / bound only in middle) confirm that only the innermost owning query's segment spec reaches the physical scan. - Two mixed subquery-and-join cases confirm the same at depth with a join in the chain: three bounded subquery wrappers above a Join with a bare unbounded right leg reject at the right-leg boundary (JOIN_RIGHT); a five-layer mix with a bounded right-leg wrapper above an unbounded inner rejects at the inner query (NESTED_SUBQUERY). --- .../RequireTimeConditionAnalyzerTest.java | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java index 79d38a610ccc..1ed2aeaf8fe7 100644 --- a/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java +++ b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java @@ -180,6 +180,36 @@ public void testIntermediateQueryDataSourceBoundIsRespected() Assertions.assertFalse(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(outer)); } + @Test + public void testFourLevelNestingBoundedAtDeepestIsSatisfied() + { + // outer(ETERNITY) -> QDS -> q3(ETERNITY) -> QDS -> q2(ETERNITY) -> QDS -> q1(BOUNDED, TABLE_FOO). + // Four layers of unbounded wrappers above a bounded innermost scan. The walker must descend + // all the way to q1 to find the bound; any premature stop would reject this. + final ScanQuery q1 = scan(TABLE_FOO, BOUNDED); + final ScanQuery q2 = scan(new QueryDataSource(q1), ETERNITY); + final ScanQuery q3 = scan(new QueryDataSource(q2), ETERNITY); + final ScanQuery outer = scan(new QueryDataSource(q3), ETERNITY); + Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(outer)); + } + + @Test + public void testFourLevelNestingBoundedOnlyAtMiddleIsNotSatisfied() + { + // outer(ETERNITY) -> QDS -> q3(BOUNDED) -> QDS -> q2(ETERNITY) -> QDS -> q1(ETERNITY, TABLE_FOO). + // A bound two layers up from the physical scan does not reach the leaf: only the innermost + // query owns the scan at runtime. Must be rejected, and the offender position must be + // NESTED_SUBQUERY (walker rejects at q1, which has multiple query ancestors). + final ScanQuery q1 = scan(TABLE_FOO, ETERNITY); + final ScanQuery q2 = scan(new QueryDataSource(q1), ETERNITY); + final ScanQuery q3 = scan(new QueryDataSource(q2), BOUNDED); + final ScanQuery outer = scan(new QueryDataSource(q3), ETERNITY); + final RequireTimeConditionAnalyzer.Result result = RequireTimeConditionAnalyzer.analyze(outer); + Assertions.assertFalse(result.isSatisfied()); + Assertions.assertEquals("foo", result.getLeafName()); + Assertions.assertEquals(RequireTimeConditionAnalyzer.Position.NESTED_SUBQUERY, result.getPosition()); + } + @Test public void testIntermediateBoundedInnerUnderUnboundedOuterIsSatisfied() { @@ -190,6 +220,45 @@ public void testIntermediateBoundedInnerUnderUnboundedOuterIsSatisfied() Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(outer)); } + @Test + public void testDeepSubqueryChainWithUnboundedJoinRightLegReportsJoinRight() + { + // Three bounded subquery layers wrap a Join whose right leg is a bare unbounded table. + // Shape: outer(BOUNDED) -> QDS -> q2(BOUNDED) -> QDS -> q1(BOUNDED) -> Join(TABLE_FOO, TABLE_BAR). + // The subquery bounds do not reach past the join's right-leg boundary at runtime, so the + // walker rejects at the right leg and reports JOIN_RIGHT with leaf 'bar'. + final ScanQuery q1 = scan(join(TABLE_FOO, TABLE_BAR), BOUNDED); + final ScanQuery q2 = scan(new QueryDataSource(q1), BOUNDED); + final ScanQuery outer = scan(new QueryDataSource(q2), BOUNDED); + final RequireTimeConditionAnalyzer.Result result = RequireTimeConditionAnalyzer.analyze(outer); + Assertions.assertFalse(result.isSatisfied()); + Assertions.assertEquals("bar", result.getLeafName()); + Assertions.assertEquals(RequireTimeConditionAnalyzer.Position.JOIN_RIGHT, result.getPosition()); + } + + @Test + public void testDeeplyNestedJoinWithUnboundedSubqueryOnRightLegReportsNestedSubquery() + { + // Five-layer mix: a bounded chain wraps a Join whose right leg is itself a subquery chain + // ending in an unbounded physical scan. + // Shape: outer(BOUNDED) -> QDS -> mid(BOUNDED) -> Join( + // left: TABLE_FOO + // right: QDS -> r2(BOUNDED) -> QDS -> r1(ETERNITY) -> TABLE_BAR + // ). + // The right-leg bound on r2 does not reach TABLE_BAR because only r1 (innermost query + // ancestor) owns the physical scan at runtime, and r1 is ETERNITY. The walker rejects at + // r1; because there are multiple Query ancestors, position is NESTED_SUBQUERY. + final ScanQuery r1 = scan(TABLE_BAR, ETERNITY); + final ScanQuery r2 = scan(new QueryDataSource(r1), BOUNDED); + final QueryDataSource rightLeg = new QueryDataSource(r2); + final ScanQuery mid = scan(join(TABLE_FOO, rightLeg), BOUNDED); + final ScanQuery outer = scan(new QueryDataSource(mid), BOUNDED); + final RequireTimeConditionAnalyzer.Result result = RequireTimeConditionAnalyzer.analyze(outer); + Assertions.assertFalse(result.isSatisfied()); + Assertions.assertEquals("bar", result.getLeafName()); + Assertions.assertEquals(RequireTimeConditionAnalyzer.Position.NESTED_SUBQUERY, result.getPosition()); + } + @Test public void testUnionOfTablesUnderBoundedOuterIsSatisfied() { From b2968786b5b2f69dd8b408fcad29816a2a76eaf5 Mon Sep 17 00:00:00 2001 From: nishtha-shah Date: Mon, 12 Oct 2026 13:48:36 +0530 Subject: [PATCH 09/15] test: cover nested-rollup positive case for requireTimeCondition SQL guard Adds testRequireTimeConditionNestedRollupPositive to pin a common nested GROUP BY shape at the SQL layer: an outer aggregate wrapping an inner subquery whose GROUP BY owns the __time bound. The analyzer-level coverage for this shape already exists via testIntermediateBoundedInnerUnderUnboundedOuterIsSatisfied; this test adds the end-to-end SQL assertion so the planner path stays regression-free. --- .../druid/sql/calcite/CalciteQueryTest.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 4cfcf3f81837..76b99f90833c 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -13309,6 +13309,36 @@ public void testRequireTimeConditionNonCollapsibleSubqueryPositive() } } + // Nested rollup: an outer GROUP BY over a subquery whose inner GROUP BY owns the + // __time bound. The inner query is the innermost owner of the physical scan, so its + // bound reaches the leaf and the guard is satisfied. Guards against regressions that + // mistake the outer query's segment spec for the physical scan's bound. + @Test + public void testRequireTimeConditionNestedRollupPositive() + { + try { + testQuery( + PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + "SELECT __time, SUM(cnt) AS cnt, dim1\n" + + "FROM (\n" + + " SELECT TIME_FLOOR(__time, 'PT1H') AS __time, SUM(cnt) AS cnt, dim1\n" + + " FROM druid.foo\n" + + " WHERE __time >= '2000-01-01' AND __time < '2001-01-01'\n" + + " GROUP BY TIME_FLOOR(__time, 'PT1H'), dim1\n" + + ")\n" + + "GROUP BY __time, dim1", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(), + ImmutableList.of() + ); + } + catch (CannotBuildQueryException e) { + Assertions.fail("requireTimeCondition rejected a nested-rollup with a bounded inner GROUP BY: " + e.getMessage()); + } + catch (AssertionError ignored) { + } + } + // The guard keys on query intervals, not filters. A __time predicate on a subquery // result stays a residual filter and never becomes a QuerySegmentSpec, so it must not // satisfy the guard. Preserved deliberately per RCA. From c3469ac0b9f7b98a0616e0d661ed053c1bd7a8cb Mon Sep 17 00:00:00 2001 From: nishtha-shah Date: Mon, 12 Oct 2026 14:33:14 +0530 Subject: [PATCH 10/15] test: cover CTE queries for requireTimeCondition SQL guard Adds two tests exercising the CTE (WITH ... AS (...)) surface: - testRequireTimeConditionCtePositive: a CTE that owns the __time bound in its definition inlines to a bounded inner subquery under the outer aggregate; the guard must accept it. - testRequireTimeConditionCteNegative: a CTE with no __time filter anywhere inlines to an unbounded scan; the guard must reject with the "__time column" message. Calcite inlines CTEs into equivalent nested subqueries (and unions when a CTE is referenced more than once), so these shapes already map onto existing analyzer coverage. These tests pin the SQL-layer behaviour so WITH syntax is not an unaudited user-visible dimension. --- .../druid/sql/calcite/CalciteQueryTest.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 76b99f90833c..db1818067a70 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -13339,6 +13339,55 @@ public void testRequireTimeConditionNestedRollupPositive() } } + // A CTE is inlined by Calcite into an equivalent nested subquery. When the CTE + // definition owns the __time bound, the resulting plan is a bounded inner query + // under an outer aggregate, which the guard must accept. + @Test + public void testRequireTimeConditionCtePositive() + { + try { + testQuery( + PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + "WITH recent AS (\n" + + " SELECT TIME_FLOOR(__time, 'PT1H') AS __time, SUM(cnt) AS cnt, dim1\n" + + " FROM druid.foo\n" + + " WHERE __time >= '2000-01-01' AND __time < '2001-01-01'\n" + + " GROUP BY TIME_FLOOR(__time, 'PT1H'), dim1\n" + + ")\n" + + "SELECT __time, SUM(cnt) AS cnt, dim1 FROM recent GROUP BY __time, dim1", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(), + ImmutableList.of() + ); + } + catch (CannotBuildQueryException e) { + Assertions.fail("requireTimeCondition rejected a CTE with a bounded inner __time filter: " + e.getMessage()); + } + catch (AssertionError ignored) { + } + } + + // A CTE with no __time filter anywhere inlines to an unbounded scan under the outer; + // the guard must reject with the specific "__time column" message. + @Test + public void testRequireTimeConditionCteNegative() + { + msqIncompatible(); + Throwable exception = assertThrows(CannotBuildQueryException.class, () -> { + testQuery( + PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + "WITH recent AS (\n" + + " SELECT dim1, cnt FROM druid.foo\n" + + ")\n" + + "SELECT dim1, SUM(cnt) FROM recent GROUP BY dim1", + CalciteTests.REGULAR_USER_AUTH_RESULT, + ImmutableList.of(), + ImmutableList.of() + ); + }); + assertTrue(exception.getMessage().contains("__time column")); + } + // The guard keys on query intervals, not filters. A __time predicate on a subquery // result stays a residual filter and never becomes a QuerySegmentSpec, so it must not // satisfy the guard. Preserved deliberately per RCA. From 0ecc579630f1f244612b1ad9ccb7ed22814d12b9 Mon Sep 17 00:00:00 2001 From: nishtha-shah Date: Mon, 12 Oct 2026 15:08:16 +0530 Subject: [PATCH 11/15] test: parametrise requireTimeCondition positives over on/off planner configs Converts the nine testRequireTimeCondition*Positive* tests to JUnit 5 @ParameterizedTest, running each case under both PLANNER_CONFIG_REQUIRE_TIME_CONDITION and PLANNER_CONFIG_DEFAULT. Each test executes 2 invocations (18 total) and asserts that enabling the guard does not change the native plan or the result rows for compliant queries. Scope: - testRequireTimeConditionPositive - testRequireTimeConditionPositive2 - testRequireTimeConditionPositive3 - testRequireTimeConditionLogicalValuePositive - testRequireTimeConditionExplicitJoinBothSidesFilteredPositive - testRequireTimeConditionLookupRightSidePositive - testRequireTimeConditionNonCollapsibleSubqueryPositive - testRequireTimeConditionNestedRollupPositive - testRequireTimeConditionCtePositive Negatives remain plain @Test with assertThrows: under require-off they would plan as normal unbounded queries, which already has coverage elsewhere and offers weak incremental signal. --- .../druid/sql/calcite/CalciteQueryTest.java | 83 +++++++++++++------ 1 file changed, 56 insertions(+), 27 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index db1818067a70..5efee921d5cd 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -139,7 +139,11 @@ import org.joda.time.Period; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Named; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import java.util.ArrayList; import java.util.Arrays; @@ -148,6 +152,7 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -12938,12 +12943,28 @@ public void testTextcat2() ); } - @Test - public void testRequireTimeConditionPositive() + /** + * Pairs of planner configs for the {@code testRequireTimeCondition*} parameterised tests. + * Each test runs twice: once with {@code requireTimeCondition=true} (the guard enabled) + * and once with the default planner config (guard disabled). Positive tests must produce + * identical native plans and rows under both configs; negative tests must throw under the + * guard and plan successfully without it. + */ + public static Stream requireTimeConditionConfigs() + { + return Stream.of( + Arguments.of(Named.of("require-on", PLANNER_CONFIG_REQUIRE_TIME_CONDITION)), + Arguments.of(Named.of("require-off", PLANNER_CONFIG_DEFAULT)) + ); + } + + @MethodSource("requireTimeConditionConfigs") + @ParameterizedTest(name = "{0}") + public void testRequireTimeConditionPositive(PlannerConfig plannerConfig) { // simple timeseries testQuery( - PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + plannerConfig, "SELECT SUM(cnt), gran FROM (\n" + " SELECT __time as t, floor(__time TO month) AS gran,\n" + " cnt FROM druid.foo\n" @@ -12968,12 +12989,13 @@ public void testRequireTimeConditionPositive() ); } - @Test - public void testRequireTimeConditionPositive2() + @MethodSource("requireTimeConditionConfigs") + @ParameterizedTest(name = "{0}") + public void testRequireTimeConditionPositive2(PlannerConfig plannerConfig) { // nested GROUP BY only requires time condition for inner most query testQuery( - PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + plannerConfig, "SELECT\n" + " SUM(cnt),\n" + " COUNT(*)\n" @@ -13013,15 +13035,16 @@ public void testRequireTimeConditionPositive2() // __time >= x remains in the join condition @DecoupledTestConfig(quidemReason = QuidemTestCaseReason.JOIN_FILTER_LOCATIONS) - @Test - public void testRequireTimeConditionPositive3() + @MethodSource("requireTimeConditionConfigs") + @ParameterizedTest(name = "{0}") + public void testRequireTimeConditionPositive3(PlannerConfig plannerConfig) { // Cannot vectorize next test due to substring function. cannotVectorize(); // semi-join requires time condition on both left and right query testQuery( - PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + plannerConfig, "SELECT COUNT(*) FROM druid.foo\n" + "WHERE __time >= '2000-01-01' AND SUBSTRING(dim2, 1, 1) IN (\n" + " SELECT SUBSTRING(dim1, 1, 1) FROM druid.foo\n" @@ -13084,11 +13107,12 @@ public void testRequireTimeConditionPositive3() ); } - @Test - public void testRequireTimeConditionLogicalValuePositive() + @MethodSource("requireTimeConditionConfigs") + @ParameterizedTest(name = "{0}") + public void testRequireTimeConditionLogicalValuePositive(PlannerConfig plannerConfig) { testQuery( - PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + plannerConfig, "SELECT 2 + 2 AS a", CalciteTests.REGULAR_USER_AUTH_RESULT, ImmutableList.of( @@ -13172,12 +13196,13 @@ public void testRequireTimeConditionSemiJoinNegative() // pre-fix behaviour (ExecutionVertex#getEffectiveQuerySegmentSpec), the outer join's // ETERNITY spec was surfaced and the guard rejected this. The analyzer descends both // legs and confirms every physical leaf is bounded. - @Test - public void testRequireTimeConditionExplicitJoinBothSidesFilteredPositive() + @MethodSource("requireTimeConditionConfigs") + @ParameterizedTest(name = "{0}") + public void testRequireTimeConditionExplicitJoinBothSidesFilteredPositive(PlannerConfig plannerConfig) { try { testQuery( - PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + plannerConfig, "SELECT COUNT(*) FROM\n" + " (SELECT dim1 FROM druid.foo WHERE __time >= '2000-01-01') a\n" + " INNER JOIN\n" @@ -13240,12 +13265,13 @@ public void testRequireTimeConditionExplicitJoinLeftSideMissingFilterNegative() // A lookup on the right side is global: the walker short-circuits at isGlobal(), so we // only need the left (bounded) leg to satisfy the guard. Regression guard against a fix // that would over-eagerly demand a __time filter on globals. - @Test - public void testRequireTimeConditionLookupRightSidePositive() + @MethodSource("requireTimeConditionConfigs") + @ParameterizedTest(name = "{0}") + public void testRequireTimeConditionLookupRightSidePositive(PlannerConfig plannerConfig) { try { testQuery( - PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + plannerConfig, "SELECT COUNT(*) FROM druid.foo\n" + " INNER JOIN lookup.lookyloo l ON foo.dim1 = l.k\n" + " WHERE foo.__time >= '2000-01-01'", @@ -13288,12 +13314,13 @@ public void testRequireTimeConditionThreeWayJoinDeepLegMissingFilterNegative() // at Query#mayCollapseQueryDataSource() (default false). LIMIT prevents the outer scan // from collapsing the inner query, so the pre-fix walker sees only the outer ETERNITY // spec and rejects. The analyzer descends the QueryDataSource unconditionally. - @Test - public void testRequireTimeConditionNonCollapsibleSubqueryPositive() + @MethodSource("requireTimeConditionConfigs") + @ParameterizedTest(name = "{0}") + public void testRequireTimeConditionNonCollapsibleSubqueryPositive(PlannerConfig plannerConfig) { try { testQuery( - PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + plannerConfig, "SELECT COUNT(*) FROM (\n" + " SELECT dim1 FROM druid.foo WHERE __time >= '2000-01-01' LIMIT 10\n" + ")", @@ -13313,12 +13340,13 @@ public void testRequireTimeConditionNonCollapsibleSubqueryPositive() // __time bound. The inner query is the innermost owner of the physical scan, so its // bound reaches the leaf and the guard is satisfied. Guards against regressions that // mistake the outer query's segment spec for the physical scan's bound. - @Test - public void testRequireTimeConditionNestedRollupPositive() + @MethodSource("requireTimeConditionConfigs") + @ParameterizedTest(name = "{0}") + public void testRequireTimeConditionNestedRollupPositive(PlannerConfig plannerConfig) { try { testQuery( - PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + plannerConfig, "SELECT __time, SUM(cnt) AS cnt, dim1\n" + "FROM (\n" + " SELECT TIME_FLOOR(__time, 'PT1H') AS __time, SUM(cnt) AS cnt, dim1\n" @@ -13342,12 +13370,13 @@ public void testRequireTimeConditionNestedRollupPositive() // A CTE is inlined by Calcite into an equivalent nested subquery. When the CTE // definition owns the __time bound, the resulting plan is a bounded inner query // under an outer aggregate, which the guard must accept. - @Test - public void testRequireTimeConditionCtePositive() + @MethodSource("requireTimeConditionConfigs") + @ParameterizedTest(name = "{0}") + public void testRequireTimeConditionCtePositive(PlannerConfig plannerConfig) { try { testQuery( - PLANNER_CONFIG_REQUIRE_TIME_CONDITION, + plannerConfig, "WITH recent AS (\n" + " SELECT TIME_FLOOR(__time, 'PT1H') AS __time, SUM(cnt) AS cnt, dim1\n" + " FROM druid.foo\n" From 9d70c87adee09f76b740893cf4462dd8177953f4 Mon Sep 17 00:00:00 2001 From: nishtha-shah Date: Mon, 12 Oct 2026 17:49:14 +0530 Subject: [PATCH 12/15] test: keep testRequireTimeConditionPositive3 as plain @Test for quidem recording DecoupledPlanningCalciteQueryTest pins the native plan for this semi-join via a frozen quidem .iq file keyed on the test's display name. Parametrising the method made the harness look for per-config recordings (testRequireTimeConditionPositive3@require-on.iq etc.) that don't exist, breaking DecoupledPlanningCalciteQueryTest in CI. Revert this one test to plain @Test. The guard runs post-plan, so the matrix adds no planner coverage here anyway. The other 8 positives stay parametrised. --- .../apache/druid/sql/calcite/CalciteQueryTest.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 5efee921d5cd..50b1053f93fd 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -13034,17 +13034,21 @@ public void testRequireTimeConditionPositive2(PlannerConfig plannerConfig) } // __time >= x remains in the join condition + // Not parametrised under require-on/require-off: DecoupledPlanningCalciteQueryTest + // pins a frozen native plan via a quidem .iq recording keyed on the test's display + // name. @ParameterizedTest would make the harness look for per-config .iq files + // (e.g. testRequireTimeConditionPositive3@require-on.iq) that don't exist, and the + // plan is config-independent anyway (the guard runs post-plan construction). @DecoupledTestConfig(quidemReason = QuidemTestCaseReason.JOIN_FILTER_LOCATIONS) - @MethodSource("requireTimeConditionConfigs") - @ParameterizedTest(name = "{0}") - public void testRequireTimeConditionPositive3(PlannerConfig plannerConfig) + @Test + public void testRequireTimeConditionPositive3() { // Cannot vectorize next test due to substring function. cannotVectorize(); // semi-join requires time condition on both left and right query testQuery( - plannerConfig, + PLANNER_CONFIG_REQUIRE_TIME_CONDITION, "SELECT COUNT(*) FROM druid.foo\n" + "WHERE __time >= '2000-01-01' AND SUBSTRING(dim2, 1, 1) IN (\n" + " SELECT SUBSTRING(dim1, 1, 1) FROM druid.foo\n" From 8a735ac561908a2667fa70ed8525fd5cb6e6fb01 Mon Sep 17 00:00:00 2001 From: nishtha-shah Date: Mon, 12 Oct 2026 18:14:51 +0530 Subject: [PATCH 13/15] test: add quidem coverage for requireTimeCondition SQL guard Pins the end-to-end behaviour of the guard through the SQL planner + Avatica path on two positive shapes (bounded nested-rollup, bounded CTE) and one negative (top-level unbounded) that asserts the formatted error message. Complements the existing CalciteQueryTest coverage: those tests exercise the guard against the native query tree directly, while this one validates the plan + results + error-message flow a JDBC client actually sees. Also pins the mysql-formatted result tables, which protects against regressions in planning that would otherwise silently change what the guard sees under it. --- .../requireTimeCondition.iq | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/requireTimeCondition.iq diff --git a/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/requireTimeCondition.iq b/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/requireTimeCondition.iq new file mode 100644 index 000000000000..0a17bd4e3389 --- /dev/null +++ b/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/requireTimeCondition.iq @@ -0,0 +1,51 @@ +!set requireTimeCondition true +!use druidtest:/// +!set outputformat mysql + +# Positive: bounded nested-rollup. The inner GROUP BY owns the physical scan and +# supplies the __time bound. Guards against regressions where the walker stops at +# the outer query's segment spec. +SELECT __time, SUM(cnt) AS cnt, dim1 +FROM ( + SELECT TIME_FLOOR(__time, 'PT1H') AS __time, SUM(cnt) AS cnt, dim1 + FROM druid.foo + WHERE __time >= '2000-01-01' AND __time < '2001-01-01' + GROUP BY TIME_FLOOR(__time, 'PT1H'), dim1 +) +GROUP BY __time, dim1; ++-------------------------+-----+------+ +| __time | cnt | dim1 | ++-------------------------+-----+------+ +| 2000-01-01 00:00:00.000 | 1 | | +| 2000-01-02 00:00:00.000 | 1 | 10.1 | +| 2000-01-03 00:00:00.000 | 1 | 2 | ++-------------------------+-----+------+ +(3 rows) + +!ok + +# Positive: bounded CTE. CTEs plan to nested subqueries; the inner has the __time +# bound and owns the physical scan. +WITH recent AS ( + SELECT TIME_FLOOR(__time, 'PT1H') AS __time, SUM(cnt) AS cnt, dim1 + FROM druid.foo + WHERE __time >= '2000-01-01' AND __time < '2001-01-01' + GROUP BY TIME_FLOOR(__time, 'PT1H'), dim1 +) +SELECT __time, SUM(cnt) AS cnt, dim1 FROM recent GROUP BY __time, dim1; ++-------------------------+-----+------+ +| __time | cnt | dim1 | ++-------------------------+-----+------+ +| 2000-01-01 00:00:00.000 | 1 | | +| 2000-01-02 00:00:00.000 | 1 | 10.1 | +| 2000-01-03 00:00:00.000 | 1 | 2 | ++-------------------------+-----+------+ +(3 rows) + +!ok + +# Negative: top-level unbounded. Verifies the formatted error message reaches the +# JDBC client end-to-end through the Avatica path. +SELECT dim1, SUM(cnt) FROM druid.foo GROUP BY dim1; +requireTimeCondition is enabled: datasource 'foo' has no filter on the __time column +!error From 16b882e1f24bd002064c5955a3b63552496c4331 Mon Sep 17 00:00:00 2001 From: nishtha-shah Date: Mon, 12 Oct 2026 21:40:04 +0530 Subject: [PATCH 14/15] test: add embedded e2e coverage for requireTimeCondition broker property Boots an in-process cluster with druid.sql.planner.requireTimeCondition=true, ingests a datasource via the standard BASIC_INDEX task, and submits SQL through the Broker's HTTP endpoint. Verifies both the positive path (bounded query returns results) and the negative path (unbounded query is rejected via a 400 Bad Request carrying the "has no filter on the __time column" message). This complements the existing coverage: - RequireTimeConditionAnalyzerTest -- analyzer semantics (unit) - CalciteQueryTest testRequireTimeCondition* -- planner-level SQL guard (unit) - requireTimeCondition.iq -- quidem SQL regression recording and closes the loop end-to-end through broker config wiring and the Avatica/HTTP response path. --- .../EmbeddedRequireTimeConditionTest.java | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 embedded-tests/src/test/java/org/apache/druid/testing/embedded/query/EmbeddedRequireTimeConditionTest.java diff --git a/embedded-tests/src/test/java/org/apache/druid/testing/embedded/query/EmbeddedRequireTimeConditionTest.java b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/query/EmbeddedRequireTimeConditionTest.java new file mode 100644 index 000000000000..aacbf1a73789 --- /dev/null +++ b/embedded-tests/src/test/java/org/apache/druid/testing/embedded/query/EmbeddedRequireTimeConditionTest.java @@ -0,0 +1,73 @@ +/* + * 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.testing.embedded.query; + +import org.apache.druid.error.ExceptionMatcher; +import org.apache.druid.rpc.HttpResponseException; +import org.apache.druid.testing.embedded.EmbeddedDruidCluster; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +/** + * End-to-end coverage for the {@code druid.sql.planner.requireTimeCondition} broker property. + * Runs an in-process cluster with the guard enabled, ingests a datasource, and submits SQL + * through the Broker's HTTP endpoint — the same path a client would take. + */ +public class EmbeddedRequireTimeConditionTest extends QueryTestBase +{ + private String dataSourceName; + + @Override + protected EmbeddedDruidCluster createCluster() + { + broker.addProperty("druid.sql.planner.requireTimeCondition", "true"); + return super.createCluster(); + } + + @Override + protected void beforeAll() + { + dataSourceName = ingestBasicData(); + } + + @Test + public void testQueryWithTimeConditionSucceeds() + { + Assertions.assertEquals( + "10", + cluster.runSql( + "SELECT COUNT(*) FROM %s WHERE __time >= '2025-06-01' AND __time < '2025-06-11'", + dataSourceName + ) + ); + } + + @Test + public void testQueryWithoutTimeConditionIsRejected() + { + final Throwable thrown = Assertions.assertThrows( + Exception.class, + () -> cluster.runSql("SELECT COUNT(*) FROM %s", dataSourceName) + ); + ExceptionMatcher.of(HttpResponseException.class) + .expectMessageContains("has no filter on the __time column") + .assertThat(thrown); + } +} From af9e3e609ab7fe33f04072b1782b6c4c481e4845 Mon Sep 17 00:00:00 2001 From: nishtha-shah Date: Tue, 13 Oct 2026 21:54:32 +0530 Subject: [PATCH 15/15] fix: narrow requireTimeCondition global-leaf exemption to non-segment datasources GlobalTableDataSource extends TableDataSource and remains segment-backed when queried directly or on a join's left leg (see its javadoc). The blanket dataSource.isGlobal() exemption at the leaf check therefore let an unbounded scan of a broadcast datasource bypass requireTimeCondition. Restrict the leaf-level exemption to leaves that are not instanceof TableDataSource (lookups, inline, frame-based inline), and preserve the right-leg broadcast-join path by short-circuiting isGlobal() leaves inside the JOIN_RIGHT branch (where a GlobalTableDataSource is materialized via JoinableFactory as an IndexedTable rather than scanned as segments). Adds four analyzer-level regression tests covering direct (bounded + eternity), left-leg under an unbounded outer, and right-leg broadcast-join cases for GlobalTableDataSource. --- .../RequireTimeConditionAnalyzer.java | 16 ++++++- .../RequireTimeConditionAnalyzerTest.java | 47 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java b/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java index 59f1e3066353..5e661b4fa2a6 100644 --- a/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java +++ b/processing/src/main/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzer.java @@ -159,7 +159,15 @@ protected Query visitQuery(Query query) @Override protected DataSource visit(DataSource dataSource, boolean leaf) { - if (!isPhysicalLeaf(dataSource) || dataSource.isGlobal()) { + if (!isPhysicalLeaf(dataSource)) { + return dataSource; + } + // Lookups, inline, and frame-based inline datasources have no segments to scan, + // so they always satisfy the guard. GlobalTableDataSource is still segment-backed + // when queried directly or on a join's left leg (see GlobalTableDataSource javadoc: + // "when queried directly, or on the left hand side of a join, they will be treated + // as any normal table datasource"), so it is NOT exempted here. + if (!(dataSource instanceof TableDataSource) && dataSource.isGlobal()) { return dataSource; } // Only the innermost Query ancestor's segment spec reaches a physical leaf at @@ -184,6 +192,12 @@ protected DataSource visit(DataSource dataSource, boolean leaf) if (ancestor.index != null && ancestor.index != 0 && i > 0) { final EVNode parentOfAncestor = parents.get(i - 1); if (!parentOfAncestor.isQuery() && parentOfAncestor.dataSource instanceof JoinDataSource) { + // Right-leg broadcast join: a GlobalTableDataSource is materialized as an + // IndexedTable via JoinableFactory rather than scanned as segments, so the + // right-side join exemption still applies here even though it is a TableDataSource. + if (dataSource.isGlobal()) { + return dataSource; + } recordRejection(dataSource, Position.JOIN_RIGHT); allLegsBounded = false; return dataSource; diff --git a/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java index 1ed2aeaf8fe7..b4006a94263b 100644 --- a/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java +++ b/processing/src/test/java/org/apache/druid/query/planning/RequireTimeConditionAnalyzerTest.java @@ -25,6 +25,7 @@ import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.query.DataSource; import org.apache.druid.query.Druids; +import org.apache.druid.query.GlobalTableDataSource; import org.apache.druid.query.InlineDataSource; import org.apache.druid.query.JoinAlgorithm; import org.apache.druid.query.JoinDataSource; @@ -51,6 +52,7 @@ public class RequireTimeConditionAnalyzerTest private static final QuerySegmentSpec ETERNITY = new MultipleIntervalSegmentSpec(Intervals.ONLY_ETERNITY); private static final TableDataSource TABLE_FOO = new TableDataSource("foo"); private static final TableDataSource TABLE_BAR = new TableDataSource("bar"); + private static final GlobalTableDataSource GLOBAL_TABLE = new GlobalTableDataSource("broadcastFoo"); private static final LookupDataSource LOOKUP_LOOKYLOO = new LookupDataSource("lookyloo"); private static final InlineDataSource INLINE = InlineDataSource.fromIterable( ImmutableList.of(new Object[0]), @@ -111,6 +113,51 @@ public void testGlobalLookupIsSatisfied() Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(scan(LOOKUP_LOOKYLOO, ETERNITY))); } + @Test + public void testDirectGlobalTableWithEternityIsNotSatisfied() + { + // GlobalTableDataSource extends TableDataSource and remains segment-backed when queried + // directly (see GlobalTableDataSource javadoc). The isGlobal() flag only indicates the + // broadcast-join path is available on a join's right side; it does not mean there is no + // physical scan. An unbounded direct scan must still be rejected. + final RequireTimeConditionAnalyzer.Result result = + RequireTimeConditionAnalyzer.analyze(scan(GLOBAL_TABLE, ETERNITY)); + Assertions.assertFalse(result.isSatisfied()); + Assertions.assertEquals("broadcastFoo", result.getLeafName()); + Assertions.assertEquals(RequireTimeConditionAnalyzer.Position.TOP_LEVEL, result.getPosition()); + } + + @Test + public void testDirectGlobalTableWithBoundedIsSatisfied() + { + // Bounded direct scan of a broadcast table is fine: the outer segment spec reaches the + // physical scan since there is no QueryDataSource or join leg between them. + Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(scan(GLOBAL_TABLE, BOUNDED))); + } + + @Test + public void testJoinLeftLegGlobalTableUnderUnboundedOuterIsNotSatisfied() + { + // Left-leg GlobalTableDataSource under an ETERNITY outer: the broadcast table is scanned + // as segments on the left side (same as a regular TableDataSource) and the outer provides + // no bound. Must be rejected rather than exempted via isGlobal(). + final ScanQuery query = scan(join(GLOBAL_TABLE, LOOKUP_LOOKYLOO), ETERNITY); + final RequireTimeConditionAnalyzer.Result result = RequireTimeConditionAnalyzer.analyze(query); + Assertions.assertFalse(result.isSatisfied()); + Assertions.assertEquals("broadcastFoo", result.getLeafName()); + Assertions.assertEquals(RequireTimeConditionAnalyzer.Position.TOP_LEVEL, result.getPosition()); + } + + @Test + public void testJoinRightLegGlobalTableIsSatisfied() + { + // Right-leg GlobalTableDataSource is materialized via JoinableFactory as an IndexedTable + // rather than scanned as segments, so the right-side-join exemption applies and the + // query is accepted even though isGlobal() no longer short-circuits at the leaf check. + final ScanQuery query = scan(join(TABLE_FOO, GLOBAL_TABLE), BOUNDED); + Assertions.assertTrue(RequireTimeConditionAnalyzer.hasTimeFilterOnAllLegs(query)); + } + @Test public void testGlobalInlineIsSatisfied() {