kadinrabo opened a new pull request, #1399:
URL: https://github.com/apache/arrow-go/pull/1399

   ### Rationale for this change
   
   Upgrade substrait-go to the latest major version (v9). I'm visiting from 
@substrait-io, and this unblocks the same bump in `iceberg-go`, which cannot 
move to v9 while `arrow/compute/exprs` exposes v8 types in its API.
   
   v9 completes the work of moving protobuf out of substrait-go core. 
Serialization now lives in a separate module, so consumers that only build and 
inspect plans no longer pull the generated types into their dependency graph. 
For arrow-go that means `github.com/substrait-io/substrait-protobuf/go` drops 
out of `go.mod` entirely; it was previously a direct dependency.
   
   Opening as a draft because v9.0.0-alpha.2 is a prerelease. It is published 
and resolvable, but the intent here is to confirm arrow-go compiles and passes 
against the new structure before substrait-go cuts a stable v9. Happy to 
retarget this at the stable tag once it exists.
   
   ### What changes are included in this PR?
   
   - Updated `github.com/substrait-io/substrait-go` from v8.1.1 to 
v9.0.0-alpha.2, and import paths from `v8` to `v9` across `arrow/compute/exprs`
   - Dropped `github.com/substrait-io/substrait-protobuf/go` v0.85.0, which is 
no longer reachable from arrow-go
   - `extensions.ID` is gone in v9, replaced by `extensions.ExtensionID` plus 
three distinct named types over it: `TypeID`, `FunctionID`, and 
`TypeVariationID`. `ExtensionIDRegistry` keys types, type variations and 
functions off a single ID type, so it now uses the common 
`extensions.ExtensionID` and converts at the five call sites where substrait-go 
asks for a specific one. The alternative, splitting the registry three ways, 
would be a much larger break for callers; happy to go that way instead if 
maintainers prefer it.
   - `types.BehaviorReturnNil`, `BehaviorThrowException` and 
`BehaviorUnspecified` became `types.CastFailBehaviorReturnNull`, 
`CastFailBehaviorThrowException` and `CastFailBehaviorUnspecified`, and are now 
a `CastFailBehavior` enum rather than protobuf constants
   - `types.PrecisionTimestamp` is a plain struct in v9 instead of wrapping 
`proto.Expression_Literal_PrecisionTimestamp`, and `PrecisionTime.Precision` is 
now a `types.TimePrecision` rather than `int32`
   
   ### Are these changes tested?
   
   Yes, by the existing tests. `go test ./arrow/compute/...` passes: `compute`, 
`compute/exec`, `compute/exprs`, and `compute/internal/kernels`. No tests were 
added, since this is a dependency upgrade with no behavior change.
   
   I also verified downstream: with `iceberg-go` pointed at this branch, `go 
build ./...` is clean and `go test ./table/...` passes, including 
`table/substrait`.
   
   ### Are there any user-facing changes?
   
   Yes. Users importing `arrow/compute/exprs` will transitively depend on 
substrait-go v9 instead of v8, and will no longer get substrait-protobuf 
transitively from arrow-go.
   
   The exported signatures in `arrow/compute/exprs` that took or returned 
`substrait-go/v8/extensions.ID` now use 
`substrait-go/v9/extensions.ExtensionID`: the `ExtensionIDSet` interface, and 
`ExtensionIDRegistry`'s `RegisterType`, `AddSubstraitScalarToArrow`, 
`GetTypeByID`, `GetIDByType` and `GetSubstraitScalarToArrow`. Callers must 
update, though any caller of these APIs is affected by the substrait-go major 
bump regardless, since the v8 type no longer exists.
   
   I followed the `chore:` convention from #754, the v7 to v8 bump, which was 
also a major upgrade with import path changes. Let me know if you would rather 
mark this one breaking given the signature changes above.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to