Skip to content
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

[mssql] Incorrect encoding of datatime2 for LocalDateTime values to the ms #977

Closed
DavideD opened this issue Jun 9, 2021 · 1 comment · Fixed by #982
Closed

[mssql] Incorrect encoding of datatime2 for LocalDateTime values to the ms #977

DavideD opened this issue Jun 9, 2021 · 1 comment · Fixed by #982
Assignees
Milestone

Comments

@DavideD
Copy link
Contributor

DavideD commented Jun 9, 2021

With MS Sql Server.

Testcase to add to MSSQLPreparedQueryNotNullableDataTypeTest

  @Test
  public void testEncodeDateTimeWithMillis(TestContext ctx) {
    final LocalDateTime start = LocalDateTime.now().truncatedTo(ChronoUnit.MILLIS);
    testPreparedQueryEncodeGeneric(ctx, "not_nullable_datatype", "test_datetime2", start, row -> {
      ColumnChecker.checkColumn(0, "test_datetime2")
        .returns(Tuple::getValue, Row::getValue, start)
        .returns(Tuple::getLocalDateTime, Row::getLocalDateTime, start)
        .returns(Tuple::getLocalDate, Row::getLocalDate, start.toLocalDate())
        .returns(Tuple::getLocalTime, Row::getLocalTime, start.toLocalTime())
        .returns(LocalDateTime.class, start)
        .forRow(row);
    });
  }

Result:

junit.framework.AssertionFailedError: Expected that public abstract java.lang.Object io.vertx.sqlclient.Tuple.getValue(int) would not fail: 
Expected :2021-06-09T17:20:40.643
Actual   :2021-06-09T17:31:23
@DavideD DavideD added the bug label Jun 9, 2021
@DavideD DavideD changed the title Incorrect encoding of datatype2 for LocalDateTime values to the ms [mssql] Incorrect encoding of datatype2 for LocalDateTime values to the ms Jun 9, 2021
@tsegismont tsegismont self-assigned this Jun 10, 2021
@tsegismont tsegismont added this to the 4.1.1 milestone Jun 10, 2021
@tsegismont
Copy link
Contributor

@DavideD thanks for the report!

@tsegismont tsegismont changed the title [mssql] Incorrect encoding of datatype2 for LocalDateTime values to the ms [mssql] Incorrect encoding of datatime2 for LocalDateTime values to the ms Jun 11, 2021
tsegismont added a commit to tsegismont/vertx-sql-client that referenced this issue Jun 11, 2021
Fixes eclipse-vertx#977

LocalTime, LocalDateTime and OffsetDateTime have nanosecond precision.
Also, the maximum scale in MSSQL is 7 (hundreds of nanos).

So when encoding time-related values, we will always send a value with maximum scale.
The server will do round/truncate as necessary.

Signed-off-by: Thomas Segismont <[email protected]>
tsegismont added a commit to tsegismont/vertx-sql-client that referenced this issue Jun 11, 2021
Fixes eclipse-vertx#977

LocalTime, LocalDateTime and OffsetDateTime have nanosecond precision.
Also, the maximum scale in MSSQL is 7 (hundreds of nanos).

So when encoding time-related values, we will always send a value with maximum scale.
The server will do round/truncate as necessary.

Signed-off-by: Thomas Segismont <[email protected]>
tsegismont added a commit that referenced this issue Jun 11, 2021
Fixes #977

LocalTime, LocalDateTime and OffsetDateTime have nanosecond precision.
Also, the maximum scale in MSSQL is 7 (hundreds of nanos).

So when encoding time-related values, we will always send a value with maximum scale.
The server will do round/truncate as necessary.

Signed-off-by: Thomas Segismont <[email protected]>
Rattenkrieg pushed a commit to Rattenkrieg/vertx-sql-client that referenced this issue Nov 8, 2021
Fixes eclipse-vertx#977

LocalTime, LocalDateTime and OffsetDateTime have nanosecond precision.
Also, the maximum scale in MSSQL is 7 (hundreds of nanos).

So when encoding time-related values, we will always send a value with maximum scale.
The server will do round/truncate as necessary.

Signed-off-by: Thomas Segismont <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants