[CALCITE-3163] AbstractCursor#convertValue() now adheres to JDBC specification #105
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vlsi
force-pushed
the
master
branch
3 times, most recently
from
d90fb8c to
92045d0
Compare
nouda
added a commit
to polypheny/Avatica
that referenced
this pull request
Mar 30, 2020
Merge [[CALCITE-3163]](apache#105) and [[CALCITE-3162]](apache#106)
zabetak
reviewed
Feb 19, 2021
Comment on lines
-1321
to
+1323
| case Types.FLOAT: | ||
| case Types.REAL: | ||
| return componentAccessor.getFloat(); | ||
| case Types.FLOAT: |
There was a problem hiding this comment.
The fix looks reasonable but I was thinking that it would be nice to have some tests to guard against future regressions. Maybe in ArrayTypeTest?
There was a problem hiding this comment.
I agree, unfortunately ArrayTypeTest being based on hsqldb, won't work as-is, because hsqldb maps Double, Real and Float to Java double (see here).
I tried and indeed a CREATE TABLE $table (id integer, vals REAL ARRAY) statement creates a table (INTEGER, DOUBLE ARRAY).
Either we find an "easy" way to use another DB in those tests, or we need to find another location, I will have a look during the weekend.
There was a problem hiding this comment.
I have managed to add unit tests, I have opened a complementing PR: #139
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Outlined in JIRA issue [CALCITE-3163]