Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Fix to ensure metadata returned follows JDBC data type specs #2326
Fix to ensure metadata returned follows JDBC data type specs #2326
Changes from 4 commits
dfdbf76
c95f6f3
552cccb
0b95bc2
71e657d
cc6c9d7
dbc5b28
19bacf2
d2d1b05
0ce310e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was mentioned in a previous comment above, but ensure that
expectedClass
is not null, don't assert.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this really shouldn't even be necessary, we just defined this above, this would never be null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did we accidentally remove the check for isAssignableFrom that was in previous commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expected class can be null if the the column label/name in the expected class lookup table does not match the actual column label from the result set (i.e. the lookup by column name will fail). This shouldn't happen but it is possible if there was a future code change or perhaps the returned metadata columns change.
Yes the IsAssignableFrom() is no longer used as I changed the BUFFER_LENGTH metadata class from Object to Integer. It was because of this generic Object class that we needed to use the IsAssignableFrom() check. Is The reason why it was originally specified as a generic Object was that the JDBC spec does not use the BUFFER_LENGTH column and so I reflected that as a generic Object being the expected data type. But in the code I have always ensured that BUFFER_LENGTH was an Integer. Therefore to simplify the logic and remove the IsAssignableFrom() check, I switched the expected class for the BUFFER_LENGTH column from Object to Integer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have kept the check but no longer use an assert to perform the check. I found that in the unlikely event there was an issue, a more detail error message was required that would include not just the actual and expected types but the column as well. Without the column name/label, it would initially be difficult t determine what the problem column was in the metadata