Skip to content

Update IntervalShardingAlgorithm.java to fix an exception caused by polymorphism#20255

Closed
cowboysync wants to merge 8 commits into
apache:masterfrom
cowboysync:master
Closed

Update IntervalShardingAlgorithm.java to fix an exception caused by polymorphism#20255
cowboysync wants to merge 8 commits into
apache:masterfrom
cowboysync:master

Conversation

@cowboysync

@cowboysync cowboysync commented Aug 18, 2022

Copy link
Copy Markdown

the method private String getDateTimeText(final Comparable<?> endpoint) will throws an java.lang.UnsupportedOperationException if the parameter endpoint's type is java.sql.Date, because of java.sql.Date values do not have a time component, so it's mehtod public Instant toInstant() always throws an java.lang.UnsupportedOperationException.

Reproduction example: examples/shardingsphere-jdbc-example/single-feature-example/sharding-example/sharding-raw-jdbc-example/src/main/java/org/apache/shardingsphere/example/sharding/raw/jdbc/ShardingRawYamlIntervalConfigurationExample.java

image

Changes proposed in this pull request:

  • fix an exception caused by polymorphism

the method private String getDateTimeText(final Comparable<?> endpoint) will throws an java.lang.UnsupportedOperationException if the parameter endpoint's type is java.sql.Date, because of java.sql.Date values do not have a time component, so it's mehtod public Instant toInstant() always throws an java.lang.UnsupportedOperationException.
lwclover
lwclover previously approved these changes Aug 18, 2022
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #20255 (1c12200) into master (3b15d53) will increase coverage by 0.02%.
The diff coverage is 64.00%.

❗ Current head 1c12200 differs from pull request most recent head 640072f. Consider uploading reports for the commit 640072f to get more accurate results

@@             Coverage Diff              @@
##             master   #20255      +/-   ##
============================================
+ Coverage     60.87%   60.90%   +0.02%     
  Complexity     2422     2422              
============================================
  Files          3876     3876              
  Lines         54690    54692       +2     
  Branches       9310     9311       +1     
============================================
+ Hits          33295    33311      +16     
+ Misses        18542    18529      -13     
+ Partials       2853     2852       -1     
Impacted Files Coverage Δ
...m/sharding/datetime/IntervalShardingAlgorithm.java 78.04% <0.00%> (-0.97%) ⬇️
...gment/expression/impl/ListExpressionConverter.java 0.00% <0.00%> (ø)
...epare/datasource/PrepareTargetTablesParameter.java 0.00% <ø> (ø)
...r/statement/impl/OpenGaussStatementSQLVisitor.java 84.76% <71.42%> (+0.49%) ⬆️
...ssion/impl/BinaryOperationExpressionConverter.java 97.05% <83.33%> (-2.95%) ⬇️
.../statement/impl/PostgreSQLStatementSQLVisitor.java 88.05% <85.71%> (+1.02%) ⬆️
...handler/distsql/ral/hint/enums/HintSourceType.java 42.85% <0.00%> (+42.85%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@linghengqian linghengqian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • The CI bug was fixed by #20274, this PR should merge the master branch.

  • It is better to add a test case of java.sql.Date to the corresponding unit test class of this class. Previous tests missed this class.

use Java 8 Date Time API to construct java.sql.Date instance
@cowboysync cowboysync changed the title Update IntervalShardingAlgorithm.java Update IntervalShardingAlgorithm.java to fix an exception caused by polymorphism Aug 29, 2022
@cowboysync cowboysync closed this Sep 3, 2022
@linghengqian

Copy link
Copy Markdown
Member
  • There are too many commits for this PR, you might open a new issue and submit a new PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants