You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/12/20 08:48:29 UTC
[GitHub] [shardingsphere] klniu opened a new pull request #8693: Fix parseDateTime error when sharding value is Date or LocalDateTime
klniu opened a new pull request #8693:
URL: https://github.com/apache/shardingsphere/pull/8693
Fix #8470
Changes proposed in this pull request:
- There is not need to import com.google.common.base.Function, and it can't compile after importing.
- The second commit is fixing the parsing of dateTime. when the sharding value is LocalDateTime or Date, toString() will give the string with the format as yyyy-MM-dd'T'HH:mm:ss. and it can not be parsed when user already set the different date format in configuation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] klniu closed pull request #8693: Fix parseDateTime error when sharding value is Date or LocalDateTime
Posted by GitBox <gi...@apache.org>.
klniu closed pull request #8693:
URL: https://github.com/apache/shardingsphere/pull/8693
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #8693: Fix parseDateTime error when sharding value is Date or LocalDateTime
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #8693:
URL: https://github.com/apache/shardingsphere/pull/8693#issuecomment-748734546
Hi @klniu ,
Thanks for your PR, could you firstly look at the CI test and fix them?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] yu199195 commented on a change in pull request #8693: Fix parseDateTime error when sharding value is Date or LocalDateTime
Posted by GitBox <gi...@apache.org>.
yu199195 commented on a change in pull request #8693:
URL: https://github.com/apache/shardingsphere/pull/8693#discussion_r555009669
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-common/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/datetime/AutoIntervalShardingAlgorithm.java
##########
@@ -124,7 +126,16 @@ private int getLastPartition(final Range<Comparable<?>> valueRange) {
}
private long parseDate(final Comparable<?> shardingValue) {
- LocalDateTime dateValue = LocalDateTime.parse(shardingValue.toString(), DATE_TIME_FORMAT);
+ LocalDateTime dateValue;
Review comment:
so this pr ,we can not merged, Do you have any other fix, if not can you close this pr?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] yu199195 commented on a change in pull request #8693: Fix parseDateTime error when sharding value is Date or LocalDateTime
Posted by GitBox <gi...@apache.org>.
yu199195 commented on a change in pull request #8693:
URL: https://github.com/apache/shardingsphere/pull/8693#discussion_r547175395
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-common/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/datetime/AutoIntervalShardingAlgorithm.java
##########
@@ -124,7 +126,16 @@ private int getLastPartition(final Range<Comparable<?>> valueRange) {
}
private long parseDate(final Comparable<?> shardingValue) {
- LocalDateTime dateValue = LocalDateTime.parse(shardingValue.toString(), DATE_TIME_FORMAT);
+ LocalDateTime dateValue;
Review comment:
Thank you very much for your contribution.
Now I would like to know when shardingValue is Date or LocalDateTime?
We can't judge the treatment so simply, because we define DateTimeFormatter like this
```
DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss")
```
if you data or config not follow this, An exception is thrown, so you need to follow this DateTimeFormatter .
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero edited a comment on pull request #8693: Fix parseDateTime error when sharding value is Date or LocalDateTime
Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on pull request #8693:
URL: https://github.com/apache/shardingsphere/pull/8693#issuecomment-748734546
Hi @klniu ,
Thanks for your PR, could you firstly look at the CI tests and fix them?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] klniu commented on a change in pull request #8693: Fix parseDateTime error when sharding value is Date or LocalDateTime
Posted by GitBox <gi...@apache.org>.
klniu commented on a change in pull request #8693:
URL: https://github.com/apache/shardingsphere/pull/8693#discussion_r547621741
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-common/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/datetime/AutoIntervalShardingAlgorithm.java
##########
@@ -124,7 +126,16 @@ private int getLastPartition(final Range<Comparable<?>> valueRange) {
}
private long parseDate(final Comparable<?> shardingValue) {
- LocalDateTime dateValue = LocalDateTime.parse(shardingValue.toString(), DATE_TIME_FORMAT);
+ LocalDateTime dateValue;
Review comment:
Thanks.
After I configured the config as you said, it is normal, but the value passed to doSharding is still LocalDateTime, The configuration just make the formatter yyyy-MM-ddTHH:mm:ss the same as LocalDateTime.toString() using. So the different configuaration is not respected.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] klniu commented on a change in pull request #8693: Fix parseDateTime error when sharding value is Date or LocalDateTime
Posted by GitBox <gi...@apache.org>.
klniu commented on a change in pull request #8693:
URL: https://github.com/apache/shardingsphere/pull/8693#discussion_r547587149
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-common/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/datetime/AutoIntervalShardingAlgorithm.java
##########
@@ -124,7 +126,16 @@ private int getLastPartition(final Range<Comparable<?>> valueRange) {
}
private long parseDate(final Comparable<?> shardingValue) {
- LocalDateTime dateValue = LocalDateTime.parse(shardingValue.toString(), DATE_TIME_FORMAT);
+ LocalDateTime dateValue;
Review comment:
It happened when the type of the sharding value is LocalDateTime. Here is the configuration:
```
dt_partition_detection:
actual-data-nodes: dsd$->{0..9}.dt_partition_detection_$->{2013..2025}0$->{1..4}
database-strategy:
standard:
sharding-column: sys_org_code
sharding-algorithmName: detection-db-sharding
tableStrategy:
standard:
sharding-column: time
sharding-algorithm-name: detection-table-sharding
key-generate-strategy:
type: SNOWFLAKE
column: id
...
detection-table-sharding:
type: INTERVAL
props:
datetime-pattern: yyyy-MM-dd HH:mm:ss
datetime-lower: '2013-01-01 00:00:00'
sharding-suffix-pattern: yyyyQQ
datetime-interval-amount: '3'
datetime-interval-unit: MONTHS
```
Using INTERVAL sharding rule，when insert a entity, whose field **time** is LocalDateTime.
After PreparedStatement.execute(); it will call doSharding using the arguments passed to PreparedStatement.
**time** is not called with the toString() method, but directly pass the raw value. Like the image showing:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] yu199195 commented on a change in pull request #8693: Fix parseDateTime error when sharding value is Date or LocalDateTime
Posted by GitBox <gi...@apache.org>.
yu199195 commented on a change in pull request #8693:
URL: https://github.com/apache/shardingsphere/pull/8693#discussion_r547613943
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-common/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/datetime/AutoIntervalShardingAlgorithm.java
##########
@@ -124,7 +126,16 @@ private int getLastPartition(final Range<Comparable<?>> valueRange) {
}
private long parseDate(final Comparable<?> shardingValue) {
- LocalDateTime dateValue = LocalDateTime.parse(shardingValue.toString(), DATE_TIME_FORMAT);
+ LocalDateTime dateValue;
Review comment:
can you config datetime-pattern: yyyy-MM-ddTHH:mm:ss ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org