Thanatos9404 opened a new pull request, #40649:
URL: https://github.com/apache/beam/pull/40649
Fixes #21789
The generic registration functions (`register.Function*`, `register.DoFn*`
and `register.Combiner*`) call `registerMethodTypes`, which passes every struct
type a function or method takes or returns to `runtime.RegisterType` and
`schema.RegisterType`. `runtime.RegisterType` panics for types without a type
key, which is every unnamed type, so a function with an anonymous struct
parameter panicked in `init()`:
```
panic: invalid registration type: struct { Counter *int64
"beam:\"counter\""; ... }
```
The panic happened before the caller and the DoFn struct wrappers were
registered, so generic registration couldn't be used for such a function at all.
`registerMethodTypes` now adds a struct type to the runtime type registry
only when it has a type key, and still adds every struct type to the schema
registry. I added a small `registerStructType` helper for this rather than
reusing `registerType` (used for emitters and iterators), because
`registerType` skips unnamed types entirely, including the schema registration.
The schema registration matters for these types. graphx encodes a type
without a type key field by field, but the schema registry is what maps a row
schema back to the user's Go type on the worker. Without it, some unnamed
structs come back as a different type: one with `json` tags comes back without
them, and one with a named struct field comes back with an unnamed copy of that
field's type. I checked this with two fresh `schema.NewRegistry()` instances
standing in for the launcher and the worker; with the type registered on the
worker, both come back identical. As with named struct types, an unnamed struct
that can't be converted to a schema (for example one with a chan or interface
field) now fails when `beam.Init` reconciles the schema registry, instead of
panicking earlier in `runtime.RegisterType`.
I also removed the TODO in
`test/integration/io/xlang/bigquery/bigquery_test.go` and enabled the
`register.Function1x1(castFn)` call it was waiting on (`castFn` takes
`TestRowPtrs`, an alias of an anonymous struct). I could only check that the
test binary initialises: it panics at init on master and starts cleanly with
this change. I didn't run the integration test itself, since it needs BigQuery
and an expansion service.
The change is in `register.tmpl`; `register.go` was regenerated with `go
generate` in `pkg/beam/register`, and the only difference is in
`registerMethodTypes` and the new helper.
Tests: three new tests register a function with an anonymous struct
parameter, a function with an anonymous struct return type, and a DoFn with an
anonymous struct parameter. They check that the unnamed types, and a named
struct type used only as a field of one of them, are in the schema registry,
that the named return and DoFn types are in both registries, and that the
caller and struct wrapper are registered and callable. All three fail on master
with the panic above, and the schema checks also fail if unnamed types are
skipped entirely. Locally (Go 1.27.2, Windows):
- `go test ./pkg/beam/register/... ./pkg/beam/core/runtime/...
./pkg/beam/core/graph/... ./pkg/beam/ ./pkg/beam/transforms/...
./pkg/beam/testing/...`: all pass except `xlangx/expansionx`
`TestNewJarGetter`, which fails the same way on master on Windows (it expects a
`~/` cache path)
- `go vet` and `gofmt -l` on the changed packages: clean
- `go generate` in `pkg/beam/register` reproduces the committed `register.go`
I updated `CHANGES.md` under Bugfixes for 2.78.0.
--
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]
