[CALCITE-3468] JDBC adapter may generate casts on Oracle for varchar without the precision and for char with the precision exceeding max length of Oracle #1861
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
danny0405
reviewed
Mar 20, 2020
danny0405
reviewed
Mar 20, 2020
| if (type.getPrecision() == RelDataType.PRECISION_NOT_SPECIFIED) { | ||
| type = allowTypeWithUnspecifiedPrecision(type) | ||
| ? type | ||
| : new SqlTypeFactoryImpl(getTypeSystem()).createSqlType(type.getSqlTypeName()); |
There was a problem hiding this comment.
Instead of create a new type, i would prefer to let the SqlTypeUtil.convertTypeToSpec(type, charSet, maxPrecision) support a explicit precision parameter.
There was a problem hiding this comment.
Thanks a lot , I have already addressed.
wenhuitang
force-pushed
the
CALCITE-3468
branch
from
56514ed to
04f89cc
Compare
wenhuitang
force-pushed
the
CALCITE-3468
branch
from
04f89cc to
e85cc72
Compare
danny0405
reviewed
Mar 26, 2020
| * @return corresponding parse representation | ||
| */ | ||
| public static SqlDataTypeSpec convertTypeToSpec(RelDataType type, | ||
| String charSetName, int maxPrecision) { | ||
| String charSetName, int maxPrecision, | ||
| boolean allowsPrecisionUnspecified, int defaultPrecision) { | ||
| SqlTypeName typeName = type.getSqlTypeName(); |
There was a problem hiding this comment.
I would suggest to replace allowsPrecisionUnspecified and defaultPrecision with a single explicitPrecision, and if a explicitPrecision is specified, use it directly. Leave out the complex allowsPrecisionUnspecified decision to the invoker.
There was a problem hiding this comment.
This pull request has been updated, thanks for your reviewing, it's really helpful.
…without the precision and for char with the precision exceeding max length of Oracle (Wenhui Tang)
wenhuitang
force-pushed
the
CALCITE-3468
branch
from
e85cc72 to
2a9a71c
Compare
danny0405
added
the
accepted-and-would-rework-soon
|
Hi @wenhuitang, could you rebase the PR? |
julianhyde
force-pushed
the
main
branch
2 times, most recently
from
8a5cf83 to
cf7f71b
Compare
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.
Oracle requires VARCHAR with precision, and the max precision of VARCHAR is 4000, the max precision of CHAR is 2000. When the precision of CHAR is not assigned, it has the default value 1 which is the same as Calcite.