Pgx: possible bug with enum handling

[dvic]

I read #287, could it be it broke something with enum handling?

I get a panic when trying to insert a custom type (with string being the underlying type):

*pgtype.GenericText is not pgtype.BinaryEncoder: missing method EncodeBinary

The panic occurs on line values.go:166 (function encodePreparedStatementArgument):

argBuf, err := value.(pgtype.BinaryEncoder).EncodeBinary(ci, buf)

Any idea?

[jackc]

I don't think #287 would have caused this.

It's strange. For some reason it is trying to encode text as binary.

Do you have a reproducible case I could try?

[dvic]

I'll see if I can publish a self contained example this weekend, thanks!

[dvic]

I tried to create a reproducible case but it worked as expected, I'll revert my workaround on the production code and see what is different there (as it didn't work there). I'll keep you posted.

[matthewmueller]

I just ran into this as well, did you find a solution @dvic? Maybe related to enums with pointers?

The enum I'm trying to add has the following implementation:

package enum

import "database/sql/driver"

// GENERATED BY POGO. DO NOT EDIT.

// Exchange is the `exchange` enum type from `1`.
type Exchange string

const (

    // ExchangeGdax is the 'GDAX' Exchange.
    ExchangeGdax = Exchange("GDAX")
)

// Value satisfies the sql/driver.Valuer interface for Exchange.
func (exchange Exchange) Value() (driver.Value, error) {
    return string(exchange), nil
}

The table that uses the column looks like this:

type columns struct {
    Exchange  *enum.Exchange    `json:"exchange,omitempty"`
}

And usage looks like this:

// Insert a `order` into the `"1"."orders"` table.
func Insert(db pogo.DB, order *Order) (*Order, error) {
    // get all the non-nil columns and prepare them for the query
    _c, _i, _v := pogo.Slice(getColumns(order), 0)

    // sql insert query, primary key provided by sequence
    sqlstr := `
    INSERT INTO "1"."orders" (` + strings.Join(_c, ", ") + `)
    VALUES (` + strings.Join(_i, ", ") + `)
    RETURNING "id", "timestamp", "exchange", "currency", "side", "strategy", "status", "type", "price", "size", "created_at", "updated_at"
    `
    pogo.Log(sqlstr, _v...)

    cols := &columns{}
    row := db.QueryRow(sqlstr, _v...)
    if e := row.Scan(&cols.ID, &cols.Timestamp, &cols.Exchange, &cols.Currency, &cols.Side, &cols.Strategy, &cols.Status, &cols.Type, &cols.Price, &cols.Size, &cols.CreatedAt, &cols.UpdatedAt); e != nil {
        return nil, e
    }

    return &Order{cols}, nil
}

[dvic]

Did you try scanning it into a string? That works? You can also try I think (*string)(&cols.Exchange)

[matthewmueller]

@dvic Yah I think it should do that as part of calling .Value(). It seems like driver.Valuer is no longer being used.

I'm not sure which commit broke it though. In my case, the #287 did not fix the issue, but it did resolve the panic.

I was able to get this working by implementing the EncodeBinary interface as the error describes in the enum:

package enum

import (
    "database/sql/driver"

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

// GENERATED BY POGO. DO NOT EDIT.

// Exchange is the `exchange` enum type from `1`.
type Exchange string

const (

    // ExchangeGdax is the 'GDAX' Exchange.
    ExchangeGdax = Exchange("GDAX")
)

// Value satisfies the sql/driver.Valuer interface for Exchange.
func (exchange Exchange) Value() (driver.Value, error) {
    return string(exchange), nil
}

// EncodeBinary fn
func (exchange Exchange) EncodeBinary(ci *pgtype.ConnInfo, buf []byte) (newBuf []byte, err error) {
    return append(buf, []byte(exchange)...), nil
}

[MOZGIII]

I also stumbled upon this issue.
pgx version e44f0f24c4bb7e7fcfaa6057497e0a61d27d5eac

This is a tricky bug - sometimes it works without errors, sometimes it gives this error when I try to INSERT a value that is an enum.

More details:
When I start the server it either works or it doesn't; stopping and starting the server again helps to randomly get to the working state.

Seems like a race condition with OID resolution or something like that.

[tejasmanohar]

I just ran across the following test code when I was upgrading pgx in a big repo--

    args := make([][]interface{}, len(sch.Properties))
    i := 0
    for key, property := range sch.Properties {
        args[i] = []interface{}{collectionID, key, property.Type, property.ValueReceived, property.Renames, property.Enabled, property.CreatedAt, property.LastSeenAt}
        i++
    }

    if _, err := conn.CopyFrom(
        pgx.Identifier{"collections_properties"},
        []string{"collection_id", "key", "type", "value_received", "renames", "enabled", "created_at", "last_seen_at"},
        pgx.CopyFromRows(args),
    ); err != nil {
        t.Fatal(err)
    }

It was working on super old pgx but now failing with the same error-
*pgtype.GenericText is not pgtype.BinaryEncoder: missing method EncodeBinary

I simply wrapped property.Type (which maps to the typeenum field) with string(property.Type) and it worked. Not sure if this helps others

[jackc]

Hmm. That's pretty strange. I expect that GenericText wouldn't work because CopyFrom requires all values to implement EncodeBinary. But I'm surprised that casting it to a string does work. I would expect that to automatically convert to a pgtype.Text or pgtype.Varchar which does implement EncodeBinary. But it would be the binary of the string -- not the integer value underlying the enum.

[lbcjbb]

Hi,

The problem that I encounter is the same here.

pgx version used: bd37aaaa6a808a2b328db9e6330cb656d2526464 (11-10-2018)
go version used: 1.11

A simple database schema:

CREATE TYPE my_type AS ENUM ('value1', 'value2', 'value3');
CREATE TABLE my_table (
   foo my_type[]
);

And the code below:

conn.Exec(
    "INSERT INTO my_table VALUES ($1)",
    []string{"value1"},
)

```
panic: interface conversion: *pgtype.EnumArray is not pgtype.BinaryEncoder: missing method EncodeBinary

goroutine 1 [running]:
github.com/jackc/pgx.encodePreparedStatementArgument(0xc0000fe1c0, 0xc0000da400, 0xd, 0x400, 0x4093, 0x67ee40, 0xc0000ff1a0, 0x0, 0x0, 0x3, ...)
$GOPATH/src/github.com/jackc/pgx/values.go:192 +0x441
github.com/jackc/pgx.appendBind(0xc0000da400, 0x6, 0x400, 0x0, 0x0, 0x0, 0x0, 0xc0000fe1c0, 0xc000015340, 0x1, ...)
$GOPATH/src/github.com/jackc/pgx/messages.go:184 +0x4fe
github.com/jackc/pgx.(Conn).sendPreparedQuery(0xc0000d4000, 0xc0000a2730, 0xc0000bde78, 0x1, 0x1, 0x0, 0xc0000a2730)
$GOPATH/src/github.com/jackc/pgx/conn.go:1353 +0x1fc
github.com/jackc/pgx.(Conn).execEx(0xc0000d4000, 0x71fda0, 0xc0000140f0, 0x6dfeff, 0x20, 0x0, 0xc0000bde78, 0x1, 0x1, 0x0, ...)
$GOPATH/src/github.com/jackc/pgx/conn.go:1806 +0x3ef
github.com/jackc/pgx.(Conn).ExecEx(0xc0000d4000, 0x71fda0, 0xc0000140f0, 0x6dfeff, 0x20, 0x0, 0xc0000bde78, 0x1, 0x1, 0x0, ...)
$GOPATH/src/github.com/jackc/pgx/conn.go:1747 +0x191
github.com/jackc/pgx.(Conn).Exec(0xc0000d4000, 0x6dfeff, 0x20, 0xc0000bde78, 0x1, 0x1, 0xc000015330, 0xc, 0x0, 0x0)
$GOPATH/src/github.com/jackc/pgx/conn.go:1387 +0x91
...

(conn is a `*pgx.Conn` object created by `pgx.Connect` with **PreferSimpleProtocol** to *false*).

With this quick and dirty patch below, the application doesn't crash and the data is correctly inserted into the database.

```diff
diff --git a/values.go b/values.go
index 0c571d7..482ff9e 100644
--- a/values.go
+++ b/values.go
@@ -189,7 +189,15 @@ func encodePreparedStatementArgument(ci *pgtype.ConnInfo, buf []byte, oid pgtype

                sp := len(buf)
                buf = pgio.AppendInt32(buf, -1)
-               argBuf, err := value.(pgtype.BinaryEncoder).EncodeBinary(ci, buf)
+               var argBuf []byte
+               switch valueEncoder := value.(type) {
+               case pgtype.BinaryEncoder:
+                       argBuf, err = valueEncoder.EncodeBinary(ci, buf)
+               case pgtype.TextEncoder:
+                       argBuf, err = valueEncoder.EncodeText(ci, buf)
+               default:
+                       panic("STOP")
+               }
                if err != nil {
                        return nil, err
                }

[bobheadxi]

The above patch worked for me as well - has anyone submitted a PR?

update: I've implemented @lbcjbb 's solution in #518

[MOZGIII]

I don't like a panic in there...

[bobheadxi]

yeah i didn't either, changed the default case to an error:

        var argBuf []byte
        switch valueEncoder := value.(type) {
        case pgtype.BinaryEncoder:
            argBuf, err = valueEncoder.EncodeBinary(ci, buf)
        case pgtype.TextEncoder:
            argBuf, err = valueEncoder.EncodeText(ci, buf)
        default:
            return nil, fmt.Errorf("invalid encode type %v", valueEncoder)
        }

[MOZGIII]

Patch looks good. I wonder though if we should pick a different approach and enable BinaryEncoder to work there. I'm not sure, but in the back of my mind something tells me there were concerns about not using TextEncoder here in the first place (as it was not there before).
@jackc what do you think?

[jackc]

Patch looked good to me. I merged it in. As far as enum and binary, I don't think that would be possible without inspecting what are the underlying integer values in the PG database and generating a PG type from that. That'd be a bit more magic than I'm comfortable with.

[bobheadxi]

thanks @jackc ! will there be a new tagged release soon?

[jackc]

@bobheadxi There are a few more changes I want to get in before the next minor release. Probably be finished within a couple weeks.