Skip to content

Query parameter parsing behavior#2

Closed
benchaplin wants to merge 1 commit intomainfrom
cursor/query-parameter-parsing-behavior-1118
Closed

Query parameter parsing behavior#2
benchaplin wants to merge 1 commit intomainfrom
cursor/query-parameter-parsing-behavior-1118

Conversation

@benchaplin
Copy link
Copy Markdown
Owner

  • Have you signed the contributor license agreement?
  • Have you followed the contributor guidelines?
  • If submitting code, have you built your formula locally prior to submission with gradle check?
  • If submitting code, is your pull request against main? Unless there is a good reason otherwise, we prefer pull requests against main and will backport as needed.
  • If submitting code, have you checked that your submission is for an OS and architecture that we support?
  • If you are submitting this code for a class then read our policy for that.

This PR introduces a new integration test suite, ClearIndicesCacheParametersIT, to investigate and confirm the query parameter parsing bug in the Clear Cache API (related to elastic#94512).

The tests demonstrate that the _cache/clear endpoint incorrectly interprets boolean query parameters (e.g., request=false or request not specified) as "clear all" due to a fallback mechanism in IndexService.clearCaches and IndicesService.clearIndexShardCache. This occurs because the system cannot distinguish between a parameter being explicitly set to false and it defaulting to false when not provided.

The 19 test methods cover various combinations of request, query, and fielddata parameters. All tests currently pass, asserting the existing (buggy) behavior. Specific test cases that highlight the bug are clearly marked with [BUG] in their assertion messages and Javadoc, making it straightforward to flip the assertions once a fix is implemented.


Open in Web Open in Cursor 

Add ClearIndicesCacheParametersIT to test and document the current
behavior of the clear cache API across all query parameter
combinations (request, query, fielddata).

Tests exercise the full REST flow via real HTTP requests and verify
which caches are actually cleared by checking index stats.

The tests document the bug from elastic#94512: when only request=true is
specified, all three caches are cleared instead of just the request
cache. This happens because IndexService.clearCaches() cannot
distinguish between 'not specified' (default false) and 'explicitly
set to false'.

Co-authored-by: Ben Chaplin <benchaplin@protonmail.com>
@cursor
Copy link
Copy Markdown

cursor bot commented Mar 6, 2026

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@benchaplin benchaplin closed this Apr 5, 2026
@benchaplin benchaplin deleted the cursor/query-parameter-parsing-behavior-1118 branch April 5, 2026 18:43
benchaplin pushed a commit that referenced this pull request Apr 7, 2026
…tic#140027) (elastic#141095)

This PR fixes the issue where `INLINE STATS GROUP BY null` was being
incorrectly pruned by `PruneLeftJoinOnNullMatchingField`.

Fixes elastic#139887

## Problem For query:

```
FROM employees
| INLINE STATS c = COUNT(*) BY n = null
| KEEP c, n
| LIMIT 3
```

During `LogicalPlanOptimizer`:

```
Limit[3[INTEGER],false,false]
\_EsqlProject[[c{r}#2, n{r}elastic#4]]
  \_InlineJoin[LEFT,[n{r}elastic#4],[n{r}elastic#4]]
    |_Eval[[null[NULL] AS n#4]]
    | \_EsRelation[employees][<no-fields>{r$}elastic#7]
    \_Aggregate[[n{r}elastic#4],[COUNT(*[KEYWORD],true[BOOLEAN],PT0S[TIME_DURATION]) AS c#2, n{r}elastic#4]]
      \_StubRelation[[<no-fields>{r$}elastic#7, n{r}elastic#4]]
```

The following join node:

```
InlineJoin[LEFT,[n{r}elastic#4],[n{r}elastic#4]]
|_Eval[[null[NULL] AS n#4]]
| \_EsRelation[employees][<no-fields>{r$}elastic#7]
\_Aggregate[[n{r}elastic#4],[COUNT(*[KEYWORD],true[BOOLEAN],PT0S[TIME_DURATION]) AS c#2, n{r}elastic#4]]
  \_StubRelation[[<no-fields>{r$}elastic#7, n{r}elastic#4]]
```

should NOT have `PruneLeftJoinOnNullMatchingField` applied, because the
right side is an `Aggregate` (originating from `INLINE STATS`). Since
`STATS` supports `GROUP BY null`, the join key being null is a valid use
case. Pruning this join would incorrectly eliminate the aggregation
results, changing the query semantics.

During `LocalLogicalPlanOptimizer`:

```
ProjectExec[[c{r}#2, n{r}elastic#4]]
\_LimitExec[3[INTEGER],null]
  \_ExchangeExec[[c{r}#2, n{r}elastic#4],false]
    \_FragmentExec[filter=null, estimatedRowSize=0, reducer=[], fragment=[<>
Project[[c{r}#2, n{r}elastic#4]]
\_Limit[3[INTEGER],false,false]
  \_InlineJoin[LEFT,[n{r}elastic#4],[n{r}elastic#4]]
    |_Eval[[null[NULL] AS n#4]]
    | \_EsRelation[employees][<no-fields>{r$}elastic#7]
    \_LocalRelation[[c{r}#2, n{r}elastic#4],Page{blocks=[LongVectorBlock[vector=ConstantLongVector[positions=1, value=100]], ConstantNullBlock[positions=1]]}]<>]]
```

The following join node:

```
InlineJoin[LEFT,[n{r}elastic#4],[n{r}elastic#4]]
|_Eval[[null[NULL] AS n#4]]
| \_EsRelation[employees][<no-fields>{r$}elastic#7]
\_LocalRelation[[c{r}#2, n{r}elastic#4],Page{blocks=[LongVectorBlock[vector=ConstantLongVector[positions=1, value=100]], ConstantNullBlock[positions=1]]}]
```

should NOT have `PruneLeftJoinOnNullMatchingField` applied, because the
right side is a `LocalRelation` (the `Aggregate` was optimized into a
`LocalRelation` containing the pre-computed aggregation results).
Pruning this join when the join key is null would discard the valid
aggregation results stored in the `LocalRelation`, incorrectly producing
null values instead of the expected count.

## Solution The fix ensures that `PruneLeftJoinOnNullMatchingField` only
applies to `LOOKUP JOIN` nodes, where `join.right()` is an `EsRelation`.
For `INLINE STATS` joins, the right side can be:

 - `Aggregate` (before optimization), or
 - `LocalRelation` (after the aggregate is optimized)

By checking `join.right() instanceof EsRelation`, we correctly skip the
pruning optimization for `INLINE STATS` joins, preserving the expected
query results when grouping by null.

(cherry picked from commit f3ccb70)

Co-authored-by: kanoshiou <uiaao@tuta.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants