googleapis / googleapis/google-cloud-go
spanner: support pointer-to-named-base-type values (e.g. *MyString)
- Dominant language
- Go
- Stars
- 4.5k
- Forks
- 1.6k
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 109
Description
## Is your feature request related to a problem? Please describe.
The Spanner Go client treats `T`, `*T`, and `type Foo T` (named base type) as valid Statement params and decode destinations, but it rejects `*Foo` — a pointer to a named base type. This breaks the symmetry users naturally expect once `*T` works.
Today:
| Form | Encode | Decode |
| ----------------- | ------ | ------ |
| `string` | ✅ | ✅ |
| `*string` | ✅ | ✅ |
| `type Foo string` | ✅ | ✅ |
| `*Foo` | ❌ | ❌ |
The same asymmetry exists for every other base type the SDK supports (full table below).
Concrete reproducer (encode):
```go
type UserID string
// Works:
_ = spanner.Statement{
SQL: "SELECT * FROM Users WHERE Id = @id",
Params: map[string]interface{}{"id": UserID("u-123")},
}
// Fails with: rpc error: code = InvalidArgument
// "client doesn't support type *pkg.UserID"
managerID := UserID("u-456")
_ = spanner.Statement{
SQL: "SELECT * FROM Users WHERE ManagerId = @mid",
Params: map[string]interface{}{"mid": &managerID},
}
```
Decode side:
```go
type UserID string
var id UserID // works
var managerID *UserID // fails: errTypeMismatch
if err := row.Column(0, &managerID); err != nil { /* ... */ }
```
This pattern (`type UserID string` plus `*UserID` for nullable foreign keys, optional fields, etc.) is idiomatic in domain-typed Go code.
The current workarounds — implementing `spanner.Encoder`/`spanner.Decoder` on every named type, dropping the named type, or wrapping in `spanner.NullString` — are all boilerplate-heavy for what users perceive as a basic case.
## Describe the solution you'd like
Make `T` and `*T` behave symmetrically for named base types: if `MyString` works as a param or destination, `*MyString` should also work, with `nil` mapping to SQL `NULL` the same way `*string` does today.
The same symmetry should also hold for slices — `[]MyString` works today, `[]*MyString` should work too (matching how `[]string` and `[]*string` are both supported).
Concretely (paths relative to `spanner/value.go` on `main`):
- Extend the `reflect.Ptr` branch in `getDecodableSpannerType`
([`value.go:3180-3190`](https://github.com/googleapis/google-cloud-go/blob/e10c9bf14d4fc23906d0b369e106420d44915660/spanner/value.go#L3180-L3190)) to detect pointers whose elem `Kind` matches a supported primitive or known struct base type, and return a new `spannerTypePtrNonNull*` enum value.
- Mirror the same extension in the slice-handling `Ptr` sub-branch
([`value.go:3266`](https://github.com/googleapis/google-cloud-go/blob/e10c9bf14d4fc23906d0b369e106420d44915660/spanner/value.go#L3266)) so `[]*Foo` maps to a new `spannerTypeArrayOfPtrNonNull*` enum value.
- Add the new enum constants alongside the existing `decodableSpannerType`
values (around [`value.go:3058`](https://github.com/googleapis/google-cloud-go/blob/e10c9bf14d4fc23906d0b369e106420d44915660/spanner/value.go#L3058)).
- Teach `convertCustomTypeValue` ([`value.go:5360`](https://github.com/googleapis/google-cloud-go/blob/e10c9bf14d4fc23906d0b369e106420d44915660/spanner/value.go#L5360)) to unwrap nil/non-nil pointers — `nil` produces a `NULL` proto + type info (the same path `case *string:` at [`value.go:4840`](https://github.com/googleapis/google-cloud-go/blob/e10c9bf14d4fc23906d0b369e106420d44915660/spanner/value.go#L4840) already uses), non-nil indirects and reuses the existing `spannerTypeNonNull*` conversion. The slice variants reuse the existing per-element `Convert` loop ([`value.go:5543-5548`](https://github.com/googleapis/google-cloud-go/blob/e10c9bf14d4fc23906d0b369e106420d44915660/spanner/value.go#L5543-L5548)), which already handles `*MyString` → `*string` element conversion correctly.
- Also add the missing `case spannerTypeNonNullInterval` (and
`spannerTypeNullInterval` for completeness) in `convertCustomTypeValue` and `decodeValueToCustomType`. These cases are absent today, which is why named `Interval` types fail with `unknown decodable type found: 24` — and without them, the new `*MyInterval` support cannot work.
- Update `isSupportedMutationType` ([`value.go:5710-5737`](https://github.com/googleapis/google-cloud-go/blob/e10c9bf14d4fc23906d0b369e106420d44915660/spanner/value.go#L5710-L5737)) so the Mutation encode gate also admits these types (otherwise `*Foo` / `[]*Foo` work in DML/queries but still fail in Mutations).
- Mirror the change on the decode side in `decodeValueToCustomType`
([`value.go:3336`](https://github.com/googleapis/google-cloud-go/blob/e10c9bf14d4fc23906d0b369e106420d44915660/spanner/value.go#L3336)): allocate a new named value via `reflect.New(elem)` + `Convert` + `Indirect(destination).Set(...)`, or set the destination pointer to `nil` on `NULL`.
Affected base types:
| Base type family | `T` today | `*T` today | `type Foo T` today | `*Foo` today | `*Foo` after fix |
| ------------------- | --------- | ---------- | ------------------ | ------------ | ---------------- |
| `string` | ✅ | ✅ | ✅ | ❌ | ✅ |
| `int64` | ✅ | ✅ | ✅ | ❌ | ✅ |
| `bool` | ✅ | ✅ | ✅ | ❌ | ✅ |
| `float64` | ✅ | ✅ | ✅ | ❌ | ✅ |
| `float32` | ✅ | ✅ | ✅ | ❌ | ✅ |
| `time.Time` | ✅ | ✅ | ✅ | ❌ | ✅ |
| `civil.Date` | ✅ | ✅ | ✅ | ❌ | ✅ |
| `Interval` | ✅ | ❌ | ❌ | ❌ | ✅ |
| `big.Rat` (Numeric) | ✅ | ✅ | ✅ | ❌ | ✅ |
| `uuid.UUID` | ✅ | ✅ | ✅ | ❌ | ✅ |
The same asymmetry and fix apply analogously to the slice variants `[]Foo` / `[]*Foo` of every row above. The `Interval` row's extra `❌` cells (`*T today` and `type Foo T today`) reflect pre-existing bugs separate from the named-pointer asymmetry: `convertCustomTypeValue` and `decodeValueToCustomType` lack a `case spannerTypeNonNullInterval`, so any named `Interval` value fails with `unknown decodable type found: 24`, and `*Interval` falls through `encodeValue`'s default and gets mis-encoded via the struct walker.
The proposed fix covers these along with the named-pointer support, since `*MyInterval` cannot work without them.
## Describe alternatives you've considered
1. **Implement `spanner.Encoder` / `spanner.Decoder` on each named type.**
Works, but requires a non-trivial pair of methods per named type and defeats the “just use your domain type” ergonomics that named base types are supposed to provide.
2. **Cast to the underlying base type at the call site**
(`(*string)(&managerID)`).
Loses domain typing at exactly the boundary where it matters and is easy to forget in one of many call sites.
3. **Use `spanner.NullString` (etc.) instead of `*MyString`.**
Works for nullability but mixes SDK wrapper types into the domain model and forces a different field type than the non-nullable case.
None of these are clean enough to justify the asymmetry — and notably, the SDK already provides the symmetric experience for non-named base types.
## Additional context
### Historical context
This case is mentioned in fbe1038431 (“spanner: Allow encoding and decoding to pointers”, 2020-01-20):
> This CL does NOT add support for decoding and encoding to pointers to
> custom types that point back to base types. I.e. the following struct
> cannot be used to decode rows from Spanner.
>
> ```go
> type CustomString string
> type Entity struct {
> ID *CustomString
> }
> ```
The commit message explicitly notes that this case is not covered by that change; I don’t see any later commit that either revisits it or reaffirms the cut.
Generalizing the `reflect.Ptr` branch in `getDecodableSpannerType` to “pointer to anything whose elem `Kind` matches a supported base” extends the same intent as that original commit, just covering the case it deferred.
### Related issues / PRs
- #1684 — original
“support pointer fields” request (closed by `fbe1038431`, which explicitly deferred this case).
- #11007, #12222, #12496 — later
refinements to custom-type support; none target this asymmetry.
- No existing open issue specifically tracks `pointer to named base type`
support, as far as I can tell.
### Environment
```
$ go version
go version go1.25.x darwin/arm64
```
Reproducible against current `main` of `cloud.google.com/go/spanner` (also reproduces on the latest released minor version).
### Not in scope (for this issue)
Calling these out so they don’t get folded into the same discussion:
- `*MyNullString` and similar (named pointer to `Null*` wrappers) — out of scope.
`Null*` already expresses nullability without a pointer, so `*MyNullString` would give a single field two ways to say NULL. Whether to support this is a separate API design question.
Contributor guide
Assessment
This issue has not been assessed yet.