New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make `BenchmarkTimestampable` care more about fractional-second parts. #406

Merged
merged 1 commit into from Mar 20, 2018

Conversation

2 participants
@MrMage
Copy link
Contributor

MrMage commented Mar 13, 2018

@MrMage MrMage force-pushed the MrMage:test-date-microseconds branch from 265682b to c295eae Mar 13, 2018

@tanner0101 tanner0101 changed the base branch from master to nio Mar 13, 2018

@tanner0101 tanner0101 added this to the 3.0.0-rc.2 milestone Mar 13, 2018

@tanner0101 tanner0101 self-assigned this Mar 13, 2018

@tanner0101

This comment has been minimized.

Copy link
Member

tanner0101 commented Mar 13, 2018

@MrMage can you take a look at the merge conflict?

@MrMage MrMage force-pushed the MrMage:test-date-microseconds branch from c295eae to 35d0fc0 Mar 13, 2018

@MrMage

This comment has been minimized.

Copy link
Contributor Author

MrMage commented Mar 13, 2018

The failure on Postgres is expected. It seems like we also need to fix the MySQL driver.

@MrMage

This comment has been minimized.

Copy link
Contributor Author

MrMage commented Mar 13, 2018

Hopefully fixed MySQL: vapor/mysql#141.

@tanner0101

This comment has been minimized.

Copy link
Member

tanner0101 commented Mar 13, 2018

Looks like PostgreSQL is now passing as expected! However, MySQL doesn't appear to be fetching the microseconds:

/root/fluent-mysql/Packages/Fluent/Sources/FluentBenchmark/BenchmarkTimestampable.swift:40: error: FluentMySQLTests.testTimestampable : failed - fetched createdAt timestamp 1520974866.0 is more than 2ms different from expected value 1520974866.09293

vapor/mysql is properly serializing and parsing microseconds from what I can tell.

Edit: Looks like we need to use TIMESTAMP(fsp) when creating the schema to specify "fractional second precision"

The fsp value, if given, must be in the range 0 to 6. A value of 0 signifies that there is no fractional part. If omitted, the default precision is 0. (This differs from the standard SQL default of 6, for compatibility with previous MySQL versions.)

My guess is we can just change the default Foundation.Date column type to be TIMESTAMP(6)

@MrMage

This comment has been minimized.

Copy link
Contributor Author

MrMage commented Mar 14, 2018

You were correct. I submitted vapor/fluent-mysql#84.

@tanner0101 tanner0101 merged commit fea4ca0 into vapor:nio Mar 20, 2018

3 of 4 checks passed

ci/circleci: linux-mysql Your tests failed on CircleCI
Details
ci/circleci: linux Your tests passed on CircleCI!
Details
ci/circleci: linux-postgresql Your tests passed on CircleCI!
Details
ci/circleci: linux-sqlite Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment