spanner: possible row mismatch in SelectAll using custom type

[jonasv3]

Client

Spanner

Environment

go version go1.23.2 darwin/arm64

Code and Dependencies

package main

func main() {
    // spannerClient created per best practices
    rowIter := spannerClient.Single().Query(
        context.Background(),
        spanner.Statement{SQL: `
        SELECT
            Id,
            OtherId,
        FROM Items
        ORDER BY OtherId DESC
        `},
    )
    defer rowIter.Stop()

    var dest []struct {
        Id        string
        OtherId Inner[string]
    }

    if err := spanner.SelectAll(rowIter, &dest); err != nil {
        t.Fatal(err)
    }
    for _, d := range dest {
        fmt.Println(d)
    }
}

type Inner[T any] struct {
    val T
}

func (n *Inner[T]) DecodeSpanner(input any) error {
    switch val := input.(type) {
    case T:
        n.val = val
        return nil
    case *T:
        if val == nil {
            return nil
        }
        n.val = *val
        return nil
    }
    panic("n/a")
}

go.mod

```text module modname go 1.23 require ( cloud.google.com/go/spanner v1.70.0 google.golang.org/api v0.201.0 google.golang.org/grpc v1.67.1 … ) ```

Expected behavior

Given the following data in the table Items:

Id        OtherId
715e8f8d  None
5865d2cd  foobar

with the schema

CREATE TABLE IF NOT EXISTS Items (
    Id STRING(36) DEFAULT (GENERATE_UUID()),
    OtherId STRING(MAX),
) PRIMARY KEY (Id);

and running the above code, I would expect

{5865d2cd {foobar}}
{715e8f8d {}}

Actual behavior

I see a row mismatch, where the row 715e8f8d somehow inherits the value foobar from 5865d2cd.

{5865d2cd {foobar}}
{715e8f8d {foobar}}

(guids truncated for pith)

Additional context

Is not an issue when iterating and decoding using ToStruct().

[jonasberdal]

This happens if the spanner.Decoder implementation assumes it is called on a freshly initialized struct and doesn't write the zero value back.

[jonasberdal]

I was working on a bug report of my own. Here is my MRE in case it is of any use:

package main

import (
    "context"
    "fmt"
    "os"
    "reflect"
    "time"

    "cloud.google.com/go/spanner"
    database "cloud.google.com/go/spanner/admin/database/apiv1"
    adminpb "cloud.google.com/go/spanner/admin/database/apiv1/databasepb"
)

type DbModel struct {
    TestId   string
    TestData Optional
}

type Optional struct {
    Value *string
}

func (o *Optional) DecodeSpanner(input any) error {
    var t string
    switch typedSrc := input.(type) {
    case string:
        o.Value = &typedSrc
    case *string:
        if typedSrc != nil {
            o.Value = typedSrc
        }
        // null vale of optional is {Value: nil}, so we don't explicitly set it to nil.
        // SellectAll reuse the pointers between rows, so the previous decoded value will not be overwritten
    default:
        return fmt.Errorf("failed to scan src to type: %s. Was type: %s. Value: %+v", reflect.TypeOf(t), reflect.TypeOf(input), input)
    }
    return nil
}

func main() {
    ctx, cancelFunc := context.WithDeadline(context.Background(), time.Now().Add(100*time.Hour))
    defer cancelFunc()
    spannerClient, err := CreateDatabase(ctx)
    if err != nil {
        panic(err)
    }
    defer spannerClient.Close()

    _, err = spannerClient.ReadWriteTransaction(ctx, func(ctx context.Context, tx *spanner.ReadWriteTransaction) error {
        _, txErr := tx.Update(ctx, spanner.Statement{
            SQL: `
            INSERT INTO TestTable
                ( TestId, TestData)
            VALUES
                (@test_id, @test_data)`,
            Params: map[string]interface{}{
                "test_id":   "id2",
                "test_data": "has data",
            },
        })
        if txErr != nil {
            return txErr
        }

        _, txErr = tx.Update(ctx, spanner.Statement{
            SQL: `
            INSERT INTO TestTable
                ( TestId)
            VALUES
                (@test_id)`,
            Params: map[string]interface{}{
                "test_id": "id1",
            },
        })
        return txErr
    })
    if err != nil {
        panic(err)
    }

    rowIter := spannerClient.Single().Query(
        ctx,
        spanner.Statement{SQL: `
        SELECT
            TestId,
            TestData,
        FROM TestTable
        `},
    )
    defer rowIter.Stop()

    var dest []DbModel
    if err = spanner.SelectAll(rowIter, &dest); err != nil {
        panic(err)
    }
    for _, row := range dest {
        fmt.Printf("TestId: %s, TestData: %s\n", row.TestId, *row.TestData.Value)
    }
}

func CreateDatabase(ctx context.Context) (*spanner.Client, error) {
    err := os.Setenv("SPANNER_EMULATOR_HOST", "localhost:9010")
    if err != nil {
        return nil, fmt.Errorf("failed to set SPANNER_EMULATOR_HOST: %w", err)
    }
    instance := "projects/test-project/instances/test-instance"
    dbName := "select-all-bug"
    db := fmt.Sprintf("%s/databases/%s", instance, dbName)

    adminClient, err := database.NewDatabaseAdminClient(ctx)
    if err != nil {
        return nil, fmt.Errorf("failed to create admin client: %w", err)
    }
    defer adminClient.Close()

    err = adminClient.DropDatabase(ctx, &adminpb.DropDatabaseRequest{
        Database: db,
    })
    if err != nil {
        return nil, fmt.Errorf("failed to drop database %s: %w", db, err)
    }

    tableSchemas := []string{
        `CREATE TABLE TestTable (
                TestId STRING(36),
                TestData  STRING(1024),
        ) PRIMARY KEY (TestId)`,
    }

    op, err := adminClient.CreateDatabase(ctx, &adminpb.CreateDatabaseRequest{
        Parent:          instance,
        CreateStatement: "CREATE DATABASE `" + dbName + "`",
        ExtraStatements: tableSchemas,
    })
    if err != nil {
        return nil, err
    }
    if _, err = op.Wait(ctx); err != nil {
        return nil, err
    }

    dataClient, err := spanner.NewClient(ctx, db)
    if err != nil {
        return nil, err
    }

    return dataClient, nil
}

[rahul2393]

@jonasv3 Thanks for opening the issue, can you please verify if updating DecodeSpanner method with below code fixes your issue?

type Inner[T any] struct {
    val T
}

func (n *Inner[T]) DecodeSpanner(input any) error {
    // Create a new instance for each decode operation
    *n = Inner[T]{} // Reset the value first

    switch val := input.(type) {
    case T:
        n.val = val
        return nil
    case *T:
        if val == nil {
            return nil
        }
        n.val = *val
        return nil
    }
    return fmt.Errorf("unsupported type for Inner[T]: %T", input)
}