Error with passing time.Time as a field for saving

[AndrewTheJavaGuy]

I am trying this as for writing DB unit tests. Pretty much seeing if I can get the the relvant tables so we can write some tests against the DB code that tries to interact with them. So mostly trying to work with code and SQL that we know works against a MySQL instance and see if it can do the same things with this instance (so can't really change around the syntax or calling too much).

Having an issue where it's returning: Error 1105 (HY000): incompatible conversion to SQL type: TIMESTAMP

From the time.Now() instance passed as part of a Prepared Statement.

The field itself is being used with: CREATE TABLE IF NOT EXIXSTS ( ... TIMESTAMP NOT NULL ...) (which I thought should be safe enough)

A bit of digging around in your codebase shows it's coming from /sql/datetime.go->ConvertWithoutRangeCheck

It looks like it thinks the time instance being passed is a uint8[] instead of the actual time.Time or any of the other int types. Dumping it as-is (with Fmt.Println), shows the slice contains: [50 48 50 50 45 48 55 45 50 57 32 48 52 58 53 48 58 52 55 46 49 57 49 51 54 54] (which probably corresponds to something in my local GMT+10 instance)

This looks very similar to this issue: https://4rum.dev/t/unsupported-scan-storing-driver-value-type-uint8-into-type-time-time/103/2 That others have reported against MySQL itself.

Having a quick scan through the repo, nothing is standing out for me as a similar flag / config value that could be set to replicate this.

Unsure what the actual fix happened for this, but will add more if I get closer to figuring out ways around this.

[AndrewTheJavaGuy]

Ok, this is very odd. The error I was seeing was actually triggered from the information_schema when it's calling: y2k, _ := Timestamp.Convert("2000-01-01 00:00:00")

I was wrong when I thought it was my created table. With some existing code written, it will try to query the information_schema table before running actual queries with our system. I had just assumed it had done this before it got to working with the records.

It seems to pass it fine through as a string, until it gets to the ConvertWithoutRangeCheck mehtod, at which point it thinks it's a []uint8.

But it's still a string at the end of it all, so adding this to the switch (in that function):

case []uint8:
		if len(value) == 0 {
			return zeroTime, nil
		}

		return t.ConvertWithoutRangeCheck(string(value))

```		
Gets it working correctly. Even though there's nothing that should be saying it's anywhere differently along the calling chain.

So don't quite know if this is an actual bug or something from my setup (go version go1.18.2 darwin/amd64 / Mac M1) causing some strangeness in Go when a value is passed through as an interface{} multiple times through functions.
		

[VinaiRachakonda]

Great find @AndrewTheJavaGuy. We'll investigate why this improper type conversion on the information schema side and get back to you. Definitely a concerning bug

[VinaiRachakonda]

We are happy to take contributions as well if you end up finding the cause.

[bpf120]

Hi @AndrewTheJavaGuy , feel free to swing by our Discord or shoot me an email if you to discuss what you're working on more with the team.

[ryanpbrewster]

I bisected this down to #1061 using this test:

if _, err := db.Exec("CREATE TABLE mytable (t TIMESTAMP)"); err != nil {
  panic(err)
}
if _, err := db.Exec("INSERT INTO mytable (t) VALUES (?)", time.Now()); err != nil {P
  panic(err)
}

This test passes at a13a7c491 and fails at 31118129be3

[ryanpbrewster]

Dug into this a bit, I'm pretty sure that this was an unintended behavioral change. #1061 fixed the conversion of VARBINARY to properly generate []byte instead of string, but failed to update the datetypetype.go logic to be able to parse []byte. As of today, that logic still expects a string.

I believe the fix here is as simple as "convert []byte to string before trying to parse it as a timestamp". I'll send out a PR shortly, along with a test.

[fulghum]

Great find @ryanpbrewster! Thank you for digging in on this one and offering to send a PR – very appreciated!

[fulghum]

Just merged in Ryan's fix for this issue. Major thanks to @ryanpbrewster for contributing this fix. 🎉

I'm resolving this issue now that Ryan's fix is merged into main; please don't hesitate to reopen this one or cut us a new issue if you're still seeing any incorrect behavior.