use sqlite3_column_type() for ColumnTypeScanType() #1327
Conversation
| return TYPE_NULLSTRING | ||
| case C.SQLITE_BLOB: | ||
| return TYPE_RAWBYTES | ||
| //case C.SQLITE_NULL: |
There was a problem hiding this comment.
Why is this commented out?
There was a problem hiding this comment.
If it doesn't apply, then just remove the code?
There was a problem hiding this comment.
Its to document that the omission is intentional and not a miss, this is the actual value that we would be getting in 2 cases:
- If we call before we have a valid row
- If the value for the row-column is NULL
Happy to remove it or add a comment clarifying.
There was a problem hiding this comment.
It looks like a mistake. An explicit comment is better.
There was a problem hiding this comment.
Added comment. Please take another look.
| SQLITE_NULL | ||
| ) | ||
|
|
||
| var ( |
There was a problem hiding this comment.
Why are these all public?
| TYPE_RAWBYTES = reflect.TypeOf(sql.RawBytes{}) | ||
| TYPE_NULLBOOL = reflect.TypeOf(sql.NullBool{}) | ||
| TYPE_NULLTIME = reflect.TypeOf(sql.NullTime{}) | ||
| TYPE_ANY = reflect.TypeOf(new(any)) |
There was a problem hiding this comment.
Well this is unfortunate - this means we are saying *any instead of any. I know that's the existing behavior, but does that even work?
| func (rc *SQLiteRows) ColumnTypeScanType(i int) reflect.Type { | ||
| //ct := C.sqlite3_column_type(rc.s.s, C.int(i)) // Always returns 5 | ||
| switch C.sqlite3_column_type(rc.s.s, C.int(i)) { |
There was a problem hiding this comment.
If the SQL statement does not currently point to a valid row, or if the column index is out of range, the result is undefined.
We cannot call this if we haven't called Next yet, or if we have iterated past the last row, even if it happens to work today.
|
Please add a unit test for the new behavior. |
The current implementation was changed in PR-909 stating that
sqlite3_column_type()always returns SQLITE_NULL, but as noted in ISSUE-600, the case was probably because for SQLite3,sqlite3_column_type()must be called after callingNext(). sqlite3_column_type() returns the types for the columns on a given row, thus the cursor must be pointing to a valid row. See SQLite docs on API lifecycle, rows and columns methods and this forum postThe Go API does not restrain the behavior of
RowsColumnTypeScanType()as to when it should be called. Thus, the implementation in this PR makes a compromise and falls back tosqlite3_column_decltype()in a best effort to remain retro-compatible, but this is still a potentially breaking change.The advantage of using
sqlite3_column_type()oversqlite3_column_decltype()is two-fold:sqlite3_column_decltype()is insufficient. Regarding this aspect, the existing implementation provides no way to access the values without potentially triggering a type conversion, unless the application is aware of the result type of the query beforehand.