bug: Median() encountered integer overflow

[petern48]

Great catch. I looked into it, and it seems like it's wrapping around due to integer overflow while taking the average of the middle two values (since the count is even).

low: [-85], high: -56, median: 57 datatype: Int8

-85 + -56 = -141 -> wraparound to 115
Then 115 / 2 -> 57.5 -> 57 (truncated due to integer type)

What's our desired behavior in this case? We could promote to a larger datatype to perform the calculation. Also is it intentional to return the value as a truncated integer instead of a float?

Originally posted by @petern48 in #19278 (comment)

[kumarUjjawal]

@Jefffrey I've been looking into the median overflow issue. Hers' what percentile_cont uses for handling integer types.

• Casting integer inputs to Float64 internally (percentile_cont.rs L193-203)
• Returning Float64 for integer inputs (percentile_cont.rs L251-258)
• Casting in update_batch (percentile_cont.rs L513-519)

This would also address issue #18867, where we expect median(integers) to match percentile_cont(column, 0.5).

Questions before I proceed:

1. Should median hange its return type for integers (from Int8 → Float64)? This is a breaking change but matches
   percentile_cont behavior.
2. Or should we only fix the overflow while keeping integer return type? (Promote intermediate calculation to i128 or f64, then cast back)

[Jefffrey]

Should median hange its return type for integers (from Int8 → Float64)? This is a breaking change but matches
percentile_cont behavior.

I think this has been brought up before, though I can't remember which issue off the top of my head. It does sound reasonable to make it mimic percentile_cont instead of being discrete, though it would be good to verify against other DB systems (DuckDB, Postgres, Spark) to understand what they are doing. And a change like that is likely to be it's own issue separate from this overflow issue (perhaps search existing issue list to see if we already raised it).

Or should we only fix the overflow while keeping integer return type? (Promote intermediate calculation to i128 or f64, then cast back)

I'd prefer to fix the overflow in this issue only.

[kumarUjjawal]

Sure, I wiil look into int.

[kumarUjjawal]

I checked how DuckDB, PostgreSQL, and Spark handle median() for integer inputs. All three databases return Float64/Double for median calculations, which avoids both the overflow issue and truncation loss.

DuckDB defines median(x) as an alias for quantile_cont(x, 0.5) (docs). The return type is always DOUBLE for all continuous quantile types. (https://github.com/duckdb/duckdb/blob/v1.1.3/src/core_functions/aggregate/holistic/quantile.cpp)

PostgreSQL doesn't have a built-in median() function but uses. percentile_cont(0.5) which returns double precision. (https://github.com/postgres/postgres/blob/master/src/backend/utils/adt/orderedsetaggs.c).

Apache Spark implements median() as equivalent to percentile_cont(0.5) (docs). (https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/percentiles.scal)

[kumarUjjawal]

@Jefffrey What should be the next steps?

[Jefffrey]

My last comment still applies

[kumarUjjawal]

Sure!

[petern48]

Given that all 3 of those other engines return a float and that I didn't find any existing issue for it already, I filed a new issue for the truncation part: #19536