github-actions[bot] commented on code in PR #65219:
URL: https://github.com/apache/doris/pull/65219#discussion_r3621217123
##########
fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java:
##########
@@ -201,23 +199,76 @@ private static long getNextPartitionSize(List<Long>
historyPartitionsSize) {
}
private static Pair<Integer, Integer>
getBucketsNum(DynamicPartitionProperty property, OlapTable table,
- String partitionName, String nowPartitionName, boolean
executeFirstTime) {
+ String partitionName, String nowPartitionName, boolean
executeFirstTime, String currentUtcBorder) {
AutoBucketCalculator.AutoBucketContext context = new
AutoBucketCalculator.AutoBucketContext(
- table, partitionName, nowPartitionName, executeFirstTime,
property.getBuckets());
+ table, partitionName, nowPartitionName, executeFirstTime,
property.getBuckets(),
+ currentUtcBorder);
AutoBucketCalculator.AutoBucketResult result =
AutoBucketCalculator.calculateAutoBuckets(context);
return Pair.of(result.getBuckets(), result.getPreviousBuckets());
}
- public static List<Partition> getHistoricalPartitions(OlapTable table,
String nowPartitionName) {
+ /**
+ * Get all partitions except the current one. When currentKey is non-null,
+ * the current partition is identified by matching its range lower bound
+ * against currentKey (parsed from currentUtcBorder). When currentKey is
+ * null, fall back to name-based exclusion against nowPartitionName.
+ *
+ * <p>The two filters are mutually exclusive — when a range key is
available
+ * it alone identifies the current partition. Applying both filters on top
+ * of each other would double-exclude: after the range match removes the
+ * current partition (whose name may use an old prefix after a prefix
change),
+ * the name filter could then catch a <em>different</em> partition whose
name
+ * happens to match the new prefix, losing two partitions and corrupting
+ * auto-bucket calculations.
+ */
+ public static List<Partition> getHistoricalPartitions(OlapTable table,
String nowPartitionName,
+ String currentUtcBorder) {
table.readLock();
try {
RangePartitionInfo info = (RangePartitionInfo)
(table.getPartitionInfo());
+ // Parse the range key once, before the stream. A single parse
+ // avoids redundant PartitionKey.createPartitionKey calls for every
+ // partition while holding the table read lock.
+ PartitionKey currentKey = null;
+ if (currentUtcBorder != null) {
+ try {
+ Column partitionColumn = info.getPartitionColumns().get(0);
+ PartitionValue currentValue = new
PartitionValue(currentUtcBorder);
+ currentKey = PartitionKey.createPartitionKey(
+ Collections.singletonList(currentValue),
+ Collections.singletonList(partitionColumn));
+ } catch (AnalysisException e) {
+ // Parsing failed — fall back to name-based exclusion
below.
+ currentKey = null;
+ }
+ }
+
+ final PartitionKey rangeKey = currentKey;
+
List<Map.Entry<Long, PartitionItem>> idToItems = new
ArrayList<>(info.getIdToItem(false).entrySet());
idToItems.sort(Comparator.comparing(o -> ((RangePartitionItem)
o.getValue()).getItems().upperEndpoint()));
return idToItems.stream()
.map(entry -> table.getPartition(entry.getKey()))
- .filter(partition -> partition != null &&
!partition.getName().equals(nowPartitionName))
+ .filter(partition -> {
+ if (partition == null) {
+ return false;
+ }
+ if (rangeKey != null) {
+ // Range-based exclusion only — mutually exclusive
+ // with name-based exclusion to avoid
double-filtering.
+ RangePartitionItem item = (RangePartitionItem)
info.getItem(partition.getId());
+ if (item != null) {
+ PartitionKey lower =
item.getItems().lowerEndpoint();
+ if (lower.compareTo(rangeKey) == 0) {
Review Comment:
[P2] Exclude the range containing the captured current instant
Exact lower-key equality still keeps a populated current range when it is
noncanonical. For example, at 2026-07-20 12:00Z a pre-fix Asia/Shanghai range
can be p20260720=[2026-07-19 16:00Z, 2026-07-20 16:00Z); the new current key
2026-07-20 00:00Z is inside it but is not its lower endpoint. If a
nonoverlapping future UTC partition is missing (for example after extending
end), that add reaches auto-bucket and this active, still-growing partition is
treated as history, whereas the old name path excluded p20260720. This is
independently reachable through the supported disable/add/re-enable flow with a
populated, current-named manual range that contains the captured `now` instant
but not the canonical border. Please pass that same instant through
`AutoBucketContext`, exclude the unique range containing it, and cover the
periodic AUTO BUCKET path with a populated noncanonical current range.
##########
fe/fe-core/src/test/java/org/apache/doris/catalog/DynamicPartitionTableTest.java:
##########
@@ -2219,17 +2318,159 @@ 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
(minute=second=00).
+ // A configured timezone with a fractional offset
(Asia/Kathmandu,
+ // UTC+05:45) would produce boundaries with non-zero minutes
if the
+ // old configured-timezone flooring were used instead of
UTC-first.
+ String lowerMinutes = lowerStr.substring(14, 16);
+ String lowerSeconds = lowerStr.substring(17, 19);
+ Assert.assertEquals("Lower bound minutes must be 00: " +
lowerStr,
+ "00", lowerMinutes);
+ Assert.assertEquals("Lower bound seconds must be 00: " +
lowerStr,
+ "00", lowerSeconds);
+ String upperMinutes = upperStr.substring(14, 16);
+ String upperSeconds = upperStr.substring(17, 19);
+ Assert.assertEquals("Upper bound minutes must be 00: " +
upperStr,
+ "00", upperMinutes);
+ Assert.assertEquals("Upper bound seconds must be 00: " +
upperStr,
+ "00", upperSeconds);
+
+ // Verify adjacency using full timestamps (handles midnight
crossing)
+ if (prevLower != null) {
+ ZonedDateTime expectedNext = prevLower.plusHours(1);
+ ZonedDateTime actual = ZonedDateTime.parse(lowerStr,
+ DateTimeFormatter.ofPattern("yyyy-MM-dd
HH:mm:ssXXX"));
+ Assert.assertEquals("Adjacent partitions' lower bounds
must differ by 1 hour",
+ expectedNext, actual);
+ ZonedDateTime expectedUpper = prevUpper.plusHours(1);
+ ZonedDateTime actualUpper = ZonedDateTime.parse(upperStr,
+ DateTimeFormatter.ofPattern("yyyy-MM-dd
HH:mm:ssXXX"));
+ Assert.assertEquals("Adjacent partitions' upper bounds
must differ by 1 hour",
+ expectedUpper, actualUpper);
+ }
+ prevLower = ZonedDateTime.parse(lowerStr,
+ DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ssXXX"));
+ prevUpper = ZonedDateTime.parse(upperStr,
+ DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ssXXX"));
+ }
+
+ // Identify the current partition (idx=0) by its stored range.
+ // start=-3,end=3 → idx=0 is at index 3.
+ RangePartitionItem currentItem = (RangePartitionItem)
sortedEntries.get(3).getValue();
+ String currentLowerStr =
currentItem.getItems().lowerEndpoint().getKeys().get(0)
+ .getStringValue();
+ ZonedDateTime currentLower = ZonedDateTime.parse(currentLowerStr,
+ DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ssXXX"));
+ String expectedCurrentName = "p"
+ +
DateTimeFormatter.ofPattern("yyyyMMddHH").format(currentLower);
+ String actualCurrentName =
table.getPartition(sortedEntries.get(3).getKey()).getName();
+ Assert.assertEquals("Current partition (idx=0) hour name must
match its UTC lower bound",
+ expectedCurrentName, actualCurrentName);
+ // With a fractional-offset timezone, only the UTC-first approach
+ // guarantees minute=second=00 on every bound.
+ Assert.assertEquals("Current partition lower bound must end
:00:00: " + currentLowerStr,
+ "00", currentLowerStr.substring(14, 16)); // minutes
+ Assert.assertEquals("Current partition lower bound must end
:00:00: " + currentLowerStr,
+ "00", currentLowerStr.substring(17, 19)); // seconds
+ } finally {
+ connectContext.getSessionVariable().setTimeZone(originalTimeZone);
+ }
+ }
+
+ @Test
+ public void testTimeStampTzDynamicPartitionDropCutoffAligned() throws
Exception {
+ // With time_zone = "Asia/Shanghai", the old drop-path code computed
+ // the reserved range lower bound at the configured timezone's midnight
+ // (16:00 UTC) instead of UTC midnight (00:00). This caused partitions
+ // just before UTC midnight to intersect the shifted reserved range and
+ // not be dropped. Verify the drop cutoff now aligns with the add path.
+ String originalTimeZone =
connectContext.getSessionVariable().getTimeZone();
+ try {
+ connectContext.getSessionVariable().setTimeZone("America/Chicago");
+
+ String createOlapTblStmt = "CREATE TABLE test.`tstz_drop_cutoff`
(\n"
+ + " `k1` TIMESTAMPTZ NULL COMMENT \"\",\n"
+ + " `k2` int NULL COMMENT \"\"\n"
+ + ") ENGINE=OLAP\n"
+ + "DUPLICATE KEY(`k1`, `k2`)\n"
+ + "PARTITION BY RANGE(`k1`)\n"
+ + "()\n"
+ + "DISTRIBUTED BY HASH(`k2`) BUCKETS 3\n"
+ + "PROPERTIES (\n"
+ + "\"replication_num\" = \"1\",\n"
+ + "\"dynamic_partition.enable\" = \"true\",\n"
+ + "\"dynamic_partition.start\" = \"-1\",\n"
+ + "\"dynamic_partition.end\" = \"1\",\n"
+ + "\"dynamic_partition.create_history_partition\" =
\"false\",\n"
+ + "\"dynamic_partition.time_unit\" = \"day\",\n"
+ + "\"dynamic_partition.prefix\" = \"p\",\n"
+ + "\"dynamic_partition.buckets\" = \"1\",\n"
+ + "\"dynamic_partition.time_zone\" = \"Asia/Shanghai\"\n"
+ + ");";
+ createTable(createOlapTblStmt);
+
+ Database db =
Env.getCurrentInternalCatalog().getDbOrAnalysisException("test");
+ OlapTable table = (OlapTable)
db.getTableOrAnalysisException("tstz_drop_cutoff");
+
+ // Add a partition for two days ago (UTC) that should be dropped
+ // by the start=-1 cutoff. Must temporarily disable dynamic
partition
+ // to manually add a partition.
+ ZonedDateTime utcNow = ZonedDateTime.now(ZoneOffset.UTC);
Review Comment:
[P2] Make this cutoff regression discriminate throughout the day
This fixture only distinguishes the old configured-zone cutoff from the UTC
cutoff before 16:00Z. At or after 16:00Z, Shanghai is already on the next
calendar day, so the legacy start=-1 cutoff is D-1 16:00Z while p_old ends at
D-1 00:00Z; the old code drops it too, and this assertion passes without the
fix for eight hours every day. Please inject a fixed clock that the scheduler
consumes and construct p_old from that same instant, so this test always fails
on the previous implementation.
--
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]
