Custom types returned by select queries don't satisfy the documented requirements of the Scanner interface

[mihaitodor]

According to the docs of the Scanner interface in the database/sql package:

// The src value will be of one of the following types: // // int64 // float64 // bool // []byte // string // time.Time // nil - for NULL values

However, this code assigns a TimeStamp to the underlying value. Unfortunately, this seems to be incompatible with packages such as encoding/json. Here's a self-contained example (I omitted all the error handling to keep it simple):

> docker run --rm -p 1521:1521 -e ORACLE_PASSWORD=testpass --name oracle gvenzl/oracle-xe:slim-faststart
> docker exec -it oracle bash
>> sqlplus
>> # user: system / password: testpass
>>> CREATE TABLE FOOBAR (TS TIMESTAMP DEFAULT CURRENT_TIMESTAMP);
>>> INSERT INTO FOOBAR VALUES(DEFAULT);
>>> SELECT * FROM FOOBAR;
>>> exit

package main

import (
    "database/sql"
    "encoding/json"
    "fmt"

    _ "github.com/sijms/go-ora/v2"
)

func main() {
    db, _ := sql.Open("oracle", "oracle://system:testpass@localhost:1521/XE")

    rows, _ := db.Query("SELECT * FROM FOOBAR")

    cols, _ := rows.Columns()

    values := make([]any, len(cols))
    valuesWrapped := make([]any, 0, len(cols))
    for i := range cols {
        valuesWrapped = append(valuesWrapped, &values[i])
    }
    _ = rows.Next()
    _ = rows.Scan(valuesWrapped...)

    jObj := map[string]any{}
    for i, v := range values {
        jObj[cols[i]] = v
    }
    b, _ := json.Marshal(jObj)

    fmt.Println(string(b))
}

which outputs {"TS":{}}.

However, if I try the same code using github.com/godror/godror like so:

package main

import (
    "database/sql"
    "encoding/json"
    "fmt"

    _ "github.com/godror/godror"
)

func main() {
    db, _ := sql.Open("godror", "oracle://system:testpass@localhost:1521/XE")

    rows, _ := db.Query("SELECT * FROM FOOBAR")

    cols, _ := rows.Columns()

    values := make([]any, len(cols))
    valuesWrapped := make([]any, 0, len(cols))
    for i := range cols {
        valuesWrapped = append(valuesWrapped, &values[i])
    }
    _ = rows.Next()
    _ = rows.Scan(valuesWrapped...)

    jObj := map[string]any{}
    for i, v := range values {
        jObj[cols[i]] = v
    }
    b, _ := json.Marshal(jObj)

    fmt.Println(string(b))
}

I get {"TS":"2022-12-24T13:43:39.423782Z"} because this code uses time.Time as the underlying type.

Note: I used DYLD_LIBRARY_PATH=./instantclient_19_8 which I downloaded from here.

I'm just speculating now, since I only have superficial knowledge of how various Golang database drivers are implemented, but maybe go-ora needs to do some extra conversions in DataSet.Next(), just like godror does in rows.Next.

I guess I can work around this by adding some go-ora custom type assertions when constructing jObj, but that's very inconvenient...

PS: Thank you for writing and maintaining this amazing package! ❤️

[sijms]

to solve the problem I will implement methods MarshalJSON and UnmarshalJSON

[sijms]

I add new release that solve the problem I add json support for the following types: TimeStamp, NullTimeStamp, NvarChar, NullNVarchar, Clob, NClob, Blob

[mihaitodor]

Hey @sijms, thank you so much for the quick fix! That approach works for me.

Unfortunately, I think something went wrong when you pushed v2.5.21 and, while the git repo looks correct, the code for this version that got cached on https://proxy.golang.org/ doesn't contain the new MarshalJSON and UnmarshalJSON methods. My best guess is that the tag was overwritten somehow after it was first pushed to GitHub, but tags are immutable on this proxy, so go get won't fetch the updated code, since it doesn't use GitHub directly as the source of truth. Would you mind bumping the version again? I think that will fix it.

[mihaitodor]

@sijms not sure if you saw my note above, but have a look on https://pkg.go.dev/github.com/sijms/go-ora/v2@v2.5.21. The new JSON methods aren't available in the v2.5.21 tag that go fetches through its proxy. I think a new tag has to be released for this stuff to work...

[freeNestor]

I have the same problem. v2.5.21 seems not to fix this.

[mihaitodor]

The issue has been resolved in v2.5.22. Thanks @sijms!

[nvx]

I add new release that solve the problem I add json support for the following types: TimeStamp, NullTimeStamp, NvarChar, NullNVarchar, Clob, NClob, Blob

While this fixes specific support for encoding/json, is there any reason for not returning the native types as documented by sql.Scanner? Otherwise this would still cause issues with any other package that expects drivers to conform to what database/sql expects (and may even break in future versions of Go if the database/sql interfaces start to rely on the documented functionality from drivers)

[jimmymackw]

I agree with nvx. This is still an issue. Is there a plan to conform to the database/sql package interface?

[sijms]

could you please specify which interface is not support?

[jimmymackw]

hi @sijms. refer to the doc link @mihaitodor provided in the original post. It provides the list of expected return types to the Scan function. As of now, Scan returns custom type TimeStamp instead of time.Time.

[mihaitodor]

Caveat: I haven't studied Go SQL drivers in detail, so not sure if it's common for people to obey the docs of that Scanner interface and what problems they're trying to address when returning custom types like TimeStamp here. Didn't mean for my issue report to sound authoritative.

[jimmymackw]

As background, I was looking to make a transition from godror/godror to go-ora/v2 but ran into the custom type TimeStamp issue. Older versions of godror/godror used to support the Scan interface types outlined in the docs, however the latest version deviates from this as well. I'm not sure if this is a "miss" in newer development, or if the approach has changed.

[sijms]

if you talk about Scanner.Scan its definition as follow

Scan(src any) error

so it will return error

if you talk about Rows.Scan it should return one of the following

*string
*[]byte
*int, *int8, *int16, *int32, *int64
*uint, *uint8, *uint16, *uint32, *uint64
*bool
*float32, *float64
*interface{}
*RawBytes
*Rows (cursor value)
any type implementing Scanner (see Scanner docs)

and TimeStamp implement Scanner interface

[nvx]

if you talk about Scanner.Scan its definition as follow

Scan(src any) error

so it will return error

if you talk about Rows.Scan it should return one of the following

*string
*[]byte
*int, *int8, *int16, *int32, *int64
*uint, *uint8, *uint16, *uint32, *uint64
*bool
*float32, *float64
*interface{}
*RawBytes
*Rows (cursor value)
any type implementing Scanner (see Scanner docs)

and TimeStamp implement Scanner interface

The problem is the docs say stuff like this:

If an argument has type *interface{}, Scan copies the value provided by the underlying driver without conversion. When scanning from a source value of type []byte to *interface{}, a copy of the slice is made and the caller owns the result.

Source values of type time.Time may be scanned into values of type *time.Time, *interface{}, *string, or *[]byte. When converting to the latter two, time.RFC3339Nano is used.

Which read together makes it sounds like if you scan a timestamp out of a database into any or interface{} that you'd expect it to be set to a time.Time. This is the sort of assumption packages like github.com/joho/sqltocsv make.

In theory you could implement every interface the out of the box types do, but it still means that code would have to then explicitly depend on go-ora if it needs to eg handle timestamps specially.

I can't think of anything preventing the use of the basic Go types when scanning to any/interface{} pointers.

[sijms]

now I get your point you mean when the receiver is of type *interface{} I should return regular types. I will add it and update release

[sijms]

I fix it in v2.5.28