You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "bvolpato (via GitHub)" <gi...@apache.org> on 2026/09/03 17:18:19 UTC
[PR] Druid OR filters should not discard matching timestamp ranges [calcite]
bvolpato opened a new pull request, #5243:
URL: https://github.com/apache/calcite/pull/5243
## Jira Link
Pending issue creation and linkage. This PR is a draft.
## Changes Proposed
Druid interval extraction currently ignores OR branches it cannot represent as timestamp intervals. The filter rule then treats the remaining partial intervals as the complete condition and removes the filter, excluding valid rows.
Abort interval extraction if any OR branch is unrepresentable. This lets the existing filter path retain the full predicate. Add regressions for both operand orders, an empty-range branch, and planner behavior that preserves the original intervals and filter.
## Reproduction
For a Druid table whose time column is `timestamp`, this predicate must retain a row dated `2021-01-15`:
```sql
"timestamp" < TIMESTAMP '2020-01-01 00:00:00'
OR EXTRACT(DAY FROM "timestamp") = 15
```
The affected plan restricts intervals to dates before 2020 and drops the filter.
```sh
./gradlew :druid:test --tests 'org.apache.calcite.test.DruidDateRangeRulesTest' --tests 'org.apache.calcite.adapter.druid.DruidQueryFilterTest'
```
## Validation
- The mixed-OR utility and planner regressions fail against the baseline.
- Direct JUnit execution against the compiled fix passes both complete changed classes: 12 tests, no failures or skips, with assertions enabled.
- `./gradlew autostyleApply` and `git diff --check` pass.
- Focused runs used freshly compiled changed classes with cached supporting artifacts. Native Gradle test-class compilation on pristine `main` was blocked by `java.io.IOException: No space left on device`; the full Gradle build has not been validated locally.
## Downsides
Mixed OR predicates may scan a wider interval and rely on Druid filtering. Narrowing the interval safely requires accounting for every disjunct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Druid OR filters should not discard matching timestamp ranges [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5243:
URL: https://github.com/apache/calcite/pull/5243#issuecomment-5543801896
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5243) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5243&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=5243&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=5243&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5243&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=5243&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=5243)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7818] Druid OR filters should not discard matching timestamp ranges [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #5243:
URL: https://github.com/apache/calcite/pull/5243#discussion_r4090268184
##########
druid/src/main/java/org/apache/calcite/adapter/druid/DruidDateTimeUtils.java:
##########
@@ -126,9 +126,11 @@ protected static List<Interval> toInterval(
for (RexNode child : call.getOperands()) {
List<Range<Long>> extracted =
extractRanges(child, withNot);
- if (extracted != null) {
- intervals.addAll(extracted);
+ if (extracted == null) {
+ // Every disjunct must be represented to avoid excluding matching rows.
+ return null;
Review Comment:
This function has no javadoc, so it's hard to guess what "null" means.
It's hard even to guess what the returned result is supposed to be.
Maybe you can improve it a bit?
##########
druid/src/test/java/org/apache/calcite/adapter/druid/DruidQueryFilterTest.java:
##########
@@ -138,6 +156,42 @@ class DruidQueryFilterTest {
+ "\"ordering\":\"lexicographic\"}"));
}
+ @Test void testOrWithExtractRetainsFilter() {
+ final Fixture f = new Fixture();
+ final HepPlanner planner =
+ new HepPlanner(new HepProgramBuilder().addRuleInstance(DruidRules.FILTER).build(),
+ Contexts.of(new CalciteConnectionConfigImpl(new Properties())));
+ final RelOptCluster cluster = RelOptCluster.create(planner, f.rexBuilder);
+ final RelDataType rowType = f.typeFactory.builder()
+ .add("timestamp", SqlTypeName.TIMESTAMP)
+ .build();
+ final DruidTable table =
+ new DruidTable(Mockito.mock(DruidSchema.class), "events", factory -> rowType,
+ ImmutableSet.of(), "timestamp", null, null, null);
+ final RelOptTable relOptTable =
+ RelOptTableImpl.create(null, rowType, ImmutableList.of("events"), table,
+ clazz -> null);
+ final RelNode scan = LogicalTableScan.create(cluster, relOptTable, ImmutableList.of());
+ final DruidQuery query =
+ DruidQuery.create(cluster, cluster.traitSet().plus(BindableConvention.INSTANCE),
+ relOptTable, table, ImmutableList.of(scan));
+ final RexNode timestamp = f.rexBuilder.makeInputRef(scan, 0);
+ final RexNode condition =
+ f.rexBuilder.makeCall(
+ SqlStdOperatorTable.OR, f.rexBuilder.makeCall(SqlStdOperatorTable.LESS_THAN, timestamp,
+ f.rexBuilder.makeTimestampLiteral(new TimestampString("2020-01-01 00:00:00"), 0)),
+ f.rexBuilder.makeCall(SqlStdOperatorTable.EQUALS,
+ f.rexBuilder.makeCall(SqlStdOperatorTable.EXTRACT,
+ f.rexBuilder.makeFlag(TimeUnitRange.DAY), timestamp),
+ f.rexBuilder.makeExactLiteral(BigDecimal.valueOf(15))));
+ planner.setRoot(LogicalFilter.create(query, condition));
Review Comment:
inferring the generated code is not easy, maybe you can help by adding some comments showing the equivalent SQL for the various expressions generated
##########
druid/src/test/java/org/apache/calcite/test/DruidDateRangeRulesTest.java:
##########
@@ -59,6 +60,23 @@ class DruidDateRangeRulesTest {
is("[2011-01-01T00:00:00.000Z/2012-02-02T00:00:00.001Z]"));
}
+ @Test void testOrWithUnextractableRange() {
+ final Fixture2 f = new Fixture2();
+ final RexNode range = f.lt(f.ts, f.timestampLiteral(2020, Calendar.JANUARY, 1));
+ final RexNode extract = f.eq(f.exDay, f.literal(15));
+ assertThat(DruidDateTimeUtils.createInterval(f.or(range, extract)), nullValue());
Review Comment:
createInterval also does not say what a "null" result means, maybe you can improve its documentation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7818] Druid OR filters should not discard matching timestamp ranges [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5243:
URL: https://github.com/apache/calcite/pull/5243#issuecomment-6393096175
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Druid OR filters should not discard matching timestamp ranges [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #5243:
URL: https://github.com/apache/calcite/pull/5243#issuecomment-5672916719
I am not planning to review these PRs until they are marked as non-draft and there is a JIRA issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org