remove sql_type_code remove usage #3651
Conversation
| default: | ||
| throw new SQLException("JDBC Type: " + type + " not supported"); | ||
| } | ||
| } |
There was a problem hiding this comment.
The test gap analysis is claiming this method is uncovered. Unless this is a mistake of the code coverage reporting tool, that would seem to suggest we're not really looking at this part of the code base that changed. Do you know what the issue is?
There was a problem hiding this comment.
I believe at least this method is covered by a test over an external JDBC server and not the in-process one.
| public static Parameter ofBoolean(boolean b) { | ||
| return Parameter.newBuilder() | ||
| .setType(Type.BOOLEAN) | ||
| .setJavaSqlTypesCode(Types.BOOLEAN) |
There was a problem hiding this comment.
It would seem to me that for cross-version compatibility reasons, we'd need to keep setting this field at least for one minor version. Otherwise, I'm not sure how old code would make sense of the Parameter's type. Is my understanding wrong, or are we not covering this in mixed mode tests? Or something else?
There was a problem hiding this comment.
Yeah, I think setting old fields need to be in-place for another release. The scope of this PR should only be limited to start using new fields in place of old fields.
| // Add the metadata and get offset at where to insert data. | ||
| int offset = addMetadata(ColumnMetadata.newBuilder() | ||
| .setName(fieldName).setJavaSqlTypesCode(Types.BOOLEAN).setType(Type.BOOLEAN).build()); | ||
| .setName(fieldName).setType(Type.BOOLEAN).build()); |
There was a problem hiding this comment.
Does this need to retain the type codes for an additional minor version release? That is, do we need to keep setting the deprecated field until we're dropped cross version support for anything that would expect it to be there
g31pranjal
changed the title
g31pranjal
force-pushed
the
remove_sql_types_from_grpc
branch
from
3a9a283 to
5e3af6a
Compare
2 participants
This PR is a follow-up PR in the series for deprecating using sql_type_codes in JDBC, and instead using "richer" Relational types. The support for relational types were introduced some time bakc in #3510. This PR shifts to relational type usage, away from sql_type usage, to make way for retiring the latter field completely from the protobuf. After this, we could remove the field in 2 steps: