Skip to content

Commit fbfae87

Browse files
committed
fix: result type of DayTransform should be date
Result type of Day transform should be `int` per the spec, but in the Java impl, it is `DateType`, this discrepancy has been discussed on devlist, see [1]. iceberg-iceberg and iceberg-rust all conform to the java impl, so make iceberg-cpp do the same. The added comments are borrowed from iceberg-python. [1] https://lists.apache.org/thread/2gq7b54nvc9q6f1j08l9lnzgm5onkmx5
1 parent 5cbf3df commit fbfae87

File tree

4 files changed

+12
-3
lines changed

4 files changed

+12
-3
lines changed

src/iceberg/test/transform_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ TEST(TransformResultTypeTest, PositiveCases) {
134134
.expected_result_type = iceberg::int32()},
135135
{.str = "day",
136136
.source_type = iceberg::timestamp(),
137-
.expected_result_type = iceberg::int32()},
137+
.expected_result_type = iceberg::date()},
138138
{.str = "hour",
139139
.source_type = iceberg::timestamp(),
140140
.expected_result_type = iceberg::int32()},

src/iceberg/transform.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,11 @@ class ICEBERG_EXPORT TransformFunction {
202202
/// \brief Get the source type of transform function
203203
const std::shared_ptr<Type>& source_type() const;
204204
/// \brief Get the result type of transform function
205+
///
206+
/// Note: This method defines both the physical and display representation of the
207+
/// partition field. The physical representation must conform to the Iceberg spec. The
208+
/// display representation can deviate from the spec, such as by transforming the value
209+
/// into a more human-readable format.
205210
virtual std::shared_ptr<Type> ResultType() const = 0;
206211

207212
friend bool operator==(const TransformFunction& lhs, const TransformFunction& rhs) {

src/iceberg/transform_function.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ Result<Literal> DayTransform::Transform(const Literal& literal) {
193193
return TemporalUtils::ExtractDay(literal);
194194
}
195195

196-
std::shared_ptr<Type> DayTransform::ResultType() const { return int32(); }
196+
std::shared_ptr<Type> DayTransform::ResultType() const { return date(); }
197197

198198
Result<std::unique_ptr<TransformFunction>> DayTransform::Make(
199199
std::shared_ptr<Type> const& source_type) {

src/iceberg/transform_function.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,11 @@ class ICEBERG_EXPORT DayTransform : public TransformFunction {
141141
/// \brief Extract a date or timestamp day, as days from 1970-01-01.
142142
Result<Literal> Transform(const Literal& literal) override;
143143

144-
/// \brief Returns INT32 as the output type.
144+
/// \brief Return the result type of a day transform.
145+
///
146+
/// Note: The physical representation conforms to the Iceberg spec as DateType is
147+
/// internally converted to int. The DateType returned here provides a more
148+
/// human-readable way to display the partition field.
145149
std::shared_ptr<Type> ResultType() const override;
146150

147151
/// \brief Create a DayTransform.

0 commit comments

Comments
 (0)