xiangfu0 opened a new pull request, #18951:
URL: https://github.com/apache/pinot/pull/18951
## Problem
With `usePhysicalOptimizer=true`, a query whose `WHERE` clause contains a
boolean scalar function such as `arrayContainsString` fails at plan time:
```
Error composing query plan: No enum constant
org.apache.pinot.sql.FilterKind.arraycontainsstring
```
Repro (fails):
```sql
SET useMultistageEngine=true;
SET usePhysicalOptimizer=true;
SELECT /*+ joinOptions(is_colocated_by_join_keys='true') */ f.PrimaryKey
FROM finding f JOIN asset a ON f.ColocationKey = a.ColocationKey
WHERE arrayContainsString(a.AssetInfo_Groups, 'y') AND f.CID = 'x' AND a.CID
= 'x'
LIMIT 100
```
The equivalent single-stage / non-physical-optimizer query works:
```sql
SELECT * FROM asset WHERE arrayContainsString(AssetInfo_Groups, 'y') AND CID
= 'x' LIMIT 100
```
## Root cause
With the V2 physical optimizer, `LeafStageWorkerAssignmentRule` calls
`LeafStageToPinotQuery.createPinotQueryForRouting(...)` to build a `PinotQuery`
**used only for broker-side segment pruning / routing**. That filter is built
via `CalciteRexExpressionParser`, which does **not** run
`PredicateComparisonRewriter`.
As a result, `arrayContainsString(col, 'y')` is emitted as a filter operand
whose operator is the lowercased canonical function name `arraycontainsstring`,
sitting directly under `AND`. Segment pruners then call
`FilterKind.valueOf("arraycontainsstring")`, which throws.
Two reasons the bug is narrow:
- The **single-stage path works** because `PredicateComparisonRewriter`
wraps non-`FilterKind` boolean functions into the canonical `EQUALS(func,
TRUE)` form, which pruners understand.
- Comparisons like `!=` never failed, because they are emitted as `SqlKind`
names (`NOT_EQUALS`, `GREATER_THAN`, ...) that happen to be valid `FilterKind`
constants.
## Fix
`LeafStageToPinotQuery.ensureFilterIsFunctionExpression` is the existing
hook that normalizes the routing filter "so segment pruners can process it" (it
already wraps bare boolean identifiers and constant-folded literals). This PR
extends it: any `FUNCTION` whose operator is not a valid `FilterKind` (and is
not `AND`/`OR`/`NOT`) is wrapped as `EQUALS(func, TRUE)` — the same canonical
form the single-stage `PredicateComparisonRewriter` produces.
This is safe and conservative for pruning. All three pruners treat the
wrapped node as non-prunable and keep all segments:
- `SinglePartitionColumnSegmentPruner` /
`MultiPartitionColumnsSegmentPruner`: `EQUALS` with a non-identifier operand 0
→ `return true` (segment kept).
- `TimeSegmentPruner`: `EQUALS` whose operand 0 is not the time column →
`return null` (no interval constraint).
So no segment that could match is ever pruned away.
Note this filter is broker-local (routing only) and is never serialized to
servers, so there is no wire-format or mixed-version impact.
The PR also collapses the repeated `EQUALS(x, TRUE)` construction in that
method into a small `wrapAsEqualsTrue` helper.
## Testing
`LeafStageToPinotQueryTest` (29 tests, all passing), including new cases
that fail without the fix:
- `testNonFilterKindFunctionWrappedAsEqualsTrue`
- `testAndWithNonFilterKindFunctionWrapsFunction` /
`testOrWithNonFilterKindFunctionWrapsFunction`
- `testNotWithNonFilterKindFunctionWrapsChild`
- `testNonEqualsFilterKindFunctionPassedThroughUnchanged` — guards against
over-wrapping valid `FilterKind`s (e.g. `IN`)
- `testReportedQueryShapeProducesOnlyFilterKindOperators` — mirrors the
reported query's `WHERE` shape and asserts the invariant pruners rely on: every
operator a pruner traverses is a valid `FilterKind`, so `FilterKind.valueOf`
never throws
`spotless:check`, `checkstyle:check`, and `license:check` pass clean on
`pinot-query-planner`.
## Follow-ups (not in this PR)
- Consider unifying this routing-filter normalization with
`PredicateComparisonRewriter` (or extracting the shared wrapping helper) so the
two cannot silently diverge. A naive "just call PCR here" is risky: PCR
performs transforms undesirable for a best-effort routing filter
(VECTOR_SIMILARITY operand validation, a `default` branch requiring literal
operands, comparison flipping), while `ensureFilterIsFunctionExpression` does
routing-only normalization PCR does not (drop `LITERAL true`, `LITERAL false` →
always-false, unwrap single-operand `AND`/`OR`).
- Add a cluster-level integration test with a partitioned table +
multi-value STRING column exercising `arrayContainsString` under
`usePhysicalOptimizer=true`, to cover the full routing pipeline end to end.
## Labels
`bugfix`, `multi-stage`
--
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]
