morrySnow commented on code in PR #65219:
URL: https://github.com/apache/doris/pull/65219#discussion_r3527186405
##########
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.
Review Comment:
**Minor efficiency:** When `isTimestampTz` is true, `ZonedDateTime.now()` is
called twice — once for `nowTz` (configured timezone) and once for `now` (UTC).
These are two independent system clock reads that could represent slightly
different instants. Consider deriving `now` from `nowTz` via
`nowTz.withZoneSameInstant(ZoneOffset.UTC)` instead — it saves a system call
and guarantees both represent the exact same instant.
##########
fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java:
##########
@@ -552,15 +568,21 @@ private ArrayList<DropPartitionOp>
getDropPartitionOpForDynamic(Database db, Ola
// int realStart =
DynamicPartitionUtil.getRealStart(dynamicPartitionProperty.getStart(),
// dynamicPartitionProperty.getHistoryPartitionNum());
int realStart = dynamicPartitionProperty.getStart();
- ZonedDateTime now =
ZonedDateTime.now(dynamicPartitionProperty.getTimeZone().toZoneId());
+ // For TIMESTAMPTZ, use UTC clock so the drop cutoff aligns with the
+ // UTC-midnight partition boundaries created by getAddPartitionOp().
+ boolean isTimestampTz = partitionColumn.getDataType() ==
PrimitiveType.TIMESTAMPTZ;
+ ZonedDateTime now = isTimestampTz
+ ? ZonedDateTime.now(ZoneOffset.UTC)
+ :
ZonedDateTime.now(dynamicPartitionProperty.getTimeZone().toZoneId());
+ TimeZone borderTimeZone = isTimestampTz
+ ? TimeZone.getTimeZone("UTC")
+ : dynamicPartitionProperty.getTimeZone();
Review Comment:
**Consistency with `getClosedRange()`:** The `borderTimeZone` logic is
applied correctly here. However, note that `getClosedRange()` (called below at
~line 602 inside the `reservedHistoryPeriods` block) still hardcodes
`dynamicPartitionProperty.getTimeZone()` instead of using the computed
`borderTimeZone`. For TIMESTAMPTZ tables with `reserved_history_periods`, this
would cause the reserved ranges to be at configured-timezone midnight while the
main drop cutoff is at UTC midnight. Consider passing `borderTimeZone` to
`getClosedRange()` to keep all boundary computations consistent.
--
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]
