Microseconds

[arnehormann]

first attempt to add full support for microsecond resolution, comes with a major improvement of the tests for TIME, DATE and DATETIME.

At the very least, this can be built upon.

[arnehormann]

Travis fails because Go 1.1 does not support positional parameters in fmt.Sprintf yet and the MySQL version is to old to recognize DATETIME/DATE/TIME with fractional seconds syntactically.
I'll adapt the tests so we skip the syntax-problems where applicable and get rid of the positional parameter.

[arnehormann]

phew @julienschmidt PTAL

All others: please have a look at it, too.
First off, I'm sorry for the noise, I couldn't debug this locally.
Second, we are now able to specify the microsecond resolution to the digit.
Yay!

[julienschmidt]

Do we need this field? I can only see it being set, not read somewhere.

[arnehormann]

For a time when I deeply mistrusted the decimals field, I did use it.
I think we should still keep it for two reasons:

• it provides a hint useable for the maximal allocation size in all types
• with it, mysqlField is perfectly aligned on 64bit boundaries

[julienschmidt]

We don't use it, we don't need it 😄 #YAGNI

Regarding alignment: The compiler takes care of it if matters on the target architecture. On a 32bit architecture this is just a waste of space.

[arnehormann]

PTAL

[julienschmidt]

Sorry, I don't have forgotten you, but I currently don't have the time to review this properly (exams phase; double credit load).
These changes are quite complex and should be carefully reviewed IMO.
Please excuse me for needing some more time.

I invite anyone else to review the changes and write their opinion 😉

[nightlyone]

A default case is missing to handle decimals you don't know about (e.g. from garbled protocol data).
If dstlen == 0 is the correct answer already, please make it explicit.

[arnehormann]

PTAL - and thanks for the review, @nightlyone

[nightlyone]

LGTM, but please wait for @julienschmidt

[arnehormann]

As by our policy, I will.
Each PR has to be LGTMed by a team member - and I won't/can't LGTM my own code.

Again, thanks for the review!

[julienschmidt]

Many thanks from me also!
I'll try to do a review ASAP, hopefully this evening or tomorrow.

[julienschmidt]

Why not leave this here instead moving it to utils.go?

[arnehormann]

Because I thought it's only used in utils.go, but that's wrong.
It's also used in packets.go.
I'll change that.

[julienschmidt]

Did you test this with older MySQL versions? I'm still not convinced that this doesn't break the compatibility, especially the tests 😉

[arnehormann]

I did on 5.0.35 - and the Travis version is pretty old, too.

rows, err = dbt.db.Query(`SELECT cast("00:00:00.1" as TIME(1)) = "00:00:00.1"`)
if err == nil {
    rows.Scan(&withFrac)
    rows.Close()
}

is the part in the tests allowing for backward compatibility - it skips tests the server doesn't support.

[julienschmidt]

Lets trade: One LGTM for one successful MySQL 4.1 test 😄

[arnehormann]

4.1.12 on a Win XP VM successfully tested.
But I cheated (a little).
PTAL - still LGTM (pretty please)?

[julienschmidt]

fine 😒

[arnehormann]

I checked and confirmed that a timestamp can be stored in a table and retrieved from it, I also started on a test. Will probably commit and merge it tomorrow. Let's turn that frown upside down..