github-actions[bot] commented on code in PR #65219:
URL: https://github.com/apache/doris/pull/65219#discussion_r3526154127
##########
fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java:
##########
@@ -282,7 +283,19 @@ private ArrayList<AddPartitionOp>
getAddPartitionOp(Database db, OlapTable olapT
ArrayList<AddPartitionOp> addPartitionOps = new ArrayList<>();
DynamicPartitionProperty dynamicPartitionProperty =
olapTable.getTableProperty().getDynamicPartitionProperty();
RangePartitionInfo rangePartitionInfo = (RangePartitionInfo)
olapTable.getPartitionInfo();
- ZonedDateTime now =
ZonedDateTime.now(dynamicPartitionProperty.getTimeZone().toZoneId());
+ // For TIMESTAMPTZ, partition boundaries must be at UTC midnight
(00:00—24:00)
+ // regardless of the configured timezone. Partition names are derived
from
+ // the configured timezone to maintain compatibility with pre-fix
behavior.
+ boolean isTimestampTz = partitionColumn.getDataType() ==
PrimitiveType.TIMESTAMPTZ;
+ // nowTz: configured-timezone clock, used for naming and storage
properties
+ ZonedDateTime nowTz =
ZonedDateTime.now(dynamicPartitionProperty.getTimeZone().toZoneId());
Review Comment:
`nowTz` and `now` are sampled independently here, so a boundary crossing
between the two calls can make the partition name and the TIMESTAMPTZ range
come from different periods. For example, with `dynamic_partition.time_zone =
'+00:00'`, if `nowTz` is captured just before midnight and the UTC `now` just
after midnight, `prevBorderForName` produces yesterday's name while
`prevBorder`/`nextBorder` produce today's UTC range. Since the later existence
check is range-based, future scheduler runs will keep the wrong name. Please
capture one `Instant` once and derive both zoned values from it.
##########
fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java:
##########
@@ -282,7 +283,19 @@ private ArrayList<AddPartitionOp>
getAddPartitionOp(Database db, OlapTable olapT
ArrayList<AddPartitionOp> addPartitionOps = new ArrayList<>();
DynamicPartitionProperty dynamicPartitionProperty =
olapTable.getTableProperty().getDynamicPartitionProperty();
RangePartitionInfo rangePartitionInfo = (RangePartitionInfo)
olapTable.getPartitionInfo();
- ZonedDateTime now =
ZonedDateTime.now(dynamicPartitionProperty.getTimeZone().toZoneId());
+ // For TIMESTAMPTZ, partition boundaries must be at UTC midnight
(00:00—24:00)
+ // regardless of the configured timezone. Partition names are derived
from
+ // the configured timezone to maintain compatibility with pre-fix
behavior.
+ boolean isTimestampTz = partitionColumn.getDataType() ==
PrimitiveType.TIMESTAMPTZ;
+ // nowTz: configured-timezone clock, used for naming and storage
properties
+ ZonedDateTime nowTz =
ZonedDateTime.now(dynamicPartitionProperty.getTimeZone().toZoneId());
+ // now: used for border computation. For TIMESTAMPTZ, borders are
UTC-based.
+ ZonedDateTime now = isTimestampTz ? ZonedDateTime.now(ZoneOffset.UTC)
: nowTz;
Review Comment:
This new UTC boundary handling also needs to be applied to the dynamic
cleanup cutoff. `getDropPartitionOpForDynamic()` still builds its reserved
range from
`ZonedDateTime.now(dynamicPartitionProperty.getTimeZone().toZoneId())` and then
converts that local midnight with the configured timezone. With `time_zone =
Asia/Shanghai` and `start = -3`, the add path creates UTC-day ranges, but the
drop path reserves from `16:00:00+00:00`, so the previous UTC-day partition
still intersects the reserved range and is not dropped. Please derive the drop
cutoff with the same TIMESTAMPTZ UTC clock/source-timezone logic as the add
path.
##########
fe/fe-core/src/test/java/org/apache/doris/catalog/DynamicPartitionTableTest.java:
##########
@@ -2219,18 +2282,48 @@ public void testTimeStampTzDynamicPartitionHourUnit()
throws Exception {
Assert.assertEquals("Hour partition name length: " +
partitionName,
11, partitionName.length());
- // Verify range validity and UTC boundaries
+ // Verify range validity
Range<PartitionKey> range = item.getItems();
Assert.assertTrue("lower must be < upper",
range.lowerEndpoint().compareTo(range.upperEndpoint())
< 0);
- // With time_zone = "+00:00", partition boundaries should be
UTC timestamps
List<LiteralExpr> lowerKeys = range.lowerEndpoint().getKeys();
Assert.assertEquals(1, lowerKeys.size());
String lowerStr = lowerKeys.get(0).getStringValue();
Assert.assertTrue("Lower key must have +00:00 suffix: " +
lowerStr,
lowerStr.contains("+00:00"));
+
+ List<LiteralExpr> upperKeys = range.upperEndpoint().getKeys();
+ Assert.assertEquals(1, upperKeys.size());
+ String upperStr = upperKeys.get(0).getStringValue();
+ Assert.assertTrue("Upper key must have +00:00 suffix: " +
upperStr,
+ upperStr.contains("+00:00"));
+
+ // Partition boundaries must be at whole UTC hours (0-23).
+ // Adjacent partitions must differ by exactly 1 hour.
+ int lowerHour = Integer.parseInt(lowerStr.substring(11, 13));
+ Assert.assertTrue("Lower bound hour must be 0-23: " + lowerStr,
+ lowerHour >= 0 && lowerHour <= 23);
+ int upperHour = Integer.parseInt(upperStr.substring(11, 13));
+ Assert.assertTrue("Upper bound hour must be 0-23: " + upperStr,
+ upperHour >= 0 && upperHour <= 23);
+ if (prevLowerHour >= 0) {
+ Assert.assertEquals("Adjacent partitions' lower bounds
must differ by 1 hour",
Review Comment:
This compares only the hour-of-day fields, so the test fails whenever the
seven generated UTC hourly partitions cross midnight. At the current UTC hour,
the sorted ranges include a `23 -> 00` transition, but the assertion expects
`prevLowerHour + 1 == 24`. Please compare the full timestamp/partition key
sequence, or parse the bounds and add one hour across the date boundary instead
of comparing bare hour integers.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
