Skip to content

Commit e19903a

Browse files
eshishkiclaude
andcommitted
[BugFix] Drop Iceberg timestamptz file-level min/max across DST fall-back
The Iceberg datetime min/max aggregate optimization converts file-level TIMESTAMP WITH TIME ZONE bounds (UTC micros) to session-local micros, one endpoint at a time. UTC->local is non-monotonic across a backward UTC-offset change (a DST fall-back or a historical rule change): the true local min/max can lie strictly inside the file, so the endpoint conversion produces an inexact bound and SELECT MIN/MAX(ts_tz) can return a wrong result. Skip emitting file-level min/max for a timestamptz column when the file's [low, high] instant range contains an offset-decreasing transition; BE then reads that file directly (a per-file fallback). Constant-offset and spring-forward ranges keep the optimization with exact endpoints. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Evgeniy Shishkin <eshishki@gmail.com>
1 parent 43e1d73 commit e19903a

2 files changed

Lines changed: 167 additions & 8 deletions

File tree

fe/fe-core/src/main/java/com/starrocks/connector/iceberg/IcebergUtil.java

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@
4444
import java.time.Instant;
4545
import java.time.ZoneId;
4646
import java.time.ZoneOffset;
47+
import java.time.zone.ZoneOffsetTransition;
48+
import java.time.zone.ZoneRules;
4749
import java.util.HashMap;
4850
import java.util.List;
4951
import java.util.Map;
@@ -191,15 +193,24 @@ public static Map<Integer, MinMaxValue> parseMinMaxValueBySlots(Schema schema,
191193
Types.TimestampType timestampType = (Types.TimestampType) type;
192194
if (timestampType.shouldAdjustToUTC() && low instanceof Long && high instanceof Long) {
193195
// Iceberg TIMESTAMP WITH TIME ZONE stores instants in UTC, while StarRocks compares DATETIME
194-
// values in the session timezone, so we convert file-level bounds into session-local micros
195-
// before sending them to BE.
196+
// values in the session timezone, so file-level bounds are converted into session-local micros
197+
// before being sent to BE.
196198
//
197-
// Note that this is an endpoint conversion on file-level min/max only. For timezones whose
198-
// UTC offset changes over time (for example DST or historical rule changes), UTC->local is
199-
// not strictly monotonic over an arbitrary interval, so the converted low/high remain a
200-
// best-effort approximation rather than an exact local min/max for the full file.
201-
minMaxValue.minValue = adjustTimestampMicrosToSessionTz((Long) low);
202-
minMaxValue.maxValue = adjustTimestampMicrosToSessionTz((Long) high);
199+
// This is an endpoint-only conversion: it maps just the file's min and max instants. It is exact
200+
// only while UTC->local is monotonic across the file's [low, high] range, i.e. local = utc + offset
201+
// never folds back on itself. A spring-forward transition (offset jumps up, e.g. -08:00 -> -07:00)
202+
// keeps local time strictly increasing, so the endpoints stay the true local min/max. A fall-back
203+
// transition (offset drops, e.g. -07:00 -> -08:00) rewinds local time, so the real min/max can sit
204+
// strictly inside the file and is unrecoverable from the endpoints alone. Only that case is
205+
// rejected: the file-level min/max is dropped and BE reads this file directly (a per-file
206+
// fallback) instead of trusting an inexact bound.
207+
ZoneId zoneId = TimeUtils.getTimeZone().toZoneId();
208+
if (isEndpointConversionExact(zoneId, (Long) low, (Long) high)) {
209+
minMaxValue.minValue = adjustTimestampMicrosToSessionTz((Long) low);
210+
minMaxValue.maxValue = adjustTimestampMicrosToSessionTz((Long) high);
211+
} else {
212+
minMaxValues.remove(field.fieldId());
213+
}
203214
}
204215
}
205216
}
@@ -215,6 +226,34 @@ private static long adjustTimestampMicrosToSessionTz(long micros) {
215226
return micros + offset.getTotalSeconds() * 1_000_000L;
216227
}
217228

229+
/**
230+
* Returns true when the endpoint UTC-&gt;local conversion in {@link #adjustTimestampMicrosToSessionTz(long)}
231+
* produces the exact local min/max for a file covering the instant range {@code [lowMicros, highMicros]} in
232+
* {@code zoneId}.
233+
*
234+
* <p>{@code local = utc + offset(utc)} preserves order as long as the offset never decreases across the range, so
235+
* the file's earliest/latest instants stay its earliest/latest local times. A spring-forward jump (offset
236+
* increases) only widens the gap and keeps that order; a fall-back jump (offset decreases) rewinds local time and
237+
* can move the true min/max onto an interior row that the endpoints no longer represent. We therefore reject the
238+
* range only if it contains an offset-decreasing transition; constant-offset and spring-forward ranges stay exact.
239+
*/
240+
private static boolean isEndpointConversionExact(ZoneId zoneId, long lowMicros, long highMicros) {
241+
Instant low = Instant.ofEpochSecond(Math.floorDiv(lowMicros, 1_000_000L));
242+
Instant high = Instant.ofEpochSecond(Math.floorDiv(highMicros, 1_000_000L));
243+
ZoneRules rules = zoneId.getRules();
244+
// nextTransition returns the first transition strictly after its argument; zone transitions never land on
245+
// sub-second boundaries, so truncating micros to whole seconds cannot skip one. Walk every transition in
246+
// (low, high] and reject as soon as one decreases the UTC offset (a fall-back).
247+
for (ZoneOffsetTransition t = rules.nextTransition(low);
248+
t != null && !t.getInstant().isAfter(high);
249+
t = rules.nextTransition(t.getInstant())) {
250+
if (t.getOffsetAfter().getTotalSeconds() < t.getOffsetBefore().getTotalSeconds()) {
251+
return false;
252+
}
253+
}
254+
return true;
255+
}
256+
218257
public static Map<Integer, TExprMinMaxValue> toThriftMinMaxValueBySlots(Schema schema,
219258
Map<Integer, ByteBuffer> lowerBounds,
220259
Map<Integer, ByteBuffer> upperBounds,

fe/fe-core/src/test/java/com/starrocks/connector/iceberg/IcebergUtilTest.java

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,17 @@
3232
import org.junit.jupiter.api.Test;
3333

3434
import java.nio.ByteBuffer;
35+
import java.time.Instant;
3536
import java.util.HashMap;
3637
import java.util.List;
3738
import java.util.Map;
3839
import java.util.TimeZone;
3940

4041
import static org.apache.iceberg.types.Types.NestedField.required;
4142
import static org.junit.jupiter.api.Assertions.assertEquals;
43+
import static org.junit.jupiter.api.Assertions.assertFalse;
4244
import static org.junit.jupiter.api.Assertions.assertThrows;
45+
import static org.junit.jupiter.api.Assertions.assertTrue;
4346

4447
public class IcebergUtilTest {
4548

@@ -258,6 +261,123 @@ public void testParseMinMaxValueBySlotsWithIcebergTimestampTypes() {
258261
}
259262
}
260263

264+
@Test
265+
public void testTimestamptzMinMaxDroppedWhenFileSpansDstTransition() {
266+
// America/Los_Angeles DST fall-back at 2023-11-05T09:00:00Z: PDT (-07:00) drops to PST (-08:00). A file whose
267+
// timestamptz values straddle that instant covers a backward (offset-decreasing) transition, so UTC->local
268+
// rewinds and the endpoint conversion is no longer exact; the file-level min/max must be dropped.
269+
Schema schema = new Schema(
270+
required(3, "ts_ntz", Types.TimestampType.withoutZone()),
271+
required(5, "ts_tz", Types.TimestampType.withZone()));
272+
List<SlotDescriptor> slots = List.of(
273+
new SlotDescriptor(new SlotId(3), "ts_ntz", DateType.DATETIME, true),
274+
new SlotDescriptor(new SlotId(5), "ts_tz", DateType.DATETIME, true));
275+
slots.get(0).setColumn(new Column("ts_ntz", DateType.DATETIME, true));
276+
slots.get(1).setColumn(new Column("ts_tz", DateType.DATETIME, true));
277+
278+
long lowMicros = Instant.parse("2023-11-05T08:30:00Z").getEpochSecond() * 1_000_000L;
279+
long highMicros = Instant.parse("2023-11-05T09:30:00Z").getEpochSecond() * 1_000_000L;
280+
Map<Integer, ByteBuffer> lowerBounds = Map.of(
281+
3, org.apache.iceberg.types.Conversions.toByteBuffer(Types.TimestampType.withoutZone(), lowMicros),
282+
5, org.apache.iceberg.types.Conversions.toByteBuffer(Types.TimestampType.withZone(), lowMicros));
283+
Map<Integer, ByteBuffer> upperBounds = Map.of(
284+
3, org.apache.iceberg.types.Conversions.toByteBuffer(Types.TimestampType.withoutZone(), highMicros),
285+
5, org.apache.iceberg.types.Conversions.toByteBuffer(Types.TimestampType.withZone(), highMicros));
286+
Map<Integer, Long> nullValueCounts = Map.of(3, 0L, 5, 0L);
287+
Map<Integer, Long> valueCounts = Map.of(3, 2L, 5, 2L);
288+
289+
ConnectContext ctx = new ConnectContext();
290+
ctx.getSessionVariable().setTimeZone("America/Los_Angeles");
291+
ctx.setThreadLocalInfo();
292+
try {
293+
Map<Integer, IcebergUtil.MinMaxValue> result = IcebergUtil.parseMinMaxValueBySlots(
294+
schema, lowerBounds, upperBounds, nullValueCounts, valueCounts, slots);
295+
// timestamp without time zone is unaffected and keeps its raw wall-clock micros.
296+
assertTrue(result.containsKey(3));
297+
assertEquals(lowMicros, result.get(3).minValue);
298+
// timestamptz spanning the DST transition is dropped, so BE falls back to reading the file.
299+
assertFalse(result.containsKey(5));
300+
301+
Map<Integer, TExprMinMaxValue> thriftValues = IcebergUtil.toThriftMinMaxValueBySlots(
302+
schema, lowerBounds, upperBounds, nullValueCounts, valueCounts, slots);
303+
assertTrue(thriftValues.containsKey(3));
304+
assertFalse(thriftValues.containsKey(5));
305+
} finally {
306+
ConnectContext.remove();
307+
}
308+
}
309+
310+
@Test
311+
public void testTimestamptzMinMaxKeptWhenFileWithinSingleOffset() {
312+
// A file whose timestamptz values stay within one offset period (here PDT, summer 2023) has no offset
313+
// change inside its range, so endpoint UTC->local conversion is exact and the bounds are converted and kept.
314+
Schema schema = new Schema(required(5, "ts_tz", Types.TimestampType.withZone()));
315+
List<SlotDescriptor> slots = List.of(
316+
new SlotDescriptor(new SlotId(5), "ts_tz", DateType.DATETIME, true));
317+
slots.get(0).setColumn(new Column("ts_tz", DateType.DATETIME, true));
318+
319+
long lowMicros = Instant.parse("2023-07-01T10:00:00Z").getEpochSecond() * 1_000_000L;
320+
long highMicros = Instant.parse("2023-07-01T11:00:00Z").getEpochSecond() * 1_000_000L;
321+
Map<Integer, ByteBuffer> lowerBounds = Map.of(
322+
5, org.apache.iceberg.types.Conversions.toByteBuffer(Types.TimestampType.withZone(), lowMicros));
323+
Map<Integer, ByteBuffer> upperBounds = Map.of(
324+
5, org.apache.iceberg.types.Conversions.toByteBuffer(Types.TimestampType.withZone(), highMicros));
325+
Map<Integer, Long> nullValueCounts = Map.of(5, 0L);
326+
Map<Integer, Long> valueCounts = Map.of(5, 2L);
327+
328+
ConnectContext ctx = new ConnectContext();
329+
ctx.getSessionVariable().setTimeZone("America/Los_Angeles");
330+
ctx.setThreadLocalInfo();
331+
try {
332+
Map<Integer, IcebergUtil.MinMaxValue> result = IcebergUtil.parseMinMaxValueBySlots(
333+
schema, lowerBounds, upperBounds, nullValueCounts, valueCounts, slots);
334+
assertTrue(result.containsKey(5));
335+
// PDT offset is -07:00 across the whole range; endpoint conversion adds that offset to each bound.
336+
assertEquals(lowMicros + TimeZone.getTimeZone("America/Los_Angeles").getOffset(
337+
Instant.parse("2023-07-01T10:00:00Z").toEpochMilli()) * 1000L, result.get(5).minValue);
338+
assertEquals(highMicros + TimeZone.getTimeZone("America/Los_Angeles").getOffset(
339+
Instant.parse("2023-07-01T11:00:00Z").toEpochMilli()) * 1000L, result.get(5).maxValue);
340+
} finally {
341+
ConnectContext.remove();
342+
}
343+
}
344+
345+
@Test
346+
public void testTimestamptzMinMaxKeptAcrossSpringForward() {
347+
// America/Los_Angeles spring-forward at 2023-03-12T10:00:00Z: PST (-08:00) jumps to PDT (-07:00). The offset
348+
// increases, so UTC->local stays monotonic and the file's endpoints remain its true local min/max even though
349+
// it straddles the transition; the bounds are converted (each with its own instant's offset) and kept.
350+
Schema schema = new Schema(required(5, "ts_tz", Types.TimestampType.withZone()));
351+
List<SlotDescriptor> slots = List.of(
352+
new SlotDescriptor(new SlotId(5), "ts_tz", DateType.DATETIME, true));
353+
slots.get(0).setColumn(new Column("ts_tz", DateType.DATETIME, true));
354+
355+
long lowMicros = Instant.parse("2023-03-12T09:30:00Z").getEpochSecond() * 1_000_000L;
356+
long highMicros = Instant.parse("2023-03-12T10:30:00Z").getEpochSecond() * 1_000_000L;
357+
Map<Integer, ByteBuffer> lowerBounds = Map.of(
358+
5, org.apache.iceberg.types.Conversions.toByteBuffer(Types.TimestampType.withZone(), lowMicros));
359+
Map<Integer, ByteBuffer> upperBounds = Map.of(
360+
5, org.apache.iceberg.types.Conversions.toByteBuffer(Types.TimestampType.withZone(), highMicros));
361+
Map<Integer, Long> nullValueCounts = Map.of(5, 0L);
362+
Map<Integer, Long> valueCounts = Map.of(5, 2L);
363+
364+
ConnectContext ctx = new ConnectContext();
365+
ctx.getSessionVariable().setTimeZone("America/Los_Angeles");
366+
ctx.setThreadLocalInfo();
367+
try {
368+
Map<Integer, IcebergUtil.MinMaxValue> result = IcebergUtil.parseMinMaxValueBySlots(
369+
schema, lowerBounds, upperBounds, nullValueCounts, valueCounts, slots);
370+
assertTrue(result.containsKey(5));
371+
// low uses PST (-08:00), high uses PDT (-07:00); each endpoint converts with its own instant's offset.
372+
assertEquals(lowMicros + TimeZone.getTimeZone("America/Los_Angeles").getOffset(
373+
Instant.parse("2023-03-12T09:30:00Z").toEpochMilli()) * 1000L, result.get(5).minValue);
374+
assertEquals(highMicros + TimeZone.getTimeZone("America/Los_Angeles").getOffset(
375+
Instant.parse("2023-03-12T10:30:00Z").toEpochMilli()) * 1000L, result.get(5).maxValue);
376+
} finally {
377+
ConnectContext.remove();
378+
}
379+
}
380+
261381
private static org.apache.iceberg.Table tableWithProperty(String key, String value) {
262382
org.apache.iceberg.Table table = org.mockito.Mockito.mock(org.apache.iceberg.Table.class);
263383
Map<String, String> props = new HashMap<>();

0 commit comments

Comments
 (0)