Skip to content

Commit 76fa723

Browse files
zhjwpkuzhjwpku
authored
refactor: move temporal utilities out of transform util (#675)
Also add internal math helpers for floor division and checked multiplication. Keep the Human* formatting helpers in TransformUtil to stay consistent with the Java TransformUtil implementation.
1 parent a8578e1 commit 76fa723

16 files changed

Lines changed: 708 additions & 650 deletions

‎src/iceberg/expression/json_serde.cc‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include "iceberg/util/json_util_internal.h"
3434
#include "iceberg/util/macros.h"
3535
#include "iceberg/util/string_util.h"
36+
#include "iceberg/util/temporal_util.h"
3637
#include "iceberg/util/transform_util.h"
3738

3839
namespace iceberg {
@@ -363,7 +364,7 @@ Result<Literal> LiteralFromJson(const nlohmann::json& json, const Type* type) {
363364
return JsonParseError("Cannot parse {} as a date value", SafeDumpJson(json));
364365
}
365366
ICEBERG_ASSIGN_OR_RAISE(auto days,
366-
TransformUtil::ParseDay(json.get<std::string>()));
367+
TemporalUtils::ParseDay(json.get<std::string>()));
367368
return Literal::Date(days);
368369
}
369370

@@ -372,7 +373,7 @@ Result<Literal> LiteralFromJson(const nlohmann::json& json, const Type* type) {
372373
return JsonParseError("Cannot parse {} as a time value", SafeDumpJson(json));
373374
}
374375
ICEBERG_ASSIGN_OR_RAISE(auto micros,
375-
TransformUtil::ParseTime(json.get<std::string>()));
376+
TemporalUtils::ParseTime(json.get<std::string>()));
376377
return Literal::Time(micros);
377378
}
378379

@@ -381,7 +382,7 @@ Result<Literal> LiteralFromJson(const nlohmann::json& json, const Type* type) {
381382
return JsonParseError("Cannot parse {} as a timestamp value", SafeDumpJson(json));
382383
}
383384
ICEBERG_ASSIGN_OR_RAISE(auto micros,
384-
TransformUtil::ParseTimestamp(json.get<std::string>()));
385+
TemporalUtils::ParseTimestamp(json.get<std::string>()));
385386
return Literal::Timestamp(micros);
386387
}
387388

@@ -391,7 +392,7 @@ Result<Literal> LiteralFromJson(const nlohmann::json& json, const Type* type) {
391392
SafeDumpJson(json));
392393
}
393394
ICEBERG_ASSIGN_OR_RAISE(
394-
auto micros, TransformUtil::ParseTimestampWithZone(json.get<std::string>()));
395+
auto micros, TemporalUtils::ParseTimestampWithZone(json.get<std::string>()));
395396
return Literal::TimestampTz(micros);
396397
}
397398

@@ -401,7 +402,7 @@ Result<Literal> LiteralFromJson(const nlohmann::json& json, const Type* type) {
401402
SafeDumpJson(json));
402403
}
403404
ICEBERG_ASSIGN_OR_RAISE(auto nanos,
404-
TransformUtil::ParseTimestampNs(json.get<std::string>()));
405+
TemporalUtils::ParseTimestampNs(json.get<std::string>()));
405406
return Literal::TimestampNs(nanos);
406407
}
407408

@@ -411,7 +412,7 @@ Result<Literal> LiteralFromJson(const nlohmann::json& json, const Type* type) {
411412
SafeDumpJson(json));
412413
}
413414
ICEBERG_ASSIGN_OR_RAISE(
414-
auto nanos, TransformUtil::ParseTimestampNsWithZone(json.get<std::string>()));
415+
auto nanos, TemporalUtils::ParseTimestampNsWithZone(json.get<std::string>()));
415416
return Literal::TimestampTzNs(nanos);
416417
}
417418

‎src/iceberg/expression/literal.cc‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
#include "iceberg/util/macros.h"
3333
#include "iceberg/util/string_util.h"
3434
#include "iceberg/util/temporal_util.h"
35-
#include "iceberg/util/transform_util.h"
3635

3736
namespace iceberg {
3837

@@ -203,29 +202,29 @@ Result<Literal> LiteralCaster::CastFromString(
203202
return Literal::UUID(uuid);
204203
}
205204
case TypeId::kDate: {
206-
ICEBERG_ASSIGN_OR_RAISE(auto days, TransformUtil::ParseDay(str_val));
205+
ICEBERG_ASSIGN_OR_RAISE(auto days, TemporalUtils::ParseDay(str_val));
207206
return Literal::Date(days);
208207
}
209208
case TypeId::kTime: {
210-
ICEBERG_ASSIGN_OR_RAISE(auto micros, TransformUtil::ParseTime(str_val));
209+
ICEBERG_ASSIGN_OR_RAISE(auto micros, TemporalUtils::ParseTime(str_val));
211210
return Literal::Time(micros);
212211
}
213212
case TypeId::kTimestamp: {
214-
ICEBERG_ASSIGN_OR_RAISE(auto micros, TransformUtil::ParseTimestamp(str_val));
213+
ICEBERG_ASSIGN_OR_RAISE(auto micros, TemporalUtils::ParseTimestamp(str_val));
215214
return Literal::Timestamp(micros);
216215
}
217216
case TypeId::kTimestampTz: {
218217
ICEBERG_ASSIGN_OR_RAISE(auto micros,
219-
TransformUtil::ParseTimestampWithZone(str_val));
218+
TemporalUtils::ParseTimestampWithZone(str_val));
220219
return Literal::TimestampTz(micros);
221220
}
222221
case TypeId::kTimestampNs: {
223-
ICEBERG_ASSIGN_OR_RAISE(auto nanos, TransformUtil::ParseTimestampNs(str_val));
222+
ICEBERG_ASSIGN_OR_RAISE(auto nanos, TemporalUtils::ParseTimestampNs(str_val));
224223
return Literal::TimestampNs(nanos);
225224
}
226225
case TypeId::kTimestampTzNs: {
227226
ICEBERG_ASSIGN_OR_RAISE(auto nanos,
228-
TransformUtil::ParseTimestampNsWithZone(str_val));
227+
TemporalUtils::ParseTimestampNsWithZone(str_val));
229228
return Literal::TimestampTzNs(nanos);
230229
}
231230
case TypeId::kBinary: {

‎src/iceberg/test/CMakeLists.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,13 @@ add_iceberg_test(util_test
128128
formatter_test.cc
129129
lazy_test.cc
130130
location_util_test.cc
131+
math_util_internal_test.cc
131132
roaring_position_bitmap_test.cc
132133
position_delete_index_test.cc
133134
retry_util_test.cc
134135
string_util_test.cc
135136
struct_like_set_test.cc
137+
temporal_util_test.cc
136138
transform_util_test.cc
137139
truncate_util_test.cc
138140
url_encoder_test.cc

‎src/iceberg/test/inclusive_metrics_evaluator_with_transform_test.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "iceberg/schema.h"
3131
#include "iceberg/test/matchers.h"
3232
#include "iceberg/type.h"
33+
#include "iceberg/util/temporal_util.h"
3334

3435
namespace iceberg {
3536

@@ -38,9 +39,8 @@ constexpr bool kRowsMightMatch = true;
3839
constexpr bool kRowCannotMatch = false;
3940
constexpr int64_t kIntMinValue = 30;
4041
constexpr int64_t kIntMaxValue = 79;
41-
constexpr int64_t kMicrosPerDay = 86'400'000'000LL;
42-
constexpr int64_t kTsMinValue = 30 * kMicrosPerDay;
43-
constexpr int64_t kTsMaxValue = 79 * kMicrosPerDay;
42+
constexpr int64_t kTsMinValue = 30 * internal::kMicrosPerDay;
43+
constexpr int64_t kTsMaxValue = 79 * internal::kMicrosPerDay;
4444

4545
std::shared_ptr<UnboundTerm<BoundTransform>> ToBoundTransform(
4646
const std::shared_ptr<UnboundTransform>& transform) {

‎src/iceberg/test/literal_test.cc‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "iceberg/test/matchers.h"
3030
#include "iceberg/test/temporal_test_helper.h"
3131
#include "iceberg/type.h"
32+
#include "iceberg/util/temporal_util.h"
3233

3334
namespace iceberg {
3435

@@ -678,10 +679,11 @@ INSTANTIATE_TEST_SUITE_P(
678679
.small_literal = Literal::Date(100),
679680
.large_literal = Literal::Date(200),
680681
.equal_literal = Literal::Date(100)},
681-
ComparisonLiteralTestParam{.test_name = "Time",
682-
.small_literal = Literal::Time(43200000000LL),
683-
.large_literal = Literal::Time(86400000000LL),
684-
.equal_literal = Literal::Time(43200000000LL)},
682+
ComparisonLiteralTestParam{
683+
.test_name = "Time",
684+
.small_literal = Literal::Time(internal::kMicrosPerDay / 2),
685+
.large_literal = Literal::Time(internal::kMicrosPerDay),
686+
.equal_literal = Literal::Time(internal::kMicrosPerDay / 2)},
685687
ComparisonLiteralTestParam{.test_name = "Timestamp",
686688
.small_literal = Literal::Timestamp(1000000LL),
687689
.large_literal = Literal::Timestamp(2000000LL),

‎src/iceberg/test/math_util_internal_test.cc‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#include "iceberg/util/math_util_internal.h"
21+
22+
#include <limits>
23+
24+
#include <gtest/gtest.h>
25+
26+
#include "iceberg/test/matchers.h"
27+
28+
namespace iceberg {
29+
30+
TEST(MathUtilInternalTest, FloorDiv) {
31+
EXPECT_EQ(0, FloorDiv(0, 1000));
32+
EXPECT_EQ(1, FloorDiv(1001, 1000));
33+
EXPECT_EQ(-1, FloorDiv(-1, 1000));
34+
EXPECT_EQ(-2, FloorDiv(-1001, 1000));
35+
EXPECT_EQ(1, FloorDiv(-1001, -1000));
36+
EXPECT_EQ(-2, FloorDiv(1001, -1000));
37+
}
38+
39+
TEST(MathUtilInternalTest, MultiplyExact) {
40+
ICEBERG_UNWRAP_OR_FAIL(auto positive, MultiplyExact(1000, 1000));
41+
EXPECT_EQ(1000000, positive);
42+
43+
ICEBERG_UNWRAP_OR_FAIL(auto negative, MultiplyExact(-1000, 1000));
44+
EXPECT_EQ(-1000000, negative);
45+
46+
ICEBERG_UNWRAP_OR_FAIL(auto min_value,
47+
MultiplyExact(std::numeric_limits<int64_t>::min(), 1));
48+
EXPECT_EQ(std::numeric_limits<int64_t>::min(), min_value);
49+
50+
EXPECT_THAT(MultiplyExact(std::numeric_limits<int64_t>::max(), 2),
51+
IsError(ErrorKind::kInvalidArgument));
52+
EXPECT_THAT(MultiplyExact(std::numeric_limits<int64_t>::min(), -1),
53+
IsError(ErrorKind::kInvalidArgument));
54+
}
55+
56+
} // namespace iceberg

‎src/iceberg/test/meson.build‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,13 @@ iceberg_tests = {
9292
'formatter_test.cc',
9393
'lazy_test.cc',
9494
'location_util_test.cc',
95+
'math_util_internal_test.cc',
9596
'position_delete_index_test.cc',
9697
'retry_util_test.cc',
9798
'roaring_position_bitmap_test.cc',
9899
'string_util_test.cc',
99100
'struct_like_set_test.cc',
101+
'temporal_util_test.cc',
100102
'transform_util_test.cc',
101103
'truncate_util_test.cc',
102104
'url_encoder_test.cc',

‎src/iceberg/test/temporal_test_helper.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
#include <chrono>
2323
#include <cstdint>
2424

25+
#include "iceberg/util/temporal_util.h"
26+
2527
namespace iceberg {
2628

2729
using namespace std::chrono; // NOLINT
@@ -64,13 +66,12 @@ struct TimestampNanosParts {
6466
};
6567

6668
class TemporalTestHelper {
67-
static constexpr auto kEpochDays = sys_days(year{1970} / January / 1);
68-
6969
public:
7070
/// \brief Construct a Calendar date without timezone or time
7171
static int32_t CreateDate(const DateParts& parts) {
7272
return static_cast<int32_t>(
73-
(sys_days(year{parts.year} / month{parts.month} / day{parts.day}) - kEpochDays)
73+
(sys_days(year{parts.year} / month{parts.month} / day{parts.day}) -
74+
internal::kEpochDays)
7475
.count());
7576
}
7677

0 commit comments

Comments
 (0)