apache / apache/arrow

[C++][Substrait] Modern extension URNs fall back to function-name matching

Open
#51,234 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
17.1k
Forks
4.3k
Avg merge
3d 13h
Merged PRs (30d)
88

Description

Acero accepts a function from an unregistered modern Substrait extension URN and executes an Arrow function with the same name. A populated extension identifier reaches the legacy name-only fallback instead of identifying the requested extension or producing an unsupported error.

### To reproduce

Install `pyarrow==25.0.1` and `substrait==0.31.0` — not `substrait-protobuf`, which makes `import pyarrow.substrait` fail on a missing `substrait.proto` — then run:

Reproducer — one file, pyarrow and the Substrait bindings

```python
import pyarrow as pa
import pyarrow.substrait as ps
from substrait import plan_pb2, type_pb2

plan = plan_pb2.Plan()
plan.version.minor_number = 102
plan.extension_urns.add(
extension_urn_anchor=1,
urn="extension:example.com:unregistered_arithmetic",
)
extension = plan.extensions.add().extension_function
extension.extension_urn_reference = 1
extension.function_anchor = 1
extension.name = "add:i64_i64"

root = plan.relations.add().root
root.names.append("r")
project = root.input.project
project.common.emit.output_mapping.append(2)
read = project.input.read
read.named_table.names.append("t")
read.base_schema.names.extend(["a", "b"])
read.base_schema.struct.nullability = type_pb2.Type.NULLABILITY_REQUIRED
for _ in range(2):
read.base_schema.struct.types.add().i64.nullability = type_pb2.Type.NULLABILITY_NULLABLE

function = project.expressions.add().scalar_function
function.function_reference = 1
function.output_type.i64.nullability = type_pb2.Type.NULLABILITY_NULLABLE
for index in range(2):
selection = function.arguments.add().value.selection
selection.direct_reference.struct_field.field = index
selection.root_reference.SetInParent()

def provider(names, schema):
assert names == ["t"]
return pa.table({"a": [2], "b": [3]}, schema=schema)

result = ps.run_query(
plan.SerializeToString(), table_provider=provider, use_threads=False
).read_all()
print(result.to_pydict())
```

It prints `{'r': [5]}` on PyArrow 25.0.1.

I expected rejection because no implementation was registered for this extension. The function name alone does not establish which extension it belongs to. This example serializes the URN fields with modern protobuf bindings; it does not use Arrow's older JSON-to-protobuf helper, which can drop those fields during encoding.

### Decimal consequence and controls

With the standard `extension:io.substrait:functions_arithmetic_decimal` URN, `divide:dec_dec` over nullable `decimal(10,2)` and `decimal(5,1)` returns `decimal(16,7)`. The [v0.102.0 extension formula](https://github.com/substrait-io/substrait/blob/v0.102.0/extensions/functions_arithmetic_decimal.yaml#L67) specifies `decimal(21,8)`: scale is `max(6, 2 + 5 + 1) = 8`, and precision is `10 - 2 + 5 + 8 = 21`.

This changes an exactly representable value: `1.00 / 256.0` returns `0.0039062` instead of `0.00390625`. Native Arrow division produces the same result. The legacy URI form naming `functions_arithmetic_decimal.yaml` is rejected because no conversion is registered for that extension. An unregistered legacy URI is also rejected, while integer addition with the registered legacy arithmetic URI succeeds.

Unsupported extensions can be rejected. The problem is that the modern identifier is lost and a different function is selected by name.

### Relevant code

[GetExtensionSetFromMessage](https://github.com/apache/arrow/blob/apache-arrow-25.0.1/cpp/src/arrow/engine/substrait/util_internal.h#L43) reads `extension_uris` and `extension_uri_reference`. Modern plans use [`extension_urns`](https://github.com/substrait-io/substrait/blob/v0.102.0/proto/substrait/plan.proto#L34) and [`extension_urn_reference`](https://github.com/substrait-io/substrait/blob/v0.102.0/proto/substrait/extensions/extensions.proto#L64), with different field numbers. The legacy reference defaults to zero, and the map lookup creates an empty URI. [Scalar function conversion](https://github.com/apache/arrow/blob/apache-arrow-25.0.1/cpp/src/arrow/engine/substrait/expression_internal.cc#L363) then uses the name-only fallback.

The fallback added in #14143 was intended for empty or `/` identifiers. This case supplies a populated modern identifier. Related PR #50635 updates Fetch, Aggregate and Join handling with a proto bump to v0.63.0; its current changes do not include URN resolution.

The reproduction is against 25.0.1; `cpp/src/arrow/engine/substrait` is unchanged on `main` since f14ae5b3.

[Five focused cases with controls](https://github.com/alexandrefimov/substrait-conformance-cases/tree/d6c83a8f806e2ab994ec88461af14aa7f5b6436c/probe/structural-cases/acero-functions) include the modern URNs, legacy URI counterparts and exact decimal values.

Contributor guide

Open the contributing guide

Research direction

Start with GetExtensionSetFromMessage in cpp/src/arrow/engine/substrait/util_internal.h and scalar function conversion in cpp/src/arrow/engine/substrait/expression_internal.cc. Run the supplied PyArrow/Substrait reproducer and compare it with the linked focused conformance cases. Done means populated modern URNs no longer use the name-only fallback, unregistered extensions are rejected, and the documented legacy controls retain their behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
data-engineering
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.