ClickHouse / ClickHouse/ClickHouse
`arrayReduce` with a seeded aggregate (`groupArraySample`) reports deterministic: the `HAVING` filter is pushed below aggregation and constant-folded, silently changing results
- Dominant language
- C++
- Stars
- 49.9k
- Forks
- 9k
- Avg merge
- 21h 32m
- Merged PRs (30d)
- 515
Description
### Company or project name
_No response_
### Describe what's wrong
`arrayReduce` and `arrayReduceInRanges` execute an aggregate function named by a string argument, but `FunctionArrayReduce` (`src/Functions/array/arrayReduce.cpp`) never overrides `isDeterministic` / `isDeterministicInScopeOfQuery`, so they report deterministic regardless of which aggregate they run. `groupArraySample` without an explicit seed draws from `thread_local_rng` on every evaluation (`src/AggregateFunctions/AggregateFunctionGroupArray.cpp`), so `arrayReduce('groupArraySample(1)', ...)` is a non-deterministic expression that the optimizer is told is deterministic. Two silently-wrong-result consequences at pure default settings:
1. A `HAVING` predicate built from it is pushed below the aggregation (legal only for deterministic predicates) and evaluated once per input row instead of once per group; a group survives if any of its rows draws favorably, so with enough rows per group every group survives.
2. With constant arguments the expression is constant-folded at analysis time, collapsing what should be an independent draw per row into a single draw reused for the whole query.
This is a different blind spot from the lambda-body one being fixed in #117747: here the nondeterminism is not hidden in a lambda — nothing consults the properties of the aggregate that the string names when the flags of `arrayReduce` itself are read.
### Does it reproduce on the most recent release?
Yes, reproduced on 26.9.1.1 (recent master build), new analyzer, default settings.
### How to reproduce
Push-down face — the predicate depends only on the grouping key, so push-down of a deterministic predicate would be legal, and the determinism lie is what makes it wrong:
```sql
SELECT count() FROM (SELECT number % 10 AS g FROM numbers(1000) GROUP BY g HAVING arrayReduce('groupArraySample(2)', [g, g + 1, 100])[1] < 50);
```
Ten runs at defaults return `10` every time (each group of 100 rows has at least one favorable draw). Control with `query_plan_filter_push_down = 0`: ten runs returned `5, 6, 7, 7, 7, 7, 7, 8, 8, 8` — the expected per-group binomial spread.
Constant-folding face:
```sql
SELECT arrayReduce('groupArraySample(1)', [1, 2, 3])[1] FROM numbers(5);
```
returns five identical values (one draw per query; the value varies across queries), while the unfoldable form varies per row as it should:
```sql
SELECT arrayReduce('groupArraySample(1)', materialize([1, 2, 3]))[1] FROM numbers(5);
-- e.g. 3 2 2 3 3
```
`arrayReduceInRanges` shows the same pair of faces:
```sql
SELECT arrayReduceInRanges('groupArraySample(1)', [(1, 3)], [10, 20, 30])[1][1]; -- one draw per query
SELECT arrayReduceInRanges('groupArraySample(1)', [(1, 3)], materialize([10, 20, 30]))[1][1] FROM numbers(5); -- varies per row
```
### Expected behavior
`arrayReduce` over a non-deterministic aggregate behaves like any other non-deterministic expression: the `HAVING` predicate is evaluated once per group after aggregation (the count varies around the binomial mean across runs), and constant arguments do not collapse the per-row draws into a single per-query value. When the named aggregate is deterministic — the overwhelmingly common case — folding and push-down stay as they are; `groupArraySample` with an explicit seed argument also stays deterministic.
### Error message and/or stacktrace
No error; silently wrong results.
### Additional context
The fix likely belongs in the overload resolver: it already creates the aggregate function object, so it can propagate the aggregate's determinism into the created `IFunctionBase` (a conservative alternative is a blocklist of seeded aggregates). Other consumers of the same flags read the same lie (for example the mutation determinism guard and query-result-cache eligibility); those consequences were not tested and are listed only as places to check when fixing.
Contributor guide
Assessment
This issue has not been assessed yet.