[C++][Substrait] RelCommon.emit indices are not bounds-checked before they index the schema
- Dominant language
- C++
- Stars
- 17.1k
- Forks
- 4.3k
- Avg merge
- 3d 13h
- Merged PRs (30d)
- 88
Description
`GetEmitInfo` passes every value in `RelCommon.emit.output_mapping` to both `FieldRef(map_id)` and the unchecked `input_schema->field(map_id)`. `ProcessEmitProject` does the same on the project path, where the value also indexes `proj_options.expressions`. With this implementation, a positive out-of-range index still reads past the end of a `FieldVector` and the process dies. For `-1`, Acero later rejects `FieldRef(-1)` with `ArrowInvalid`, but the unchecked schema lookup happens first. `ProcessExtensionEmit`, in the same file, already returns `Status::Invalid("Out of bounds emit index ", emit_idx)`.
```
read, emit [1, 0] [DataType(double), DataType(int64)]
read, emit [5] exit 139
read, emit [-1] ArrowInvalid: No match for FieldRef.FieldPath(-1)
project, emit [5] exit 139
aggregate, emit [1, 0] exit 139
```
The last variant is why this is more than a check on malformed input: that plan is valid Substrait. Arrow's vendored proto has no `expression_references`, so Arrow drops the grouping keys and derives an empty aggregate schema. The plan's valid `[1, 0]` mapping is then out of range for that empty schema. That grouping-key loss is #50634. A bounds check would turn the process crash into an error that reports the rejected `[1, 0]` mapping; fixing #50634 is still required for the plan to run.
Rechecked with PyArrow `26.0.0.dev244+g79e074ace`, built from `main` at `79e074ace3a7c4ca26f211dfd84a1984ad53c362`, on macOS arm64. The two positive out-of-range cases and the aggregate case still exit 139; the negative case now returns `ArrowInvalid`. The original PyArrow 25.0.1 reproduction crashed for the negative case as well on macOS arm64 and Linux x86-64. The source is `cpp/src/arrow/engine/substrait/relation_internal.cc`.
Reproducer — one file, pyarrow only
```python
"""RelCommon.emit.output_mapping indices are used to index the input schema unchecked.
The control succeeds and the negative case is caught as `ArrowInvalid`. Run each variant in a process of its own because the other three terminate:
for v in 0 1 2 3 4; do python3 emit_repro.py $v || echo " exit $?"; done
"""
import json, sys
import pyarrow as pa
import pyarrow.substrait as ps
from pyarrow._substrait import _parse_json_plan
SCHEMA = pa.schema([pa.field("c0", pa.int64(), nullable=False),
pa.field("c1", pa.float64(), nullable=False)])
def provider(names, schema=None):
return pa.table({"c0": [1], "c1": [2.5]}, schema=schema or SCHEMA)
def ref(i):
return {"selection": {"directReference": {"structField": {"field": i} if i else {}},
"rootReference": {}}}
def emit(m):
return {"common": {"emit": {"outputMapping": m}}}
def read(m=None):
r = {"baseSchema": {"names": ["c0", "c1"], "struct": {
"types": [{"i64": {"nullability": "NULLABILITY_REQUIRED"}},
{"fp64": {"nullability": "NULLABILITY_REQUIRED"}}],
"nullability": "NULLABILITY_REQUIRED"}},
"namedTable": {"names": ["t"]}}
return {"read": dict(r, **(emit(m) if m else {}))}
def run(rel, names):
plan = {"version": {"minorNumber": 102, "producer": "repro"},
"relations": [{"root": {"input": rel, "names": names}}]}
return ps.run_query(_parse_json_plan(json.dumps(plan).encode()), table_provider=provider)
# A plan that is valid Substrait: the grouping keys are where current Substrait puts them.
# Arrow's vendored proto has no expression_references, so it drops them (#50634) and the
# aggregate's output schema has no fields at all - which puts a correct mapping out of range.
AGG = {"aggregate": dict({"input": read(),
"groupings": [{"expressionReferences": [0, 1]}],
"groupingExpressions": [ref(0), ref(1)]}, **emit([1, 0]))}
VARIANTS = [
("read, emit [1, 0]", lambda: run(read([1, 0]), ["c1", "c0"])),
("read, emit [5]", lambda: run(read([5]), ["x"])),
("read, emit [-1]", lambda: run(read([-1]), ["x"])),
("project, emit [5]", lambda: run({"project": dict(
{"input": read(), "expressions": [ref(0)]},
**emit([5]))}, ["x"])),
("aggregate, emit [1, 0]", lambda: run(AGG, ["c1", "c0"])),
]
label, case = VARIANTS[int(sys.argv[1])]
print("%-22s" % label, end=" ", flush=True)
try:
print(case().read_all().schema.types)
except Exception as e:
print(type(e).__name__ + ":", str(e).replace("\n", " ")[:70])
```
Contributor guide
Research direction
Start with cpp/src/arrow/engine/substrait/relation_internal.cc, reading GetEmitInfo, ProcessEmitProject, and ProcessExtensionEmit to compare their emit-index handling. Run the supplied emit_repro.py variants to reproduce the crashes and invalid-index behavior. Done means out-of-range mappings return an error instead of indexing past the schema, while valid mappings continue to work.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, python
- Domain
- data-engineering
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100