Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression in jsonb handling for types implementing sql.Scanner and json.Unmarshaler #2250

Open
iamnoah opened this issue Feb 4, 2025 · 1 comment
Labels

Comments

@iamnoah
Copy link

iamnoah commented Feb 4, 2025

Describe the bug

In v5.7.1, when scanning into types that implemented both sql.Scanner and json.Unmarshaler, UnmarshalJSON was preferred. Since v5.7.2, Scan is preferred.

To Reproduce

Something like this shows the difference:

package main

import (
	"context"
	"fmt"
	"github.com/stretchr/testify/require"
	"testing"
)

type JsonB struct {
	Val string
}

func (x *JsonB) Scan(src any) error {
	switch s := src.(type) {
	case []byte:
		return x.UnmarshalJSON(s)
	}
	return fmt.Errorf("unable to convert %T to %T", src, x)
}

func (x *JsonB) UnmarshalJSON(s []byte) error {
	x.Val = string(s)
	return nil
}

func TestJsonB(t *testing.T) {
	db, err := pgx.Connect(context.Background(), os.Getenv("DATABASE_URL"))
	if err != nil {
		log.Fatal(err)
	}
	defer db.Close(context.Background())

	var foo *JsonB
	rows, err := db.Query(context.Background(), `select '{"foo": "bar"}'::jsonb as foo`)
	require.NoError(t, err)
	require.True(t, rows.Next())
	require.NoError(t, rows.Scan(&foo))
	require.Equal(t, `{"foo": "bar"}`, foo.Val)
}

Passes on v5.7.1, but errors on v5.7.2 and master as of right now.

Expected behavior

UnmarshalJSON is invoked.

Actual behavior

Scan is invoked with a string argument instead.

Version

  • pgx: v5.7.2

Additional Context

This isn't hard to fix/workaround, but it is a change in behavior in a patch release.

@iamnoah iamnoah added the bug label Feb 4, 2025
@felix-roehrich
Copy link
Contributor

In pgx the sql.Scanner interface is preferred over other possibilities. JSON(B) was the exception under certain circumstances (see #2146 and #2151). This was changed in 5.7.2, but forgotten in the patch notes by @jackc. Thus, this change in behaviour is a bug fix.

I would leave this open until the patch notes are updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants