Inconsistent parsing of DATETIME type, causing errors

[scosman]

I use DATETIME type, with sub second precision in my schema. A scan against this table sometimes errors with this error:
Scan error on column index 0, name "created_at": converting driver.Value type time.Time ("2024-03-05 20:36:02 +0000 UTC") to a float64: invalid syntax

This only happens if a created_at value in the table is perfectly round (no decimal place). Then it seems to try to scan to time.Time, while the values with fractions try to scan to float64. It's a rare repo since it doesn't happen unless the timestamp exactly rounds to an even second (1/1000 chance)

Interestingly this error occurs if any value in the table has a perfectly round value, not just the row being returned by "LIMIT 1". I would have though only the returned value makes it up to the golang parsing layer.

IMO it should alway return time.Time. I used float64 when I saw the scan return floats. However, inconsistent and rare is a problem.

Query:

• Fails if any created_at value in the table has happens to be an integer: SELECT created_at FROM property_history WHERE name = ? ORDER BY created_at DESC LIMIT 1 when I call Scan(&float64Val)
• Always works (force real with *1.0): SELECT created_at*1.0 FROM property_history WHERE name = ? ORDER BY created_at DESC LIMIT 1

Create a table like so.

		CREATE TABLE IF NOT EXISTS property_history (
			id INTEGER PRIMARY KEY,
			name TEXT NOT NULL,
			text_value TEXT,
			created_at DATETIME,
			updated_at DATETIME
		);

		CREATE INDEX IF NOT EXISTS property_history_name_created_at ON property_history (name, created_at);

		CREATE TRIGGER IF NOT EXISTS insert_property_history_created_at
		AFTER INSERT ON property_history
		BEGIN
			UPDATE property_history SET created_at =unixepoch('subsec') WHERE id = NEW.id;
		END;

Then run this test case (db.sqldb is a sql.DB created with this driver):

func TestTimestampRoundingAndLatestPropHistory(t *testing.T) {
	db := testBuildTestDb(t)
	defer db.Close()

	// insert a row into table
	_, err := db.sqldb.Exec(`
		INSERT INTO property_history (name, type, text_value, sample_type)
		VALUES ('test', ?, 'val', 1)
	`, DBPropertyTypeString)
	if err != nil {
		t.Fatal(err)
	}

        // Running `SELECT created_at FROM property_history WHERE name = ? ORDER BY created_at DESC LIMIT 1`
	ct, err := db.latestPropertyHistoryTime("test")
	if err != nil {
                // this fails about 1 in 1000 times, but usually passes. We force a 100% failure below.
		t.Fatal(err)
	}
	if ct == nil {
		t.Fatal("LatestPropHistoryTime returned nil")
	}
	if math.Abs(time.Since(*ct).Seconds()) > 0.01 {
		t.Fatal("LatestPropHistoryTime returned wrong time")
	}

	// update the row to exact time, no milliseconds
	// This previously caused a scan error (even when the row returned still was fractional)
	_, err = db.sqldb.Exec(`
		UPDATE property_history SET created_at = 1710791550
		WHERE name = 'test'
	`)
	if err != nil {
		t.Fatal(err)
	}

        // Running `SELECT created_at FROM property_history WHERE name = ? ORDER BY created_at DESC LIMIT 1`
	ct, err = db.latestPropertyHistoryTime("test")
	if err != nil {
		t.Fatal(err) // **fails here with error above**
	}
	if ct == nil {
		t.Fatal("LatestPropHistoryTime returned nil")
	}
	if math.Abs(float64(ct.Unix())-1710791550.0) > 0.1 {
		t.Fatal("LatestPropHistoryTime returned wrong time")
	}
}

The function call used in test above:

const latestPropHistoryTimeByNameQuery = `SELECT created_at FROM property_history WHERE name = ? ORDER BY created_at DESC LIMIT 1`

func (db *DB) latestPropertyHistoryTime(name string) (*time.Time, error) {
	var epochTime float64
	err := db.sqldb.
		QueryRow(latestPropHistoryTimeByNameQuery, name).
		Scan(&epochTime)
	if err == sql.ErrNoRows {
		return nil, nil
	}
	if err != nil {
		return nil, err
	}

	_, fractionalSeconds := math.Modf(epochTime)
	nanoseconds := int64(fractionalSeconds * 1_000_000_000)
	time := time.Unix(int64(epochTime), nanoseconds)
	return &time, nil
}

[mattn]

There is no datetime type in sqlite3. Everything is treated as text on sqlite3 for unknown types.
go-sqlite3 implements a special treatment that allows the text to be treated as a string or time.
If you use time.Time or string instead of float32, you will not have this problem. If you want to treat created_at as int64 or float64, you should declare the type of created_at as integer or real.

[mattn]

See https://www.sqlite.org/datatype3.html

[scosman]

Ah. I knew sqlite was mapping SQL-spec types to a smaller subset. I didn't realize DATETIME -> NUMERIC, and numeric prefers int to real. Looks like in this case, values close to round numbers went to ints.

Note: Scanning DATETIME into time.Time doesn't work. It would be ideal if it did. I only went the epochTime/float64 route when I saw the query returning float64. When a DATETIME (aka NUMERIC) column has a real (non int) value it errors with sql: Scan error on column index 0, name "created_at": unsupported Scan, storing driver.Value type float64 into type *time.Time

Can the time.Time be updated to accept both int and real values?

[scosman]

Seems like a case could be added in convert.go for src=float64, dest=time.Time?

[mattn]

You can make new type which can convert integer/real to time.Time.

func (v *MyDateTime) Scan(value interface{}) error {
    // ...
}

func (v *MyDateTime) Value() (driver.Value, error) {
    // ...
}

[mattn]

BTW, why you don't define epochTime as time.Time ?

[scosman]

why you don't define epochTime as time.Time ?

That's what I'd like to do, but it hits Scan error on column index 0, name "created_at": unsupported Scan, storing driver.Value type float64 into type *time.Time if any values are floating point.

I def could go the custom type route.

Since the driver support epoch integers, and the SQL DATETIME type maps to both ints and reals (and strings) in sqlite, it would be nice if reals were also supported by the driver as time.Time, like ints are. Correct me if I'm wrong, but I think this is all valid SQL, and only errors on this driver?

Let me know if you'd be open to a patch adding time.Time to float64 conversion in convert.go. I could see not wanting to break back compatibility. There's also the question of information loss: I don't think it will happen in practice often, but someone could add higher than nanosecond precision to a floating point field, in which case time.Time would be losing data.