Skip to content

Commit 833fbdf

Browse files
committed
[fix](be) guard timestamp pruning across DST rollback
Disable unsafe ORC timestamp SARG pushdown in civil years with backward timezone transitions, while preserving Parquet min/max pruning for TIMESTAMPTZ values that retain UTC ordering. Add ORC, Parquet, and timezone transition regression coverage.
1 parent c66a4ca commit 833fbdf

6 files changed

Lines changed: 161 additions & 7 deletions

File tree

be/src/format_v2/orc/orc_search_argument.cpp

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
#include "exprs/vliteral.h"
4646
#include "exprs/vslot_ref.h"
4747
#include "exprs/vtopn_pred.h"
48+
#include "format_v2/timestamp_statistics.h"
4849

4950
namespace doris::format::orc {
5051
namespace {
@@ -597,20 +598,47 @@ std::optional<DateTimeLiteralParts> date_time_literal_parts(const Field& field)
597598

598599
std::optional<::orc::Literal> make_timestamp_literal(const Field& field,
599600
const cctz::time_zone& timezone) {
601+
const auto civil_year_is_monotonic = [&](const cctz::civil_second& civil_seconds) {
602+
// Localized file predicates may already have selected one side of a repeated civil time.
603+
// Checking the whole civil year catches that lossy representation while retaining SARG
604+
// pruning for years and zones without a backward transition.
605+
const auto year_start =
606+
cctz::convert(cctz::civil_second(civil_seconds.year(), 1, 1), timezone);
607+
const auto next_year_start =
608+
cctz::convert(cctz::civil_second(civil_seconds.year() + 1, 1, 1), timezone);
609+
return format::utc_timestamp_range_is_monotonic(year_start.time_since_epoch().count(),
610+
next_year_start.time_since_epoch().count(),
611+
timezone);
612+
};
600613
switch (field.get_type()) {
601614
case TYPE_DATETIME: {
602615
const auto& datetime = field.get<TYPE_DATETIME>();
603616
const cctz::civil_second civil_seconds(datetime.year(), datetime.month(), datetime.day(),
604617
datetime.hour(), datetime.minute(),
605618
datetime.second());
606-
return ::orc::Literal(cctz::convert(civil_seconds, timezone).time_since_epoch().count(), 0);
619+
const auto lookup = timezone.lookup(civil_seconds);
620+
// ORC SearchArguments accept one UTC timestamp literal. A repeated or skipped local civil
621+
// time has no unique UTC representation, so pushing it down could prune a stripe that
622+
// contains another valid interpretation. Keep such predicates for row-level evaluation.
623+
if (!civil_year_is_monotonic(civil_seconds) ||
624+
lookup.kind != cctz::time_zone::civil_lookup::UNIQUE) {
625+
return std::nullopt;
626+
}
627+
return ::orc::Literal(lookup.pre.time_since_epoch().count(), 0);
607628
}
608629
case TYPE_DATETIMEV2: {
609630
const auto& datetime = field.get<TYPE_DATETIMEV2>();
610631
const cctz::civil_second civil_seconds(datetime.year(), datetime.month(), datetime.day(),
611632
datetime.hour(), datetime.minute(),
612633
datetime.second());
613-
const auto seconds = cctz::convert(civil_seconds, timezone).time_since_epoch().count();
634+
const auto lookup = timezone.lookup(civil_seconds);
635+
// See the DATETIME path above. The fractional part does not disambiguate a civil time
636+
// repeated by a backward timezone transition.
637+
if (!civil_year_is_monotonic(civil_seconds) ||
638+
lookup.kind != cctz::time_zone::civil_lookup::UNIQUE) {
639+
return std::nullopt;
640+
}
641+
const auto seconds = lookup.pre.time_since_epoch().count();
614642
const auto nanos = cast_set<int32_t>(datetime.microsecond() * 1000);
615643
return ::orc::Literal(seconds, nanos);
616644
}

be/src/format_v2/parquet/parquet_statistics.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,10 @@ int64_t floor_timestamp_seconds(int64_t value, ParquetTimeUnit time_unit) {
141141
bool timestamp_min_max_is_safe(const ParquetColumnSchema& column_schema, int64_t min_value,
142142
int64_t max_value, const cctz::time_zone* timezone) {
143143
if (!column_schema.type_descriptor.is_timestamp ||
144-
!column_schema.type_descriptor.timestamp_is_adjusted_to_utc || timezone == nullptr) {
144+
!column_schema.type_descriptor.timestamp_is_adjusted_to_utc || timezone == nullptr ||
145+
remove_nullable(column_schema.type)->get_primitive_type() == TYPE_TIMESTAMPTZ) {
146+
// TIMESTAMPTZ keeps the original UTC ordering, so local civil-time rollback does not make
147+
// its converted min/max non-monotonic.
145148
return true;
146149
}
147150
return format::utc_timestamp_range_is_monotonic(

be/src/format_v2/timestamp_statistics.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,10 @@ inline bool utc_timestamp_range_is_monotonic(int64_t min_seconds, int64_t max_se
5151
if (transition.to < transition.from) {
5252
return false;
5353
}
54-
current = transition_time;
54+
// Move past the transition that was just inspected. Some cctz implementations return the
55+
// same transition again when queried at its exact instant, which would otherwise prevent
56+
// us from seeing a later rollback in the requested range.
57+
current = transition_time + cctz::seconds(1);
5558
}
5659
return true;
5760
}

be/test/format_v2/orc/orc_reader_test.cpp

Lines changed: 92 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
#include "runtime/runtime_state.h"
7575
#include "storage/segment/condition_cache.h"
7676
#include "storage/utils.h"
77+
#include "util/timezone_utils.h"
7778

7879
namespace doris {
7980
namespace {
@@ -2455,6 +2456,49 @@ class NullableGreaterThanExpr final : public VExpr {
24552456
const std::string _expr_name;
24562457
};
24572458

2459+
template <PrimitiveType Primitive>
2460+
class NullableEqualsExpr final : public VExpr {
2461+
public:
2462+
using ColumnType = typename PrimitiveTypeTraits<Primitive>::ColumnType;
2463+
using ValueType = typename PrimitiveTypeTraits<Primitive>::CppType;
2464+
2465+
NullableEqualsExpr(int column_id, DataTypePtr type, const Field& value, std::string column_name)
2466+
: VExpr(std::make_shared<DataTypeUInt8>(), false),
2467+
_column_id(column_id),
2468+
_value(value.get<Primitive>()),
2469+
_expr_name("NullableEqualsExpr") {
2470+
_node_type = TExprNodeType::BINARY_PRED;
2471+
_opcode = TExprOpcode::EQ;
2472+
add_child(TableSlotRef::create_shared(column_id, column_id, -1, make_nullable(type),
2473+
std::move(column_name)));
2474+
add_child(TableLiteral::create_shared(std::move(type), value));
2475+
}
2476+
2477+
Status execute_column_impl(VExprContext* context, const Block* block, const Selector* selector,
2478+
size_t count, ColumnPtr& result_column) const override {
2479+
const auto& nullable_column =
2480+
assert_cast<const ColumnNullable&>(*block->get_by_position(_column_id).column);
2481+
const auto& input = assert_cast<const ColumnType&>(nullable_column.get_nested_column());
2482+
auto result = ColumnUInt8::create();
2483+
auto& result_data = result->get_data();
2484+
result_data.resize(count);
2485+
for (size_t row = 0; row < count; ++row) {
2486+
const size_t input_row = selector == nullptr ? row : (*selector)[row];
2487+
result_data[row] = !nullable_column.is_null_at(input_row) &&
2488+
input.get_element(input_row) == _value;
2489+
}
2490+
result_column = std::move(result);
2491+
return Status::OK();
2492+
}
2493+
2494+
const std::string& expr_name() const override { return _expr_name; }
2495+
2496+
private:
2497+
const int _column_id;
2498+
const ValueType _value;
2499+
const std::string _expr_name;
2500+
};
2501+
24582502
template <PrimitiveType Primitive>
24592503
class NullableInExpr final : public VExpr {
24602504
public:
@@ -4036,7 +4080,9 @@ void write_multi_stripe_orc_sarg_types_file(const std::string& file_path) {
40364080
out.write(memory_stream.getData(), static_cast<std::streamsize>(memory_stream.getLength()));
40374081
}
40384082

4039-
void write_multi_stripe_orc_timestamp_instant_sarg_file(const std::string& file_path) {
4083+
void write_multi_stripe_orc_timestamp_instant_sarg_file(
4084+
const std::string& file_path, int64_t first_timestamp_second = 0,
4085+
int64_t second_timestamp_second = 1609459200) {
40404086
auto type = std::unique_ptr<::orc::Type>(::orc::Type::buildTypeFromString(
40414087
"struct<timestamp_instant_col:timestamp with local time zone,payload:string>"));
40424088

@@ -4068,8 +4114,8 @@ void write_multi_stripe_orc_timestamp_instant_sarg_file(const std::string& file_
40684114
writer->add(*batch);
40694115
};
40704116

4071-
add_batch(0);
4072-
add_batch(1609459200);
4117+
add_batch(first_timestamp_second);
4118+
add_batch(second_timestamp_second);
40734119
writer->close();
40744120

40754121
std::ofstream out(file_path, std::ios::binary);
@@ -8327,6 +8373,49 @@ TEST_F(NewOrcReaderTest, SargTimestampInstantConjunctUsesSessionTimezone) {
83278373
EXPECT_EQ(reader->reader_statistics().filtered_group_rows, 200);
83288374
}
83298375

8376+
TEST_F(NewOrcReaderTest, SargTimestampInstantRepeatedCivilTimeDoesNotPruneStripes) {
8377+
const auto multi_stripe_file_path =
8378+
(_test_dir / "sarg_timestamp_instant_dst_rollback.orc").string();
8379+
// These UTC instants both decode to 2021-11-07 01:30:00.123 in America/New_York,
8380+
// once before and once after the UTC-04:00 to UTC-05:00 rollback.
8381+
write_multi_stripe_orc_timestamp_instant_sarg_file(multi_stripe_file_path, 1636263000,
8382+
1636266600);
8383+
ASSERT_EQ(get_orc_stripe_count(multi_stripe_file_path), 2);
8384+
8385+
auto reader = create_reader_for_path(multi_stripe_file_path);
8386+
RuntimeState state {TQueryOptions(), TQueryGlobals()};
8387+
TimezoneUtils::load_timezones_to_cache();
8388+
state.set_timezone("America/New_York");
8389+
ASSERT_TRUE(reader->init(&state).ok());
8390+
8391+
std::vector<format::ColumnDefinition> schema;
8392+
ASSERT_TRUE(reader->get_schema(&schema).ok());
8393+
ASSERT_EQ(schema.size(), 2);
8394+
8395+
const auto literal =
8396+
Field::create_field<TYPE_DATETIMEV2>(make_datetime_v2(2021, 11, 7, 1, 30, 0, 123000));
8397+
auto request = std::make_shared<format::FileScanRequest>();
8398+
request->predicate_columns = {field_projection(0)};
8399+
request->conjuncts.push_back(
8400+
VExprContext::create_shared(std::make_shared<NullableEqualsExpr<TYPE_DATETIMEV2>>(
8401+
0, remove_nullable(schema[0].type), literal, "timestamp_instant_col")));
8402+
ASSERT_TRUE(reader->open(request).ok());
8403+
8404+
bool eof = false;
8405+
size_t result_rows = 0;
8406+
while (!eof) {
8407+
Block block = build_file_block({schema[0]});
8408+
size_t rows = 0;
8409+
ASSERT_TRUE(reader->get_block(&block, &rows, &eof).ok());
8410+
result_rows += rows;
8411+
}
8412+
8413+
EXPECT_EQ(result_rows, 2);
8414+
EXPECT_EQ(reader->reader_statistics().filtered_row_groups, 0);
8415+
EXPECT_EQ(reader->reader_statistics().filtered_row_groups_by_min_max, 0);
8416+
EXPECT_EQ(reader->reader_statistics().filtered_group_rows, 0);
8417+
}
8418+
83308419
TEST_F(NewOrcReaderTest, SargTimestampLowerPrecisionCastDoesNotPruneStripes) {
83318420
const auto multi_stripe_file_path =
83328421
(_test_dir / "sarg_timestamp_lower_precision_cast.orc").string();

be/test/format_v2/parquet/parquet_statistics_test.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636

3737
#include "core/data_type/data_type_number.h"
3838
#include "core/data_type/data_type_string.h"
39+
#include "core/data_type/data_type_timestamptz.h"
3940
#include "core/field.h"
4041
#include "exprs/vexpr.h"
4142
#include "exprs/vexpr_context.h"
@@ -420,6 +421,32 @@ TEST(ParquetStatisticsTransformTest, DisablesUtcTimestampMinMaxAcrossDstRollback
420421
EXPECT_LT(utc_stats.min_value, utc_stats.max_value);
421422
}
422423

424+
TEST(ParquetStatisticsTransformTest, KeepsTimestampTzMinMaxAcrossDstRollback) {
425+
constexpr int64_t MICROS_PER_SECOND = 1000000;
426+
auto table = arrow::Table::Make(
427+
arrow::schema(
428+
{arrow::field("ts", arrow::timestamp(arrow::TimeUnit::MICRO, "UTC"), false)}),
429+
{timestamp_array({1636263000 * MICROS_PER_SECOND, 1636266600 * MICROS_PER_SECOND})});
430+
auto reader = make_reader(table, 2, false, true);
431+
auto schema = build_file_schema(*reader);
432+
auto statistics = reader->metadata()->RowGroup(0)->ColumnChunk(0)->statistics();
433+
434+
// This is the effective type produced by enable_mapping_timestamp_tz. The physical timestamp
435+
// flags intentionally remain adjusted-to-UTC so decoding can preserve the source semantics.
436+
schema[0]->type = std::make_shared<DataTypeTimeStampTz>(6);
437+
schema[0]->type_descriptor.doris_type = schema[0]->type;
438+
439+
cctz::time_zone new_york;
440+
ASSERT_TRUE(cctz::load_time_zone("America/New_York", &new_york));
441+
const auto timestamp_tz_stats =
442+
format::parquet::ParquetStatisticsUtils::TransformColumnStatistics(
443+
*schema[0], statistics, &new_york);
444+
EXPECT_TRUE(timestamp_tz_stats.has_min_max);
445+
EXPECT_EQ(timestamp_tz_stats.min_value.get_type(), TYPE_TIMESTAMPTZ);
446+
EXPECT_EQ(timestamp_tz_stats.max_value.get_type(), TYPE_TIMESTAMPTZ);
447+
EXPECT_LT(timestamp_tz_stats.min_value, timestamp_tz_stats.max_value);
448+
}
449+
423450
TEST(ParquetStatisticsTransformTest, HandlesMissingStatisticsAndAllNullChunks) {
424451
auto no_stats_table = arrow::Table::Make(
425452
arrow::schema({arrow::field("i", arrow::int32(), true)}), {int32_array({1, 2, 3})});

be/test/format_v2/timestamp_statistics_test.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ TEST(TimestampStatisticsTest, DetectsBackwardTimezoneTransitionsInUtcRange) {
3131
EXPECT_FALSE(utc_timestamp_range_is_monotonic(1636263000, 1636264800, new_york));
3232
EXPECT_TRUE(utc_timestamp_range_is_monotonic(1636264800, 1636266600, new_york));
3333

34+
// A range beginning before the spring-forward transition must continue scanning and find the
35+
// later rollback in the same year.
36+
EXPECT_FALSE(utc_timestamp_range_is_monotonic(1609477200, 1641013200, new_york));
37+
3438
// The 2021 spring transition at 07:00 UTC skips civil values but preserves ordering.
3539
EXPECT_TRUE(utc_timestamp_range_is_monotonic(1615703400, 1615707000, new_york));
3640
}

0 commit comments

Comments
 (0)