matrixorigin / matrixorigin/matrixone

[Bug]: percentile aggregates order NaN opposite to WITHIN GROUP ORDER BY

Open
#28,854 0 comments 0 reactions 1 assignee Claimed by @XuPeng-SH View on GitHub
kind/bug needs-triage
Dominant language
Go
Stars
1.9k
Forks
311
Avg merge
1d 3h
Merged PRs (30d)
768

Description

## Description

`PERCENTILE_CONT` and `APPROX_PERCENTILE` use a NaN-first comparator for `DOUBLE`, contradicting the SQL `ORDER BY` relation used by the same `WITHIN GROUP (ORDER BY ...)` expression. This reverses percentile endpoints and changes interpolated values whenever NaN is present.

## Environment

- Branch: `main`
- Commit: `07fdd4ae0f80f287b93fc4b525fd296d5617abc7`
- Deployment: local standalone, 1 CN / 1 TN / 1 LogService

## Steps to reproduce

```sql
drop database if exists percentile_nan_repro;
create database percentile_nan_repro;
use percentile_nan_repro;
create table t(x double);
insert into t values (-1),(0),(1),(cast('NaN' as double));

select approx_percentile(x,0) as approx_p0,
approx_percentile(x,0.5) as approx_p50,
approx_percentile(x,1) as approx_p100,
median(x) as median_x
from t;

select percentile_cont(0) within group (order by x) as exact_p0,
percentile_cont(0.5) within group (order by x) as exact_p50,
percentile_cont(1) within group (order by x) as exact_p100
from t;

select cast(x as varchar) as ordered_x from t order by x;
```

## Actual behavior

```text
approx_p0 approx_p50 approx_p100 median_x
NaN -0.5 1 0.5

exact_p0 exact_p50 exact_p100
NaN -0.5 1

ordered_x
-1
0
1
NaN
```

The percentile functions behave as if the ordered values were `[NaN,-1,0,1]`, even though the explicit `ORDER BY x` relation is `[-1,0,1,NaN]`.

## Expected behavior

`WITHIN GROUP (ORDER BY x)` must use the same total ordering as `ORDER BY x`. For the displayed ordering, the endpoints should be `p0=-1`, `p100=NaN`, and the continuous median should be `0.5`, matching `MEDIAN(x)`.

## Stability and controls

- Reproducer: `3/3` with a freshly recreated database and table.
- Finite-only control `[-1,0,1]`: `p0=-1`, `p50=0`, `p100=1`.
- Infinity-only control `[-Inf,-1,0,1,+Inf]`: endpoints and median are correct.
- Ordinary `ORDER BY`, `GROUP BY`, index order, and `MEDIAN` consistently place NaN after numeric values.
- No crash or data mutation occurs.

## Evidence

- Tested commit: https://github.com/matrixorigin/matrixone/commit/07fdd4ae0f80f287b93fc4b525fd296d5617abc7
- The outputs above were reproduced in three independent client executions.

## Code analysis

`pkg/sql/colexec/aggexec/approx_percentile.go` defines `orderedCompare` so that NaN compares less than every non-NaN value. Both approximate and ordered-set percentile execution use that comparator. In contrast, `pkg/container/types/compare.go` documents and implements the SQL float ordering with NaNs after numeric values, and `pkg/sql/colexec/aggexec/median2.go` uses that SQL comparator. The percentile-specific comparator is therefore inconsistent with its own `WITHIN GROUP ORDER BY` contract.

## Regression coverage

Add focused aggregate UT and SQL regression cases for `FLOAT/DOUBLE` containing finite values, signed infinities, multiple NaN payloads, NULL, and percentiles `0/0.5/1`. Assert parity among explicit ordering, ordered-set percentile endpoints, median, partial/final merge, and spill execution.

## Related

Discovered while expanding statistical aggregate coverage. Duplicate searches covered `PERCENTILE_CONT`, `APPROX_PERCENTILE`, ordered-set aggregation, NaN ordering, and open/closed issues.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.