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 2022/01/28 03:10:41 UTC
[GitHub] [shardingsphere] sandynz commented on a change in pull request #15131: Enhanced timestamp format handling in AutoIntervalShardingAlgorithm
sandynz commented on a change in pull request #15131:
URL: https://github.com/apache/shardingsphere/pull/15131#discussion_r794170961
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/main/java/org/apache/shardingsphere/sharding/algorithm/sharding/datetime/AutoIntervalShardingAlgorithm.java
##########
@@ -64,16 +65,16 @@
public void init() {
dateTimeLower = getDateTime(DATE_TIME_LOWER_KEY);
shardingSeconds = getShardingSeconds();
- autoTablesAmount = (int) (Math.ceil(parseDate(props.getProperty(DATE_TIME_UPPER_KEY)) / shardingSeconds) + 2);
+ autoTablesAmount = (int) (Math.ceil((double) (parseDate(props.getProperty(DATE_TIME_UPPER_KEY)) / shardingSeconds)) + 2);
Review comment:
Question: `parseDate` return `long` (just as PR says), is `(double)` casting necessary?
And I found there's `(float)` casting in `doSharding` method, should it be `(double)` too?
##########
File path: shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-core/src/test/java/org/apache/shardingsphere/sharding/algorithm/sharding/datetime/AutoIntervalShardingAlgorithmTest.java
##########
@@ -119,4 +119,45 @@ public void assertRangeDoShardingWithGreaterTenTables() {
Collection<String> actual = shardingAlgorithm.doSharding(availableTargetNames, new RangeShardingValue<>("t_order", "create_time", Range.closed("2020-01-01 00:00:00", "2020-01-01 00:00:10")));
assertThat(actual.size(), is(11));
}
+
+ @Test
+ public void assertRangeDoShardingInValueWithMilliseconds() {
+ AutoIntervalShardingAlgorithm shardingAlgorithm = new AutoIntervalShardingAlgorithm();
+ shardingAlgorithm.getProps().setProperty("datetime-lower", "2020-01-01 00:00:00");
+ shardingAlgorithm.getProps().setProperty("datetime-upper", "2020-01-01 00:00:30");
+ shardingAlgorithm.getProps().setProperty("sharding-seconds", "1");
+ shardingAlgorithm.init();
+ List<String> availableTargetNames = new LinkedList<>();
+ for (int i = 0; i < 32; i++) {
+ availableTargetNames.add("t_order_" + i);
+ }
+ Collection<String> actualWithoutMilliseconds = shardingAlgorithm.doSharding(availableTargetNames,
+ new RangeShardingValue<>("t_order", "create_time", Range.closed("2020-01-01 00:00:11", "2020-01-01 00:00:21")));
+ assertThat(actualWithoutMilliseconds.size(), is(11));
+ Collection<String> actualWithOneMillisecond = shardingAlgorithm.doSharding(availableTargetNames,
+ new RangeShardingValue<>("t_order", "create_time", Range.closed("2020-01-01 00:00:11.1", "2020-01-01 00:00:21.1")));
+ assertThat(actualWithOneMillisecond.size(), is(11));
+ Collection<String> actualWithTwoMilliseconds = shardingAlgorithm.doSharding(availableTargetNames,
+ new RangeShardingValue<>("t_order", "create_time", Range.closed("2020-01-01 00:00:11.12", "2020-01-01 00:00:21.12")));
+ assertThat(actualWithTwoMilliseconds.size(), is(11));
+ Collection<String> actualWithThreeMilliseconds = shardingAlgorithm.doSharding(availableTargetNames,
+ new RangeShardingValue<>("t_order", "create_time", Range.closed("2020-01-01 00:00:11.123", "2020-01-01 00:00:21.123")));
+ assertThat(actualWithThreeMilliseconds.size(), is(11));
+ }
+
+ @Test
+ public void assertRangeDoShardingInPropertyWithMilliseconds() {
+ AutoIntervalShardingAlgorithm shardingAlgorithm = new AutoIntervalShardingAlgorithm();
+ shardingAlgorithm.getProps().setProperty("datetime-lower", "2020-01-01 00:00:00.123");
+ shardingAlgorithm.getProps().setProperty("datetime-upper", "2020-01-01 00:00:30.123");
+ shardingAlgorithm.getProps().setProperty("sharding-seconds", "1");
+ shardingAlgorithm.init();
+ List<String> availableTargetNames = new LinkedList<>();
+ for (int i = 0; i < 32; i++) {
+ availableTargetNames.add("t_order_" + i);
+ }
+ Collection<String> actualWithoutMilliseconds = shardingAlgorithm.doSharding(availableTargetNames,
+ new RangeShardingValue<>("t_order", "create_time", Range.closed("2020-01-01 00:00:11", "2020-01-01 00:00:21")));
+ assertThat(actualWithoutMilliseconds.size(), is(11));
Review comment:
Could we add more test cases for `doSharding` to verify certain value's sharding target, but not only the target size? (I've added comment in PR about date parsing)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org