felipepessoto commented on code in PR #12967:
URL: https://github.com/apache/gluten/pull/12967#discussion_r4077448462
##########
cpp/velox/tests/VeloxToSubstraitTypeTest.cc:
##########
@@ -62,4 +62,13 @@ TEST_F(VeloxToSubstraitTypeTest, basic) {
testTypeConversion(ROW({}, {}));
}
+TEST_F(VeloxToSubstraitTypeTest, timestampUtc) {
+ google::protobuf::Arena arena;
+ const auto& substraitType = typeConvertor_->toSubstraitType(arena,
TIMESTAMP_UTC());
+
+ ASSERT_TRUE(substraitType.has_precision_timestamp());
+ ASSERT_EQ(substraitType.precision_timestamp().precision(), 6);
+
ASSERT_TRUE(SubstraitParser::parseType(substraitType)->equivalent(*TIMESTAMP_UTC()));
Review Comment:
I exercised this locally with both required and nullable inputs. Assertions
expecting REQUIRED to survive the round trip fail for both TIMESTAMP_UTC
and ordinary TIMESTAMP ; the proposed nullable = false overload does not
exist. I’ve kept the reproducer separate for the potential follow-up issue
rather than adding assertions that make the existing loss of requiredness an
expectation of this PR. The original precision and logical-type coverage
remains unchanged.
--
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]
