xiangfu0 opened a new pull request, #19654:
URL: https://github.com/apache/pinot/pull/19654
Stacked on #19647 (aggregate call binding contract). Review only the top
commit; the base updates when #19647 merges.
## Summary
Attaches `AggregateCallBinding`s to single-stage requests. **No aggregate
requires a binding yet** (`AggregationFunctionType.isTypeBindingRequired` is
false everywhere), so request bytes and results are unchanged. This PR adds the
machinery that follow-ups turn on per aggregate family.
- **`ExpressionTypeResolver`** derives the logical type and cardinality of
an aggregate's input expressions from the table schema and function metadata.
It follows the precedence SSE uses at execution: native transforms first, then
typed scalar overloads. It never constructs or evaluates a transform, so it is
safe to run on a broker.
- **`AggregationFunctionBinder`** binds calls throughout a request: SELECT,
GROUP BY, ORDER BY, WHERE, HAVING, aggregates nested in transforms, and FILTER.
It leaves call names, operands and expression identity unchanged, and never
overwrites an existing binding.
- **Where binding happens:**
- `QueryOptimizer`, whenever a schema is available. This covers the
broker, the multi-stage leaf and the recommender.
- Broker expression overrides, after overrides are applied. This binds
calls that an override introduces.
- Direct server SQL, which has no broker. `QueryContext` defers aggregate
construction until the executor supplies the table schema through `setSchema`.
Only the top-level query is deferred.
## Behavior at the edges
These address review findings on #19523:
- **No HTTP 500 for unbindable calls.**
- A call that cannot be bound raises `BadQueryRequestException`.
- The broker returns it as `QUERY_VALIDATION` from `compileRequest` and
after the materialized-view step.
- Previously the exception escaped `handleRequest` and became an internal
server error.
- **Server-only transforms.**
- A broker cannot see transforms registered only on servers, so an unknown
function has "no schema-only type" rather than being invalid.
- Aggregates that already supported unbound execution keep that path, and
the server validates the function as before.
- Invalid expressions (unknown column, incompatible CASE branches, bad
type literal) still fail.
- **Built-in virtual columns.** A server's table schema does not declare
`$docId`, `$segmentName` and the other built-in virtual columns, while a
broker's schema does. The resolver falls back to
`BuiltInVirtualColumnDefinitions`, so direct server SQL resolves them like the
broker.
- **Expression overrides** are matched by `ExpressionContext`, which
excludes execution metadata. The configured override is copied, never bound in
place. A storage column substituted for a logical one keeps the call's logical
binding.
- **Materialized views.**
- View definitions are matched by Thrift expression equality, which
includes bindings.
- The broker now strips bindings before the view match. It then binds the
query that executes against the schema of the table it targets: the view after
a full rewrite, otherwise the base table.
## Tests
The tests stub the per-call rule so MODE requires a binding, because nothing
opts in yet. The first real opt-in (two-argument `FIRST_WITH_TIME` /
`LAST_WITH_TIME`) follows with end-to-end tests.
- `ExpressionTypeResolverTest` checks the inferred type against the type of
an independently constructed execution transform, for 35 expressions. It also
covers invalid expressions, unknown functions, and virtual columns.
- `AggregationFunctionBinderTest` covers:
- calls that need no binding are untouched
- every clause is bound and call identity is unchanged
- an existing binding is authoritative
- a failure is reported as `BadQueryRequestException`
- `unbind`
- `BaseSingleStageBrokerRequestHandlerTest` covers:
- override matching ignores bindings and binds the replacement
- a storage override keeps the logical binding
- an unbindable call returns `QUERY_VALIDATION`
- the view match sees the unbound query, and the executed view query is
bound against the view's schema
- `ServerQueryRequestTest` covers deferred construction of direct-SQL
aggregates, which happens once, and virtual-column resolution against a server
schema.
--
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]
