jackc / pgx

PostgreSQL driver and toolkit for Go
MIT License
10.41k stars 825 forks source link

`driver.Valuer` not handled properly on pointer receivers #443

Open johngibb opened 6 years ago

johngibb commented 6 years ago

Because the encodePreparedStatementArgument dereferences the pointer before checking for the driver.Valuer interface, it will never be used for types which define Value() on a pointer receiver.

i.e. this works:

type MyType struct {}
func (m MyType) Value() (driver.Value, error)

but this doesn't

type MyType struct {}
func (m *MyType) Value() (driver.Value, error)
WGH- commented 2 years ago

I believe this affects not only driver.Valuer, but also things like json.Marshaler. For example, the following program:

package main

import (
    "context"
    "encoding/json"
    "log"

    "github.com/jackc/pgx/v4"
)

type valueReceiver struct {
    InStruct int
}

func (a valueReceiver) MarshalJSON() ([]byte, error) {
    return json.Marshal(struct {
        OverridenInMarshalJSON int
    }{OverridenInMarshalJSON: a.InStruct})
}

type pointerReceiver struct {
    InStruct int
}

func (b *pointerReceiver) MarshalJSON() ([]byte, error) {
    return json.Marshal(struct {
        OverrideInMarshalJSON int
    }{OverrideInMarshalJSON: b.InStruct})
}

func main() {
    conn, err := pgx.Connect(context.Background(), "")
    if err != nil {
        log.Fatal(err)
    }

    for _, x := range []interface{}{valueReceiver{1}, &valueReceiver{2}, pointerReceiver{3}, &pointerReceiver{4}} {
        var y interface{}
        err := conn.QueryRow(context.Background(), "SELECT $1::json", x).Scan(&y)
        if err != nil {
            log.Fatal(err)
        }
        log.Printf("%#v -> %v", x, y)
    }
}

Prints:

2021/09/28 01:48:11 main.valueReceiver{InStruct:1} -> map[OverridenInMarshalJSON:1]
2021/09/28 01:48:11 &main.valueReceiver{InStruct:2} -> map[OverridenInMarshalJSON:2]
2021/09/28 01:48:11 main.pointerReceiver{InStruct:3} -> map[InStruct:3]
2021/09/28 01:48:11 &main.pointerReceiver{InStruct:4} -> map[InStruct:4]
gogingersnap777 commented 3 months ago

Looking into this issue today

kbrgl commented 3 months ago

@gogingersnap777 Were you able to figure out why this issue occurs? Spent ~2 hours debugging my code until I found this thread (fortunately) 😅

jackc commented 3 months ago

I'm not able to duplicate this on the current version of pgx.

package main

import (
    "context"
    "database/sql/driver"
    "fmt"
    "log"

    "github.com/jackc/pgx/v5"
)

type pointerReceiver struct {
    InStruct int
}

func (b *pointerReceiver) Value() (driver.Value, error) {
    return fmt.Sprintf("OverridenInValue: %v", b.InStruct), nil
}

func main() {
    conn, err := pgx.Connect(context.Background(), "")
    if err != nil {
        log.Fatal(err)
    }

    x := &pointerReceiver{1}
    var y interface{}
    err = conn.QueryRow(context.Background(), "SELECT $1::text", x).Scan(&y)
    if err != nil {
        fmt.Printf("Error: %v\n", err)
        return
    }
    fmt.Printf("%#v -> %v\n", x, y)
}

Output:

&main.pointerReceiver{InStruct:1} -> OverridenInValue: 1

It's calling Value defined on the pointer.

gogingersnap777 commented 3 months ago

I searched through the code for any examples of the below:

type MyType struct {}
func (m *MyType) Value() (driver.Value, error)

The only example I found was below, but I believe this is ok since it is written specifically for a test.

func (v *nilPointerAsEmptyJSONObject) Value() (driver.Value, error) {
    if v == nil {
        return "{}", nil
    }

    return json.Marshal(v)
}

Given the description, I believe this issue is a no-op now!

gogingersnap777 commented 3 months ago

@jackc

Seperate question, I tried running the code example you posted but I'm getting the below. Is there something I need to set up that I'm missing?

Steps

go mod init hello
touch main.go
# copy your code
pbpaste > main.go
go mod tidy 
go run main.go
$ go run main.go
2024/06/04 20:28:55 failed to connect to `user=zach database=`: /private/tmp/.s.PGSQL.5432 (/private/tmp): dial error: dial unix /private/tmp/.s.PGSQL.5432: connect: no such file or directory
exit status 1
jackc commented 3 months ago

My code is expecting all database connection information to be defined in PG* variables like PGDATABASE. You would either need to set those environment variables or change the pgx.Connect call to use your connection string.