Add flag to restore v1.2 time rounding behavior

[josephbuchma]

In v1.2 time is rounded DOWN, in v1.3 time is rounded UP. Can we introduce a flag for backward compatibility?

[methane]

I'm very conservative about adding options. More options makes people don't read manual,
hard to maintain.

In this case,

• How many user needs this? 1% of users? 5% or 10%?
• How difficult to resolve it in application layer?

[methane]

As my understanding, you can fix it by using now.Truncate(time.Second).
Until you fix your code, you can use old version.

What is rational for adding new option in new version?

[josephbuchma]

@methane When I insert time.Now(), I naturally assume that time I just inserted is in the past in subsequent lines of code. Rounding time up is rather exception, and I don't see why it should be a default behavior.

[methane]

That's default behavior of MySQL, not this driver.
I think MySQL driver shouldn't affect MySQL's behavior as possible.
So I can't agree that's enough reason to add such option.

[josephbuchma]

More options makes people don't read manual

I'd say it already has more than enough options to discourage people from reading manual.
Or at least the way they're presented is not super welcoming. Hiding/compacting some stuff might help,

For example using spoilers:

allowAllFiles=true disables the file Whitelist for LOAD DATA LOCAL INFILE and allows all files.

Might be insecure!

Type:           bool
Valid Values:   true, false
Default:        false

[methane]

It doesn't change my mind. Adding option makes hard to maintain for long time in long-time run.
Adding option make sense only when:

• It is required by hundreds of people.
• Solving problem in application layer is not realistic.

In this case, the problem looks solved in application layer.
Rounding issue is just a MySQL behavior. And this project is MySQL driver, not utility library.
I don't think option to hide some MySQL behavior is important enough to add option.

[methane]

See also: rails/rails#18067

Rails (ActiveRecord = ORM, not driver) cares this backward incompatible change between 5.6 and 5.5.
Of course, Rails is higher layer than MySQL driver. It has information about column type.
And Ruby's mysql2 (driver) doesn't have such option.

I think it's good example. Driver should be transparent. Driver should pass complete information
as possible. Higher layer library or application should care about SQL and DB's behavior.

[methane]

And see also too: https://bugs.mysql.com/bug.php?id=68760

[josephbuchma]

ok, I see your point @methane
So far I truncated time everywhere in app code, but possible driver-level workaround I see here is to write wrapper for mysql driver, e.g:

package mymysql

import "time"
import "database/sql"
import "database/sql/driver"
import "github.com/go-sql-driver/mysql"

type stmt struct {
	driver.Stmt
}

func (s *stmt) Exec(args []driver.Value) (driver.Result, error) {
	for i, v := range args {
		switch v := v.(type) {
		case time.Time:
			args[i] = v.Truncate(time.Second)
		case *time.Time:
			*v = v.Truncate(time.Second)
		}
	}
	return s.Stmt.Exec(args)
}

type conn struct {
	driver.Conn
}

func (c *conn) Prepare(query string) (driver.Stmt, error) {
	s, err := c.Conn.Prepare(query)
	if err != nil {
		return nil, err
	}
	return &stmt{s}, nil
}

type myMySQL struct {
	my mysql.MySQLDriver
}

func (d myMySQL) Open(dsn string) (driver.Conn, error) {
	c, err := d.my.Open(dsn)
	if err != nil {
		return nil, err
	}
	return &conn{c}, nil
}

func init() {
	sql.Register("mymysql", myMySQL{})
}