[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #948: feat(c/driver): Date32 support

["WillAyd (via GitHub)" <gi...@apache.org>]

WillAyd commented on code in PR #948:
URL: https://github.com/apache/arrow-adbc/pull/948#discussion_r1278031289


##########
c/validation/adbc_validation.cc:
##########
@@ -1095,8 +1100,12 @@ void StatementTest::TestSqlIngestBinary() {
       NANOARROW_TYPE_BINARY, {std::nullopt, "", "\x00\x01\x02\x04", "\xFE\xFF"}));
 }
 
+void StatementTest::TestSqlIngestDate32() {
+  ASSERT_NO_FATAL_FAILURE(TestSqlIngestNumericType<int32_t>(NANOARROW_TYPE_DATE32));

Review Comment:
   Right now this tests the postgres implementation, but the SQLite implementation doesn't get tested since the roundtrip value is a string.
   
   For the timestamps we have separate test bodies and a function like `ValidateIngestedTimestampData` that the tests use to override what should be returned. We could follow that same pattern here though it is a bit heavy-handed given how this fits into `TestSqlIngestNumericType` pretty easily
   
   The general problem is finding the right pattern to round trip tests for things where the input type doesn't match the output 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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org