Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fixes QueryResultUtil#getValueByColumnType unsigned int & bigint return value. #3288

Merged
merged 5 commits into from Oct 17, 2019
Merged

fixes QueryResultUtil#getValueByColumnType unsigned int & bigint return value. #3288

merged 5 commits into from Oct 17, 2019

Conversation

dongzl
Copy link
Contributor

@dongzl dongzl commented Oct 16, 2019

Fixes #3231 .

Changes proposed in this pull request:

  • INT UNSIGNED type, if value beyond java Integer range, return Long type.
  • INT UNSIGNED type, if value don't beyond java Integer range, return Integer type.
  • INT SIGNED type, return Integer type.
  • BIGINT UNSIGNED type, if value beyond java Long range, return BigDemical type.
  • BIGINT UNSIGNED type, if value don't beyond java Long range, return Long type.
  • BIGINT SIGNED type, return Long type.

@tuohai666 Please help me with the code review, Thanks.

@coveralls
Copy link

coveralls commented Oct 16, 2019 •
edited

Pull Request Test Coverage Report for Build 681

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 66 unchanged lines in 9 files lost coverage.
  • Overall coverage increased (+0.4%) to 67.861%
Files with Coverage Reduction New Missed Lines %
sharding-core/sharding-core-rewrite/src/main/java/org/apache/shardingsphere/core/rewrite/feature/encrypt/EncryptCondition.java 1 93.33%
sharding-core/sharding-core-rewrite/src/main/java/org/apache/shardingsphere/core/rewrite/feature/encrypt/token/generator/impl/EncryptAssignmentTokenGenerator.java 1 98.33%
sharding-core/sharding-core-rewrite/src/main/java/org/apache/shardingsphere/core/rewrite/feature/encrypt/token/pojo/EncryptPredicateToken.java 2 93.1%
sharding-core/sharding-core-entry/src/main/java/org/apache/shardingsphere/core/BaseShardingEngine.java 5 70.37%
sharding-proxy/sharding-proxy-backend/src/main/java/org/apache/shardingsphere/shardingproxy/backend/communication/jdbc/wrapper/PreparedStatementExecutorWrapper.java 6 5.41%
sharding-proxy/sharding-proxy-backend/src/main/java/org/apache/shardingsphere/shardingproxy/backend/communication/jdbc/wrapper/StatementExecutorWrapper.java 7 5.0%
sharding-core/sharding-core-rewrite/src/main/java/org/apache/shardingsphere/core/rewrite/feature/encrypt/EncryptConditionEngine.java 10 64.15%
sharding-jdbc/sharding-jdbc-core/src/main/java/org/apache/shardingsphere/shardingjdbc/jdbc/core/statement/EncryptPreparedStatement.java 11 75.0%
sharding-jdbc/sharding-jdbc-core/src/main/java/org/apache/shardingsphere/shardingjdbc/jdbc/core/statement/EncryptStatement.java 23 56.96%
Totals Coverage Status
Change from base Build 476: 0.4%
Covered Lines: 11305
Relevant Lines: 16659
💛 - Coveralls

qixiaobo
qixiaobo reviewed Oct 16, 2019
View reviewed changes
...src/main/java/org/apache/shardingsphere/core/execute/sql/execute/result/QueryResultUtil.java Outdated
}
BigDecimal value = resultSet.getBigDecimal(columnIndex);
if (new BigDecimal(Long.MAX_VALUE).compareTo(value) <= -1) {
return value;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe return value.toBigInteger()?

Copy link
Contributor Author

@dongzl dongzl Oct 17, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe return value.toBigInteger()?

Hi, yes, return BigInteger is a good way, as the same with ShardingSphere team review suggestion. Thanks.

KomachiSion
KomachiSion requested changes Oct 16, 2019
View reviewed changes
...src/main/java/org/apache/shardingsphere/core/execute/sql/execute/result/QueryResultUtil.java Outdated Show resolved Hide resolved
...src/main/java/org/apache/shardingsphere/core/execute/sql/execute/result/QueryResultUtil.java Outdated Show resolved Hide resolved
...src/main/java/org/apache/shardingsphere/core/execute/sql/execute/result/QueryResultUtil.java Outdated
BigDecimal value = resultSet.getBigDecimal(columnIndex);
if (new BigDecimal(Long.MAX_VALUE).compareTo(value) <= -1) {
return value;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use BigInteger replace BigDecimal may be better.

can use getBigDecimal().toBigInteger to get BigInteger

@terrymanu terrymanu added this to the 4.0.0-RC3 milestone Oct 17, 2019
@terrymanu terrymanu merged commit 502240b into apache:dev Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qixiaobo qixiaobo qixiaobo left review comments

@KomachiSion KomachiSion KomachiSion approved these changes

Assignees
No one assigned
Labels
in: JDBC type: bug
Projects
None yet
Milestone
4.0.0-RC3
Development

Successfully merging this pull request may close these issues.

Issue with data type "INT UNSIGNED"
5 participants
@dongzl @coveralls @qixiaobo @KomachiSion @terrymanu