MySQL data types need to be refactored

[alexanderfefelov]

• dl, ul, ping, jitter are numbers, not a text,
• text and longtext are too big data types,
• nullability requires more attention.

Environment

• speedtest-go 1.1.1

Links

• https://github.com/librespeed/speedtest-go/blob/master/database/mysql/telemetry_mysql.sql
• https://dev.mysql.com/doc/refman/8.0/en/storage-requirements.html#data-types-storage-reqs-strings

[maddie]

Sorry for the late reply, I've been held up by work lately.

I agree that the data types aren't quite accurate, but I'm not really sure that I'd want to implement this change now, because:

1. It would break current users if the Go code changes (types aren't compatible)
2. It's working fine for now (well...)
3. I can't really think of the difference between using text and numbers if we're just "showing them in the stats page", and if one wants to use the numbers for statistical use, there are still a lot of ways to write simple code to parse the string into numbers

I'll leave it open for now, but implementing this change is very unlikely in the near future. Maybe in v2?

[alexanderfefelov]

Ping and co generates numerical data by nature. Using numerical data allows the LibreSpeed database to be treated as a time series database without any additional coding.

[maddie]

Yes, I understand, but in the code:

speedtest-go/database/mysql/mysql.go

Lines 30 to 46 in c815103

	func (p *MySQL) Insert(data *schema.TelemetryData) error {
	stmt := `INSERT INTO speedtest_users (ip, ispinfo, extra, ua, lang, dl, ul, ping, jitter, log, uuid) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?);`
	_, err := p.db.Exec(stmt, data.IPAddress, data.ISPInfo, data.Extra, data.UserAgent, data.Language, data.Download, data.Upload, data.Ping, data.Jitter, data.Log, data.UUID)
	return err
	}
	func (p *MySQL) FetchByUUID(uuid string) (*schema.TelemetryData, error) {
	var record schema.TelemetryData
	row := p.db.QueryRow(`SELECT * FROM speedtest_users WHERE uuid = ?`, uuid)
	if row != nil {
	var id string
	if err := row.Scan(&id, &record.Timestamp, &record.IPAddress, &record.ISPInfo, &record.Extra, &record.UserAgent, &record.Language, &record.Download, &record.Upload, &record.Ping, &record.Jitter, &record.Log, &record.UUID); err != nil {
	return nil, err
	}
	}
	return &record, nil
	}

... fields in schema.TelemetryData are all strings. If this change is to be implemented, users with old schemas will not be able to insert/load records, unless they recreate the database. This would qualify as a breaking change.

[Enrico204]

This might be fixed with a migration: the code can detect this change at runtime and ask the user to migrate (or automatically migrate)