use sqlite3_column_type() for ColumnTypeScanType()

[alkemir]

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 calling Next(). 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 post

The 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 to sqlite3_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() over sqlite3_column_decltype() is two-fold:

1. It is closer to the SQLite3 API
2. It will return the correct scanType for expressions and aggregate functions, which are the cases in which 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.

[rittneje]

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?

[alkemir]

*any is the expected type, see the Rows.Scan() docs:

// 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.

Relevant code:

case nil:
    switch d := dest.(type) {
    case *any:
	if d == nil {
		return errNilPtr
	}
	*d = nil
	return nil

In our case, this works:

	for rr.Next() {
		cc, err := rr.ColumnTypes()
		if err != nil {
			log.Fatal(err)
		}

		pointers := make([]any, 0)
		for _, ct := range cc {
			fmt.Printf("%s ", ct.ScanType())
			pointers = append(pointers, reflect.New(ct.ScanType()).Interface())
		}
		fmt.Println()

		if err := rr.Scan(pointers...); err != nil {
			log.Fatal(err)
		}

		fmt.Println(pointers...)
		for _, p := range pointers {
			if pn, ok := p.(**any); ok {
				fmt.Printf("%t ", *pn == nil)
			}
		}

	}

sql.NullInt64 sql.NullString sql.NullFloat64
&{1 true} &{hello true} &{3.1415 true}
sql.NullInt64 *interface {} *interface {}
&{2 true} 0xc000064090 0xc000064098
true true

[rittneje]

That's unrelated. Rows.Scan is talking about its arguments, which by necessity all have to be pointers as they are essentially output parameter. Meanwhile, ColumnTypeScanType is talking about the type to scan into (so not a pointer).

For example, ColumnTypeScanType mentions returning reflect.TypeOf(int64(0)) while Rows.Scan mentions *int64.

You'll also notice that all the other types being returned (e.g., TYPE_NULLSTRING) here are not pointers.

In other words, you are supposed to create a variable of the type from ColumnTypeScanType, and then pass a pointer to it to Rows.Scan.

[alkemir]

You are right. I will return interface{} now, following Go 1.21 , although this has been changed for Go 1.24 for compatibility.

Still, this might break code depending on the previous behavior.

[rittneje]

Please add a unit test for the new behavior.

[alkemir]

Should we still return the nullable types? When SQLite returns a NULL value, the column type will be type_any, so there is no need for nullable types now.

[alkemir]

Thank you for all the reviews, is there anything else that I can do to improve this PR?

[mattn]

Please add unit tests

[alkemir]

Unit tests are already included, is the PR missing any particular aspect?

[mattn]

Thank you for this PR. Using sqlite3_column_type() is the right direction — the current sqlite3_column_decltype() approach cannot handle expressions and aggregate functions. However, I have some concerns I'd like to discuss before merging:

• Return type change from nullable to non-nullable: sql.NullInt64 → int64, sql.NullString → string, etc. Some downstream code that uses reflect.New(ct.ScanType()) and type-asserts on the result may break.
• Exported constants removed: SQLITE_INTEGER, SQLITE_TEXT, SQLITE_BLOB, SQLITE_REAL, SQLITE_NUMERIC, SQLITE_TIME, SQLITE_BOOL, SQLITE_NULL are deleted. Any downstream code referencing these will fail to compile.
• Behavior now depends on call timing: Before Next() it returns interface{}, after Next() it returns the actual type. The old implementation always returned based on the declared type regardless of cursor position.
• Loss of NUMERIC/DECIMAL mapping: The old implementation mapped NUMERIC/DECIMAL declared types to float64. Now sqlite3_column_type() decides, so a NUMERIC column holding an integer value may return int64 instead.
• Loss of CLOB/CHAR(N) mapping: These declared types were previously recognized as TEXT but are now entirely up to sqlite3_column_type().

Also, could you add tests for BOOLEAN type (to verify type_bool is returned), expressions (e.g. price * 2), and NUMERIC/DECIMAL declared types?

Some of these may be acceptable trade-offs, but I'd like to hear your thoughts.