ykisana opened a new pull request, #20685:
URL: https://github.com/apache/druid/pull/20685
Fixes #20682.
### Description
`PartialQueryableIndexServableBundlesTest.testUseProjectionNamingUnservedProjectionFails`
has failed deterministically on master since #20633. #20617 and #20633 merged
about an hour apart and conflict with each other:
- #20617 added `PartialQueryableIndex.getServableProjection`. It builds a
filtered copy of the segment's projections that contains only the servable
ones, then passes that copy to `Projections.findMatchingProjection`.
- #20633 added a `Projection[%s] does not exist` check to
`findMatchingProjection`. The check runs against whatever set the caller passes.
So a `useProjection` name that is in the segment but not servable was
reported as `does not exist` instead of `specified, but does not satisfy query`.
#### Separate "exists" from "servable" in `findMatchingProjection`
- Added a `findMatchingProjection` overload that takes a `Predicate<String>
servableProjections`:
- only projections that pass the predicate are candidates for matching;
- the `does not exist` check still runs against the full `projections` set.
- The existing signature delegates with an always-true predicate, so
`SimpleQueryableIndex`, `OnheapIncrementalIndex` and
`PartialQueryableIndex.getProjection` behave exactly as before.
- `getServableProjection` now passes `projections` and `servableBundles`
directly instead of building a filtered `ObjectAVLTreeSet`. Iteration order is
the same comparator order, so the first servable match is unchanged.
Resulting behavior on the servable path:
- name not in the segment: `Projection[x] does not exist` (the #20633
behavior);
- name in the segment but not servable: `Projection[x] specified, but does
not satisfy query` (the #20617 behavior).
Alternatives considered:
- **Check existence in `getServableProjection` before filtering.** On its
own this doesn't help, because `findMatchingProjection` would still see only
the servable set. To work, `getServableProjection` would have to throw the
"does not satisfy query" error itself, which duplicates the message and the
`noProjections`/`useProjection` handling from `Projections`.
- **Change the test to expect "does not exist".** The projection does exist
in the segment, so that message would be misleading.
#### Release note
None. This fixes a regression in an error message that is only reachable
through the experimental `PARTIAL_RULE_HELD` acquire mode, which has not been
released.
<hr>
##### Key changed/added classes in this PR
* `Projections`
* `PartialQueryableIndex`
* `PartialQueryableIndexServableBundlesTest`
<hr>
This PR has:
- [x] been self-reviewed.
- [x] added Javadocs for most classes and all non-trivial methods. Linked
related entities via Javadoc links.
- [x] added unit tests or modified existing tests to cover new code paths,
ensuring the threshold for [code
coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md)
is met.
### Verification
```
mvn test -pl processing -am
-Dtest="org.apache.druid.segment.PartialQueryableIndexServableBundlesTest,org.apache.druid.segment.CursorFactoryProjectionTest"
-Dsurefire.failIfNoSpecifiedTests=false -Pskip-static-checks
-Dweb.console.skip=true -T1C
```
- `PartialQueryableIndexServableBundlesTest`: 8 tests, 0 failures. This
includes the previously failing test and the new
`testUseProjectionNamingMissingProjectionFails`, which asserts that an absent
name on the servable path still gets `does not exist`.
- `CursorFactoryProjectionTest`: 352 tests, 0 failures, 28 skipped. This
includes #20633's `testTimeseriesQueryMissingNamedProjection`.
- With only the test changes applied (fix reverted),
`testUseProjectionNamingUnservedProjectionFails` fails with the same error as
CI: `Projection[dim1_count] does not exist`.
- `mvn -pl processing checkstyle:check forbiddenapis:check
forbiddenapis:testCheck pmd:check`: clean.
### Generative AI
Generated by: Claude Opus 5.5
Fix, tests, PR description, and comments by Claude. All code manually
reviewed by @ykisana.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
