Null* isn't compatible with std interface

[sankalp-amigo]

Issue description

Unable to read Rows from a Table that has Null Values anywhere using the 'database/sql' interface. Even though 'sql.Nullstring' is being used. Expected to be able scan into a slice of 'sql.Nulstring' but instead get an error.

Example code

for rows.Next() {
            columns := make([]sql.NullString, len(cols))
            columnPointers := make([]interface{}, len(cols))
            for i := range columns {
                columnPointers[i] = &columns[i]
            }
            err = rows.Scan(columnPointers...)
            // Error occured while scanning a row
            if err != nil {
                fmt.Println()
                fmt.Println(err) ̑
                fmt.Println()
            }

Error log

sql: Scan error on column index 0, name "col1": unsupported Scan, storing driver.Value type *string into type *string

Configuration

OS: Ubuntu 20.04.4 LTS

Interface: database/sql

Driver version: v2.3.0

Go version: go version go1.18.2 linux/amd64

ClickHouse Server version: version 22.9.1

[gingerwizard]

YOu need to pass *string not string to represent the null value i.e. a pointer to a pointer.

[gingerwizard]

e.g. https://github.com/ClickHouse/clickhouse-go/blob/main/examples/clickhouse_api/nullable.go#L63-L75

[gingerwizard]

and https://github.com/ClickHouse/clickhouse-go/blob/main/tests/std/string_test.go#L60-L69

[gingerwizard]

Ok i can reproduce this. It will scan nil but not string values.

[gingerwizard]

we should support this. Workaround is use **string but NullString needs to work.

[gingerwizard]

This is subtly tricky. When using the std interface the method Next and is responsible for reading the rows - it requires the driver to implement func (r *stdRows) Next(dest []driver.Value) error {. Its here we actually read the value from the block. The dest slice gives us no indication as to whether the user intends to Scan into a NullString or string** - as the user later passes this in Scan.

We need to decide in Next whether to return a pointer or primitive string. For null we return a pointer - specifically here. Why? because we support the user reading into a string**.

Now at the point of Scan, the user passes string* for a Nullable(String). The sql.driver layer supports this marshaling of string into string* and it works due to this code. NullString needs a string value, however - and we have a pointer - see 1, then 2. The string to string* isn't supported inside sql.go i.e. here

So in summary, by supporting string** we make supporting NullString tricky. I can't see any extension points in Scan to make this happen.

[sankalp-amigo]

Using this workaround, I'm able to Read Nullable Strings but unable to read Nullable Integer or Float Columns that have any value (other than Null) in them. Which I could Read as String when the column was not Null and I was reading into *string.

sql: Scan error on column index 1, name \"fid\": unsupported Scan, storing driver.Value type *int32 into type *string"```

[gingerwizard]

Use float and intX - same principle.

[sankalp-amigo]

I do not have knowledge of the type of the columns of the table I'm querying. I use strings because it is mentioned that numbers are auto converted to strings. Which works fine for the case when none of my columns are Nullable. The Issue only arises when I have a Nullable Column of type NOT string and try to read it as string.

[gingerwizard]

https://github.com/ClickHouse/clickhouse-go/blob/main/examples/std/dynamic_scan_types.go is an example where the column types are unknown.

I agree we need to address this, hence issue is open - I'm just not quite clear how to implement this. Open to PRs and/or ideas.

[stephenafamo]

Is there any update on this?

If supporting both *string and sql.NullString is impossible, I believe it is more in line with the database ecosystem to support sql.NullString

I'm not sure if there are any plans for a v3 but that may be a reasonable time to introduce the breaking change.

[montanaflynn]

I was using sql.Null* and found out it's just inserting zero values which was surprising. I think a note in the docs should be added, I had to search through issues to find this behavior.