Repository navigation
Open
Description
Describe the bug, including details regarding any error messages, version, and platform.
In cpp/src/arrow/engine/substrait/extension_set.cc (verified on PyArrow 26.0.1 and current main HEAD 78188dd8d7, lines 1080–1089), DecodeBasicAggregate maps Substrait's distribution option for variance and std_dev ("stddev") with ddof inverted:
// cpp/src/arrow/engine/substrait/extension_set.cc:1080-1089
int ddof;
if (prefs[0] == "POPULATION") {
ddof = 1;
} else if (prefs[0] == "SAMPLE") {
ddof = 0;
} else {
return Status::Invalid("unknown distribution preference ", prefs[0]);
}
options = std::make_shared<compute::VarianceOptions>(ddof);In arrow::compute::VarianceOptions, the divisor is N - ddof, so:
-
Population variance / standard deviation (
$\frac{1}{N}\sum (x_i - \mu)^2$ ) requiresddof = 0. -
Sample variance / standard deviation (
$\frac{1}{N - 1}\sum (x_i - \bar{x})^2$ ) requiresddof = 1.
Because the two branches are swapped:
- A Substrait
AggregateRelrequestingdistribution = ["POPULATION"]silently computes sample variance/stddev (ddof = 1), and on a 1-row group (N = 1) returnsNULLinstead of0.0. - A Substrait
AggregateRelrequestingdistribution = ["SAMPLE"]silently computes population variance/stddev (ddof = 0), and on a 1-row group (N = 1) returns0.0instead ofNULL.
Minimal Reproducer (Python pyarrow 26.0.1)
import pyarrow as pa
import pyarrow.compute as pc
import pyarrow.substrait as pas
from substrait import algebra_pb2, plan_pb2, type_pb2
def _varint(n: int) -> bytes:
out = bytearray()
while n >= 0x80:
out.append((n & 0x7F) | 0x80)
n >>= 7
out.append(n)
return bytes(out)
def _ld(tag: int, payload: bytes) -> bytes:
return _varint((tag << 3) | 2) + _varint(len(payload)) + payload
def run_substrait_variance(tbl: pa.Table, dist: str):
plan = plan_pb2.Plan()
plan.version.minor_number = 27
root = plan.relations.add().root
root.names.append("res")
agg = root.input.aggregate
read = agg.input.read
read.named_table.names.append("t")
read.base_schema.names.append("x")
read.base_schema.struct.types.append(
type_pb2.Type(fp64=type_pb2.Type.FP64(nullability=type_pb2.Type.NULLABILITY_NULLABLE))
)
read.base_schema.struct.nullability = type_pb2.Type.NULLABILITY_REQUIRED
m = agg.measures.add().measure
m.function_reference = 10
m.phase = algebra_pb2.AGGREGATION_PHASE_INITIAL_TO_RESULT
m.output_type.fp64.nullability = type_pb2.Type.NULLABILITY_NULLABLE
opt = m.options.add()
opt.name = "distribution"
opt.preference.append(dist)
arg = m.arguments.add().value
arg.selection.direct_reference.struct_field.field = 0
arg.selection.root_reference.CopyFrom(algebra_pb2.Expression.FieldReference.RootReference())
uri = b"https://github.com/substrait-io/substrait/blob/main/extensions/functions_arithmetic.yaml"
ext_u = _varint(8) + _varint(1) + _ld(2, uri)
fn_m = _varint(8) + _varint(1) + _varint(16) + _varint(10) + _ld(3, b"variance:fp64")
buf = _ld(1, ext_u) + _ld(2, _ld(3, fn_m)) + plan.SerializeToString()
return pas.run_query(buf, table_provider=lambda names, schema: tbl).read_all()["res"].to_pylist()
tbl4 = pa.table({"x": pa.array([2.0, 4.0, 6.0, 8.0], type=pa.float64())})
print("Substrait POPULATION:", run_substrait_variance(tbl4, "POPULATION"), "expected ddof=0:", pc.variance(tbl4["x"], ddof=0).as_py())
print("Substrait SAMPLE: ", run_substrait_variance(tbl4, "SAMPLE"), "expected ddof=1:", pc.variance(tbl4["x"], ddof=1).as_py())
tbl1 = pa.table({"x": pa.array([5.0], type=pa.float64())})
print("N=1 POPULATION:", run_substrait_variance(tbl1, "POPULATION"), "expected:", pc.variance(tbl1["x"], ddof=0).as_py())
print("N=1 SAMPLE: ", run_substrait_variance(tbl1, "SAMPLE"), "expected:", pc.variance(tbl1["x"], ddof=1).as_py())Actual vs. Expected Output
- Actual:
Substrait POPULATION: [6.666666666666667] expected ddof=0: 5.0 Substrait SAMPLE: [5.0] expected ddof=1: 6.666666666666667 N=1 POPULATION: [None] expected: 0.0 N=1 SAMPLE: [0.0] expected: None - Expected:
"POPULATION"maps toddof = 0(5.0on[2, 4, 6, 8],0.0on[5.0]) and"SAMPLE"maps toddof = 1(6.666666666666667on[2, 4, 6, 8],Noneon[5.0]), for bothvarianceandstd_dev.
Component(s)
C++, Python
Activity
[-][C++][Substrait] DecodeBasicAggregate inverts POPULATION (ddof=1) and SAMPLE (ddof=0) for variance and std_dev[/-][+][Python][C++][Substrait] DecodeBasicAggregate inverts POPULATION (ddof=1) and SAMPLE (ddof=0) for variance and std_dev[/+]apacheGH-55575: [C++][Substrait] Fix inverted ddof for POPULATION/SAM…