Skip to content

correct processing of OffsetDateTime in SQLServerDataTable #2651

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

correct processing of OffsetDateTime in SQLServerDataTable #2651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e8a7ba
correct processing of OffsetDateTime in SQLServerDataTable
LeosBitto Apr 26, 2025
1532667
add a column to SQLServerDataTableTest with Types.TIMESTAMP_WITH_TIME…
LeosBitto Apr 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.text.MessageFormat;
import java.time.OffsetDateTime;
import java.time.OffsetTime;
import java.time.format.DateTimeFormatter;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -275,8 +276,12 @@ private void internalAddrow(JDBCType jdbcType, Object val, Object[] rowValues,

// java.sql.Date, java.sql.Time and java.sql.Timestamp are subclass of java.util.Date
if (val instanceof java.util.Date || val instanceof microsoft.sql.DateTimeOffset
|| val instanceof OffsetDateTime || val instanceof OffsetTime)
|| val instanceof OffsetTime)
rowValues[key] = val.toString();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a unit test for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to ammend an existing unit test with OffsetDateTime and SQLServerDataTable, but I was not able to find any. Could you please point me?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you consider adding one in SQLServerDataTableTest.java ?

Copy link
Contributor Author

@LeosBitto LeosBitto Apr 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing, I will add a column of type TIMESTAMP_WITH_TIMEZONE there and fill it with OffsetDateTime - that's exactly what I do in my application where I have found this bug (and have solved it with this change to the JDBC driver).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@machavan is the unit test now OK, or is anything else needed before merge?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LeosBitto it looks fine. We will run our ADO tests on this PR and then move ahead for merging it.

else if (val instanceof OffsetDateTime)
// avoid calling OffsetDateTime#toString() because when there are no seconds we would get
// a format which is incompatible with the server - see LocalTime#toString()
rowValues[key] = ((OffsetDateTime)val).format(DateTimeFormatter.ISO_OFFSET_DATE_TIME);
else
rowValues[key] = val;
break;
Expand Down
26 changes: 17 additions & 9 deletions src/test/java/com/microsoft/sqlserver/jdbc/tvp/SQLServerDataTableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@

import java.math.BigDecimal;
import java.sql.Types;
import java.time.OffsetDateTime;
import java.time.format.DateTimeFormatter;

import org.junit.jupiter.api.Test;
import org.junit.platform.runner.JUnitPlatform;
import org.junit.runner.RunWith;
Expand All @@ -26,17 +29,20 @@ public void testClear() throws SQLServerException {
SQLServerDataTable table = new SQLServerDataTable();
SQLServerDataColumn a = new SQLServerDataColumn("foo", Types.VARCHAR);
SQLServerDataColumn b = new SQLServerDataColumn("bar", Types.INTEGER);
SQLServerDataColumn c = new SQLServerDataColumn("baz", Types.TIMESTAMP_WITH_TIMEZONE);

table.addColumnMetadata(a);
table.addColumnMetadata(b);
assertEquals(2, table.getColumnMetadata().size());
table.addColumnMetadata(c);
assertEquals(3, table.getColumnMetadata().size());

table.clear();
assertEquals(0, table.getColumnMetadata().size());

table.addColumnMetadata(a);
table.addColumnMetadata(b);
assertEquals(2, table.getColumnMetadata().size());
table.addColumnMetadata(c);
assertEquals(3, table.getColumnMetadata().size());
}

@Test
Expand Down Expand Up @@ -66,13 +72,14 @@ public void testHashCodes() throws SQLServerException {
assert (a.equals(aClone));

SQLServerDataColumn b = new SQLServerDataColumn("bar", Types.DECIMAL);
SQLServerDataTable table = createTable(a, b);
SQLServerDataColumn c = new SQLServerDataColumn("baz", Types.TIMESTAMP_WITH_TIMEZONE);
SQLServerDataTable table = createTable(a, b, c);

// Test consistent generation of hashCode
assert (table.hashCode() == table.hashCode());
assert (table.equals(table));

SQLServerDataTable tableClone = createTable(aClone, b);
SQLServerDataTable tableClone = createTable(aClone, b, c);

// Test for different instances generating same hashCode for same data
assert (table.hashCode() == tableClone.hashCode());
Expand All @@ -82,21 +89,22 @@ public void testHashCodes() throws SQLServerException {
assert (a.hashCode() != b.hashCode());
assert (!a.equals(b));

SQLServerDataColumn c = new SQLServerDataColumn("bar", Types.FLOAT);
SQLServerDataColumn bb = new SQLServerDataColumn("bar", Types.FLOAT);
table.clear();
table = createTable(a, c);
table = createTable(a, bb, c);

// Test for non equal hashCodes
assert (table.hashCode() != tableClone.hashCode());
assert (!table.equals(tableClone));
}

private SQLServerDataTable createTable(SQLServerDataColumn a, SQLServerDataColumn b) throws SQLServerException {
private SQLServerDataTable createTable(SQLServerDataColumn a, SQLServerDataColumn b, SQLServerDataColumn c) throws SQLServerException {
SQLServerDataTable table = new SQLServerDataTable();
table.addColumnMetadata(a);
table.addColumnMetadata(b);
table.addRow("Hello", new BigDecimal(1.5));
table.addRow("World", new BigDecimal(5.5));
table.addColumnMetadata(c);
table.addRow("Hello", new BigDecimal(1.5), OffsetDateTime.parse("1977-07-24T12:34+02:00", DateTimeFormatter.ISO_OFFSET_DATE_TIME));
table.addRow("World", new BigDecimal(5.5), OffsetDateTime.parse("1977-07-24T12:34:56+02:00", DateTimeFormatter.ISO_OFFSET_DATE_TIME));
table.setTvpName("TVP_HashCode");
return table;
}
Expand Down