From dc6b8bf6bf16e7c53df519d29efd9323cc6aac9c Mon Sep 17 00:00:00 2001 From: Benjamin Hopp Date: Mon, 27 Apr 2026 16:44:39 -0500 Subject: [PATCH 1/4] Fix hang in PeriodGranularity when using sub-day compound periods with imprecise timezone days --- .../common/granularity/PeriodGranularity.java | 10 ++++++++ .../granularity/PeriodGranularityBugTest.java | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 processing/src/test/java/org/apache/druid/java/util/common/granularity/PeriodGranularityBugTest.java diff --git a/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java b/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java index 909e80f72c95..f6a524e02a2d 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java @@ -600,6 +600,16 @@ private long truncateMillisPeriod(final long t) offset += millis; } return t - offset; + } else if (period.getYears() == 0 && period.getMonths() == 0 && period.getWeeks() == 0 && period.getDays() == 0) { + // If the period only contains hours, minutes, seconds, and millis, + // their duration is always precise even across daylight saving transitions. + // Therefore, we can safely convert the period to milliseconds. + final long millis = period.toStandardDuration().getMillis(); + long offset = t % millis - origin % millis; + if (offset < 0) { + offset += millis; + } + return t - offset; } else { throw new UnsupportedOperationException( "Period cannot be converted to milliseconds as some fields mays vary in length with chronology " + chronology diff --git a/processing/src/test/java/org/apache/druid/java/util/common/granularity/PeriodGranularityBugTest.java b/processing/src/test/java/org/apache/druid/java/util/common/granularity/PeriodGranularityBugTest.java new file mode 100644 index 000000000000..6ab46e44ee5f --- /dev/null +++ b/processing/src/test/java/org/apache/druid/java/util/common/granularity/PeriodGranularityBugTest.java @@ -0,0 +1,25 @@ +package org.apache.druid.java.util.common.granularity; + +import org.joda.time.DateTimeZone; +import org.joda.time.Period; +import org.junit.Assert; +import org.junit.Test; + +public class PeriodGranularityBugTest { + @Test(timeout = 5000) + public void testCompoundPeriodWithTimeZone() { + // America/New_York has DST, so days are imprecise + PeriodGranularity pg = new PeriodGranularity(new Period("PT1M1S"), null, DateTimeZone.forID("America/New_York")); + long time = 1704067200000L; // 2024-01-01T00:00:00Z + long start = System.currentTimeMillis(); + long bucket = pg.bucketStart(time); + long end = System.currentTimeMillis(); + + Assert.assertTrue("Should run quickly", (end - start) < 1000); + + // Let's also check if it returns the same as a non-compound period of same duration (PT61S) + PeriodGranularity pg2 = new PeriodGranularity(new Period("PT61S"), null, DateTimeZone.forID("America/New_York")); + long bucket2 = pg2.bucketStart(time); + Assert.assertEquals(bucket2, bucket); + } +} From 6fc794d30cbedc1a76685614a02e1227a4268e2b Mon Sep 17 00:00:00 2001 From: Benjamin Hopp Date: Mon, 27 Apr 2026 16:49:01 -0500 Subject: [PATCH 2/4] style: fix checkstyle issues in PeriodGranularityBugTest --- .../granularity/PeriodGranularityBugTest.java | 56 +++++++++++++------ 1 file changed, 39 insertions(+), 17 deletions(-) diff --git a/processing/src/test/java/org/apache/druid/java/util/common/granularity/PeriodGranularityBugTest.java b/processing/src/test/java/org/apache/druid/java/util/common/granularity/PeriodGranularityBugTest.java index 6ab46e44ee5f..97f39f3a755a 100644 --- a/processing/src/test/java/org/apache/druid/java/util/common/granularity/PeriodGranularityBugTest.java +++ b/processing/src/test/java/org/apache/druid/java/util/common/granularity/PeriodGranularityBugTest.java @@ -1,3 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + package org.apache.druid.java.util.common.granularity; import org.joda.time.DateTimeZone; @@ -5,21 +24,24 @@ import org.junit.Assert; import org.junit.Test; -public class PeriodGranularityBugTest { - @Test(timeout = 5000) - public void testCompoundPeriodWithTimeZone() { - // America/New_York has DST, so days are imprecise - PeriodGranularity pg = new PeriodGranularity(new Period("PT1M1S"), null, DateTimeZone.forID("America/New_York")); - long time = 1704067200000L; // 2024-01-01T00:00:00Z - long start = System.currentTimeMillis(); - long bucket = pg.bucketStart(time); - long end = System.currentTimeMillis(); - - Assert.assertTrue("Should run quickly", (end - start) < 1000); - - // Let's also check if it returns the same as a non-compound period of same duration (PT61S) - PeriodGranularity pg2 = new PeriodGranularity(new Period("PT61S"), null, DateTimeZone.forID("America/New_York")); - long bucket2 = pg2.bucketStart(time); - Assert.assertEquals(bucket2, bucket); - } +public class PeriodGranularityBugTest +{ + @Test(timeout = 5000) + public void testCompoundPeriodWithTimeZone() + { + // America/New_York has DST, so days are imprecise + PeriodGranularity pg = new PeriodGranularity(new Period("PT1M1S"), null, DateTimeZone.forID("America/New_York")); + long time = 1704067200000L; // 2024-01-01T00:00:00Z + long start = System.currentTimeMillis(); + long bucket = pg.bucketStart(time); + long end = System.currentTimeMillis(); + + Assert.assertTrue("Should run quickly", (end - start) < 1000); + + // Let's also check if it returns the same as a non-compound period of same duration (PT61S) + PeriodGranularity pg2 = new PeriodGranularity(new Period("PT61S"), null, DateTimeZone.forID("America/New_York")); + long bucket2 = pg2.bucketStart(time); + Assert.assertEquals(bucket2, bucket); + } } + From 91781938b7ca380ff644cf73b32943aa94e91711 Mon Sep 17 00:00:00 2001 From: Benjamin Hopp Date: Mon, 27 Apr 2026 16:51:45 -0500 Subject: [PATCH 3/4] fix: resolve forbidden api usage in test --- .../util/common/granularity/PeriodGranularityBugTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/processing/src/test/java/org/apache/druid/java/util/common/granularity/PeriodGranularityBugTest.java b/processing/src/test/java/org/apache/druid/java/util/common/granularity/PeriodGranularityBugTest.java index 97f39f3a755a..e7d19fe3b931 100644 --- a/processing/src/test/java/org/apache/druid/java/util/common/granularity/PeriodGranularityBugTest.java +++ b/processing/src/test/java/org/apache/druid/java/util/common/granularity/PeriodGranularityBugTest.java @@ -19,7 +19,7 @@ package org.apache.druid.java.util.common.granularity; -import org.joda.time.DateTimeZone; +import org.apache.druid.java.util.common.DateTimes; import org.joda.time.Period; import org.junit.Assert; import org.junit.Test; @@ -30,7 +30,7 @@ public class PeriodGranularityBugTest public void testCompoundPeriodWithTimeZone() { // America/New_York has DST, so days are imprecise - PeriodGranularity pg = new PeriodGranularity(new Period("PT1M1S"), null, DateTimeZone.forID("America/New_York")); + PeriodGranularity pg = new PeriodGranularity(new Period("PT1M1S"), null, DateTimes.inferTzFromString("America/New_York")); long time = 1704067200000L; // 2024-01-01T00:00:00Z long start = System.currentTimeMillis(); long bucket = pg.bucketStart(time); @@ -39,7 +39,7 @@ public void testCompoundPeriodWithTimeZone() Assert.assertTrue("Should run quickly", (end - start) < 1000); // Let's also check if it returns the same as a non-compound period of same duration (PT61S) - PeriodGranularity pg2 = new PeriodGranularity(new Period("PT61S"), null, DateTimeZone.forID("America/New_York")); + PeriodGranularity pg2 = new PeriodGranularity(new Period("PT61S"), null, DateTimes.inferTzFromString("America/New_York")); long bucket2 = pg2.bucketStart(time); Assert.assertEquals(bucket2, bucket); } From c9c3beb1cf281ab30c3e51d571ec8eb578b4176f Mon Sep 17 00:00:00 2001 From: Benjamin Hopp Date: Mon, 27 Apr 2026 17:23:23 -0500 Subject: [PATCH 4/4] fix: simplify truncateMillisPeriod condition to avoid CodeQL warning --- .../common/granularity/PeriodGranularity.java | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java b/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java index f6a524e02a2d..0900050638b1 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java @@ -593,17 +593,10 @@ private long truncateMillisPeriod(final long t) { // toStandardDuration assumes days are always 24h, and hours are always 60 minutes, // which may not always be the case, e.g if there are daylight saving changes. - if (chronology.days().isPrecise() && chronology.hours().isPrecise()) { - final long millis = period.toStandardDuration().getMillis(); - long offset = t % millis - origin % millis; - if (offset < 0) { - offset += millis; - } - return t - offset; - } else if (period.getYears() == 0 && period.getMonths() == 0 && period.getWeeks() == 0 && period.getDays() == 0) { - // If the period only contains hours, minutes, seconds, and millis, - // their duration is always precise even across daylight saving transitions. - // Therefore, we can safely convert the period to milliseconds. + // However, if the period only contains hours, minutes, seconds, and millis, + // their duration is always precise even across daylight saving transitions. + if ((chronology.days().isPrecise() && chronology.hours().isPrecise()) + || (period.getYears() == 0 && period.getMonths() == 0 && period.getWeeks() == 0 && period.getDays() == 0)) { final long millis = period.toStandardDuration().getMillis(); long offset = t % millis - origin % millis; if (offset < 0) {