SQL-804: Make MongoSQLResultSet.getObject().toString() consistent with getString()

[mattChiaravalloti]

This PR updates MongoSQLResultSet.getObject() to return a value such that calling toString() on it is consistent with calling MongoSQLResultSet.getString(). Both versions now produce what getString() was already returning. As discussed in standup, this was accomplished by wrapping the result of getObject() for Types.OTHER types in a new wrapper class that overrides toString() as needed.

~Note: we separately discussed having all Types.OTHER types stringify to extended JSON, but that change is outside the scope of this ticket. We technically haven't even decided if that is the direction we want to go as that is still up for debate in the design doc. Therefore, this ticket does not introduce any new behavior to the stringification -- it exactly matches what getString() was already doing.~

After the design discussion and seeing this ticket is included in the epic, I will update stringification to use extended json for the OTHER types as part of this ticket.

[nbagnard]

(Putting in the right PR) ah, bummer, I looked at the spec again because I was surprised that all integer related types are mapped to getInt and I think that there are a couple of types for which getObject().toString will not match getString if we want to match the JDBC spec. See B.3 JDBC Types Mapped to Java Object Types from the spec:

DECIMAL/NUMERIC => java.math.BigDecimal
BINARY/VARBINARY/LONGVARBINARY => byte[]
TIMESTAMP => java.sql.Timestamp

I would still keep the getString() value to be the extended JSON string though because I did not find anything regarding what getString is supposed to return. The spec only says that it should return a value for all mainstream SQL/JDBC types (we were not obligated to return a String for OTHER for example according to the spec, but it looks like they just missed it, same for Null).

It seems safer to follow the spec for getObject, just in case someone's application uses getObject unconditionally and then work directly on the objects with the assumption it match the spec and that getObject for a TIMESTAMP return a java.sql.timestamp for example. WDYT?

[mattChiaravalloti]

(Putting in the right PR) ah, bummer, I looked at the spec again because I was surprised that all integer related types are mapped to getInt and I think that there are a couple of types for which getObject().toString will not match getString if we want to match the JDBC spec. See B.3 JDBC Types Mapped to Java Object Types from the spec:

DECIMAL/NUMERIC => java.math.BigDecimal
BINARY/VARBINARY/LONGVARBINARY => byte[]
TIMESTAMP => java.sql.Timestamp

I would still keep the getString() value to be the extended JSON string though because I did not find anything regarding what getString is supposed to return. The spec only says that it should return a value for all mainstream SQL/JDBC types (we were not obligated to return a String for OTHER for example according to the spec, but it looks like they just missed it, same for Null).

It seems safer to follow the spec for getObject, just in case someone's application uses getObject unconditionally and then work directly on the objects with the assumption it match the spec and that getObject for a TIMESTAMP return a java.sql.timestamp for example. WDYT?

discussed offline -- I updated the getObject method to return the instances of the appropriate classes for the ones you listed above (although VARBINARY AND LONGVARBINARY are still left as invalid for now. I believe there is another ticket to update the JDBC type for BSON binary).

This is now implemented and up to date!

[mattChiaravalloti]

There is one unit test failing for the date column:

AssertionFailedError: expected: <2020-12-25 12:13:14.0> but was: <2020-12-25 17:13:14.0>

Oh that's odd. I'm not getting that locally. I wonder if it has to do with what timezone my local machine is in vs. what timezone the evergreen machine is in. I will update the test to expect the appropriate result based on where the test is run.

[mattChiaravalloti]

evergreen merge

[mattChiaravalloti]

evergreen merge

[mattChiaravalloti]

just realized the commit queue task went purple the other day (system failure). resubmitted.

[mattChiaravalloti]

@rychipman It looks like this cannot be merged until you approve since your last comment moved you to a "Requested Changes" state. (I attempted to remove you as a reviewer, but that moved you from the yellow circle to the red box). From evergreen:

405 1 review requesting changes and 2 approving reviews by reviewers with write access.

[mattChiaravalloti]

evergreen merge