BackendArchitectX opened a new pull request, #20633:
URL: https://github.com/apache/druid/pull/20633

   Fixes #20632.
   
   ### Description
   
   User-input errors raised on data servers can lose their original 
classification while propagating through the query execution path. As a result, 
errors such as an invalid `useProjection` setting may reach clients as operator 
failures instead of `INVALID_INPUT (USER)` errors.
   
   This PR addresses both exception-propagation paths identified in #20632 and 
improves projection error messages.
   
   #### 1. Preserve the original exception in GroupByMergingQueryRunner
   
   `GroupByMergingQueryRunner` previously wrapped an `ExecutionException` in a 
new `RuntimeException`, obscuring the underlying `DruidException`.
   
   The updated handling:
   
   - Unwraps the original cause of `ExecutionException`.
   - Propagates unchecked exceptions directly, preserving their type and error 
classification.
   - Wraps checked exceptions using their underlying cause.
   - Retains the existing per-segment timeout handling and pending-future 
cancellation behavior.
   
   A regression test verifies that `InvalidInput` propagates unchanged through 
the multithreaded groupBy execution path.
   
   #### 2. Preserve DruidException metadata in JsonParserIterator
   
   `JsonParserIterator` previously deserialized structured error responses as 
legacy `QueryException` objects, losing the category and persona of modern 
`DruidException` responses.
   
   The updated handling:
   
   - Recognizes modern `DruidException` error responses.
   - Reconstructs the original exception using `ErrorResponse.fromMap`.
   - Preserves the error code, persona, category, message, and context.
   - Maintains compatibility with legacy `QueryException` responses.
   - Preserves existing handling for exceptions encountered while reading 
streamed responses.
   
   Regression tests cover modern JSON and Smile error responses, legacy Smile 
timeout responses, and malformed modern error responses.
   
   #### 3. Improve explicit projection validation errors
   
   When a query explicitly requests a projection that cannot be selected, 
`Projections` now distinguishes between two cases:
   
   - **Projection does not exist:** `Projection[p] does not exist`
   - **Projection exists but cannot satisfy the query:** `Projection[p] 
specified, but does not satisfy query`
   
   Both remain user-input errors.
   
   ### Validation
   
   **Passed**
   
   - 
`GroupByQueryRunnerFailureTest#testInvalidInputExceptionOnQueryable_multiThreaded`
 — 12 parameterized cases passed.
   - `JsonParserIteratorTest*` — 20 tests passed, including existing nested 
regression tests.
   - Maven compilation, Checkstyle, and forbidden-API checks completed 
successfully for the tested modules.
   - Git staged-diff whitespace validation passed.
   
   **Pending Linux CI verification**
   
   - `CursorFactoryProjectionTest#testTimeseriesQueryMissingNamedProjection` 
could not execute on Windows. Its parameterized test initialization failed 
because Windows could not delete an open memory-mapped `druid.segment` file. 
The failure occurred before the new assertion was reached.
   - An end-to-end broker/data-server verification of the expected HTTP 400 
response has not yet been performed.
   
   The projection test and broader regression checks should be verified in 
Linux CI.
   
   #### Release note
   
   Fixed an issue where invalid query inputs originating on data servers could 
be reported to clients as internal operator failures. Error classifications are 
now preserved across the affected query paths, and explicitly requested 
projections produce clearer messages when missing or incompatible.
   
   ---
   
   ##### Key changed/added classes in this PR
   
   - `GroupByMergingQueryRunner` — propagate underlying execution failures.
   - `JsonParserIterator` — deserialize modern structured errors while 
retaining legacy compatibility.
   - `Projections` — distinguish missing projections from incompatible 
projections.
   - `GroupByQueryRunnerFailureTest` — regression coverage for multithreaded 
exception propagation.
   - `JsonParserIteratorTest` — JSON, Smile, legacy, and malformed-response 
coverage.
   - `CursorFactoryProjectionTest` — regression coverage for missing named 
projections.
   
   ---
   
   This PR has:
   
   - [x] been self-reviewed.
   - [x] a release note entry in the PR description.
   - [x] added unit tests covering the modified code paths.
   - [x] been checked for compatibility with existing exception and timeout 
handling.


-- 
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]

Reply via email to