Scan(interface{}) produces Go int64 instead of int32 for PG integer

[felixge]

In the process of upgrading telegraf to use the latest version of pgx from b84338d7d62598f75859b2b146d830b22f1b9ec8 to 63f58fd32edb5684b9e9f4cfaac847c6b42b3917 I noticed that some of their regression tests started to fail. E.g.:

--- FAIL: TestPostgresqlFieldOutput (0.01s)
    <autogenerated>:1: 

    Error Trace:    postgresql_extensible_test.go:201

    Error:          Should be true

    Messages:       expected numbackends to be an int32
    <autogenerated>:1: 

    Error Trace:    postgresql_extensible_test.go:211

    Error:          Should be true

    Messages:   

Digging deeper, I was able to produce this demonstration which fails for 63f58fd32edb5684b9e9f4cfaac847c6b42b3917 but passes for b84338d7d62598f75859b2b146d830b22f1b9ec8:

package main

import (
    "database/sql"
    "fmt"
    "os"

    _ "github.com/jackc/pgx/stdlib"
)

func main() {
    db, err := sql.Open("pgx", "")
    if err != nil {
        panic(err)
    }
    var val interface{}
    if err := db.QueryRow("SELECT 1::int").Scan(&val); err != nil {
        panic(err)
    }
    if fmt.Sprintf("%T", val) != "int32" {
        fmt.Printf("%T: %v\n", val, val)
        os.Exit(1)
    }
}

Since Postgres integers are using a storage size of 4 bytes I feel like the old behavior of using a Go int32 in b84338d7d62598f75859b2b146d830b22f1b9ec8 is more appropriate. However, since using an int64 to store an int32 isn't technically wrong, I'm wondering if the current behavior is intentional. If it's intentional, I'll try to adjust the CrateDB test cases. Otherwise I can try to send a patch for pgx that restores the old behavior.

I also tried to identify the first commit that caused the change in behavior using the following command:

git bisect start 63f58fd32edb5684b9e9f4cfaac847c6b42b3917 b84338d && git bisect run go run path/to/main.go

According to bisect 19c668975218ca857f07e0506cdbcaa83f68fb24 is the first commit where the behavior changed.

[jackc]

I don't see anything in that commit that indicates it was changed purposely. However, I believe that the current behavior of returning an int64 is correct. The relevant interface that pgx implements for compatibility with database/sql is database/sql/driver.Rows The Next(dest []Value) error function populates the slice of driver.Value.

driver.Value is a interface{} so we can put anything we want there, but the documentation states that it must only hold values from a short list of types. int64 is on that list and int32 is not.

The previous behavior may have happened to work, but it was not conforming the required interface.

[felixge]

@jackc makes sense. Thanks for clarifying.