jmoiron/sqlx

Error while doing a MapScan or SliceScan time.Time with PostgreSQL

frankbb opened this issue · 7 comments

Not really sure if it's a sqlx issue. When i use a MapScan or a SliceScan in combination with a Postgresql timestamp field it will result in a scan error, or did i miss something?

rows, err := db.NamedQuery(query, where)
[...] -> err
for rows.Next() {
        results := make(map[string]interface{})
        err = rows.MapScan(results)
        [...] -> err

        fmt.Println(fmt.Sprintf("%v", results)))
}

Error:

sql: Scan error on column index 3: unsupported driver -> Scan pair: time.Time -> *string

Database Postgresql column:

\d users
  Column  |            Type             |                     Modifiers
----------+-----------------------------+----------------------------------------------------
 id       | integer                     | not null default nextval('users_id_seq'::regclass)
 added    | timestamp without time zone | not null default now()

MapScan tries to scan everything out as a string. You should really be using a time.Time datatype to pull out timestamps--but if you want to do this with MapScan, you can convert to a string in the query with to_char or something.

Yes, this is a bug. The way lib/pq supports time natively, sql.RawBytes/[]byte/string/NullString aren't a safe destination for it. I have to change this code to use *interface{} instead, but that will probably be a breaking change. The way I did it wasn't good though; I learned this when writing the Unsafe() methods.

edit:

The map[string]*string interface is a lot more convenient than map[string]interface{} for most cases where MapScan is actually a good API, because you won't have a good idea of what the types are and a string representation is probably good enough. Still it's probably good enough to let fmt.Sprint do that work for you.

Do you have a suggestion for a workaround? Maybe you can also add this to the documentation for now.

In case of a SELECT * FROM A you don't know what type you are selecting so as far as i know there is no place for a to_char.

@frankbb Fixing this is ironically a breaking change. I will create a branch for you to use which fixes this while I gather a bunch of planned breaking changes together for a single release + announcement.

Even though the current behavior is convenient, I think it is wrong. It's also unnecessarily opinionated to coerce everything to string or nil in the first place; if that's what someone desires, they could surely just fmt.Sprint the results for themselves.

Great, thanks i will try that branch, if i run into bugs i will let you know.

This has been integrated into master now, along with many other fixes.