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.

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

Add support for more JSR-310 related classes about JDBC Timestamp in IntervalShardingAlgorithm #17754

Merged
merged 4 commits into from
May 19, 2022

Conversation

linghengqian
Copy link
Member

@linghengqian linghengqian commented May 17, 2022

For #17752.

Changes proposed in this pull request:

  • Added support for java.time.Instant, java.time.OffsetDateTime, java.time.ZonedDateTime in org.apache.shardingsphere.sharding.algorithm.sharding.datetime.IntervalShardingAlgorithm.
  • Added unit tests for java.sql.Timestamp, java.time.Instant, java.time.OffsetDateTime, java.time.ZonedDateTime in org.apache.shardingsphere.sharding.algorithm.sharding.datetime.IntervalShardingAlgorithmTest.

….OffsetDateTime, java.time.ZonedDateTime. IntervalShardingAlgorithm Added tests for java.time.Instant, java.time.OffsetDateTime, java.time.ZonedDateTime support.
@linghengqian
Copy link
Member Author

linghengqian commented May 17, 2022

  • Handling of classes such as java.time.LocalDate(JDBC Type is Date in Mybatis),java.time.LocalTime(JDBC Type is Time in Mybatis), java.time.OffsetTime(JDBC Type is Time in Mybatis), java.time.Year(JDBC Type is INTEGER in Mybatis), java.time.Month(JDBC Type is INTEGER in Mybatis), java.time.YearMonth(JDBC Type is VARCHAR or LONGVARCHAR in Mybatis) and java.time.chrono.JapaneseDate(JDBC Type is Date in Mybatis) will be added later.

@strongduanmu strongduanmu added this to the 5.1.2 milestone May 17, 2022
@linghengqian
Copy link
Member Author

linghengqian commented May 18, 2022

  • @strongduanmu I've noticed that when IntervalShardingAlgorithm and LocalDateTime are strongly coupled to properties other than the incoming value, this is a difficult thing for me to deal with due to my lack of experience dealing with the common superclass of LocalDateTime and LocalDate. I will not close the original issue, because I hope some friends can help improve the design of org.apache.shardingsphere.sharding.algorithm.sharding.datetime.IntervalShardingAlgorithm. At the same time, an unsupported list will be listed in the corresponding issue. 🙈

  • Although ZonedDateTime has a time zone, since its JDBC Type(Timestamp) has no concept of a time zone, I don't feel it should add an extra time zone to Greenwich Mean Time.

@linghengqian linghengqian marked this pull request as ready for review May 18, 2022 03:15
@linghengqian linghengqian changed the title Add support for more JSR-310 related classes in IntervalShardingAlgorithm Add support for more JSR-310 related classes about JDBC TimeStamp in IntervalShardingAlgorithm May 18, 2022
@linghengqian linghengqian changed the title Add support for more JSR-310 related classes about JDBC TimeStamp in IntervalShardingAlgorithm Add support for more JSR-310 related classes about JDBC Timestamp in IntervalShardingAlgorithm May 18, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #17754 (b6de16c) into master (c1fe31c) will increase coverage by 0.14%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master   #17754      +/-   ##
============================================
+ Coverage     59.02%   59.16%   +0.14%     
- Complexity     2128     2131       +3     
============================================
  Files          3593     3593              
  Lines         53378    53373       -5     
  Branches       9116     9115       -1     
============================================
+ Hits          31506    31580      +74     
+ Misses        19207    19121      -86     
- Partials       2665     2672       +7     
Impacted Files Coverage Δ
...m/sharding/datetime/IntervalShardingAlgorithm.java 90.19% <100.00%> (+0.40%) ⬆️
...gsphere/infra/metadata/ShardingSphereMetaData.java 9.52% <0.00%> (-3.81%) ⬇️
...n/updatable/AlterSQLParserRuleStatementAssert.java 66.66% <0.00%> (-2.09%) ⬇️
...gsphere/mode/metadata/MetaDataContextsBuilder.java 95.45% <0.00%> (-0.98%) ⬇️
.../manager/cluster/ClusterContextManagerBuilder.java 67.46% <0.00%> (-0.39%) ⬇️
...dingsphere/proxy/backend/context/ProxyContext.java 60.00% <0.00%> (ø)
...ser/core/common/CommonDistSQLStatementVisitor.java 0.00% <0.00%> (ø)
.../SQLParserCacheOptionConfigurationYamlSwapper.java 0.00% <0.00%> (ø)
...de/manager/memory/MemoryContextManagerBuilder.java 0.00% <0.00%> (ø)
...rdingGeneratedKeyInsertValueParameterRewriter.java 89.47% <0.00%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1fe31c...b6de16c. Read the comment docs.

@strongduanmu
Copy link
Member

@linghengqian Thank you, let's see if anyone else is willing to submit a PR to support the LocalDate.

@strongduanmu strongduanmu merged commit 800fcc8 into apache:master May 19, 2022
@linghengqian linghengqian deleted the fix-jsr-310 branch May 19, 2022 02:25
zhaoguhong added a commit to zhaoguhong/shardingsphere that referenced this pull request May 19, 2022
* master:
  Fix the exception when specifying schema when JDBC connects to Proxy (apache#17611)
  Add ProxyContextRestorer to restore proxy context in test cases (apache#17802)
  Remove redundant blank chars and lines in codes (apache#17799)
  Supports scaling MySQL table which only contains unique index (apache#17786)
  Add unit test for ModShardingAlgorithm check Argument(apache#17695) (apache#17772)
  Move getSchemas method to ShardingSphereDatabase (apache#17798)
  Fix unit tests in proxy-frontend-core affected by other tests (apache#17796)
  Update powered by (apache#17795)
  Add support for more JSR-310 related classes about JDBC Timestamp in IntervalShardingAlgorithm (apache#17754)
  Rename ShardingSphereMetaData to ShardingSphereDatabaseMetaData (apache#17792)
  Rename MetaDataContexts.persistService (apache#17789)
  Move ContextManager.executorEngine (apache#17783)
  Change proxy log print at Docker images, get more information (apache#17788)
  fix datasource closed exception for db-discovery (apache#17774)
  Add lock method for schema in lock manager (apache#17787)
  Refactor MetaDataChangedWatcher (apache#17781)
  Refactor PipelineTableMetaDataLoader and fix typo in PipelineTableMetaData (apache#17778)
  Rename DatabaseRulesBuilder (apache#17780)

# Conflicts:
#	shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/test/java/org/apache/shardingsphere/encrypt/merge/dql/EncryptAlgorithmMetaDataTest.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants