shangxinli commented on code in PR #285:
URL: https://github.com/apache/iceberg-cpp/pull/285#discussion_r2484223682
##########
src/iceberg/test/transform_test.cc:
##########
@@ -134,7 +134,7 @@ TEST(TransformResultTypeTest, PositiveCases) {
.expected_result_type = iceberg::int32()},
{.str = "day",
.source_type = iceberg::timestamp(),
- .expected_result_type = iceberg::int32()},
+ .expected_result_type = iceberg::date()},
Review Comment:
Adding tests for Wire Format Compatibility would be better. The PR changes
the return type from int32() to date() but doesn't verify:
1. That date() values serialize to int32 on the wire
2. That existing partitioned data files can still be read
3. That this change is backward compatible with tables partitioned using
the old int32 type
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
