You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by pa...@apache.org on 2023/02/23 16:07:10 UTC
[arrow-nanoarrow] branch main updated: fix: Correct storage type for timestamp and duration types (#116)
This is an automated email from the ASF dual-hosted git repository.
paleolimbot pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-nanoarrow.git
The following commit(s) were added to refs/heads/main by this push:
new eb1812d fix: Correct storage type for timestamp and duration types (#116)
eb1812d is described below
commit eb1812d6fef5c1476a37b9a53fefa1f8bec9b339
Author: Dewey Dunnington <de...@dunnington.ca>
AuthorDate: Thu Feb 23 12:07:05 2023 -0400
fix: Correct storage type for timestamp and duration types (#116)
After spending some quality time with
https://github.com/apache/arrow/blob/main/format/Schema.fbs recently, I
noticed that the storage types for timestamp and duration types were
incorrect: they are always 64-bit integers (as opposed to the time type,
which switches width depending on the unit).
---
r/tests/testthat/test-as-array.R | 83 ++++++++++++++++++++++++++++++++++++++++
src/nanoarrow/schema.c | 8 ++--
src/nanoarrow/schema_test.cc | 8 ++--
3 files changed, 91 insertions(+), 8 deletions(-)
diff --git a/r/tests/testthat/test-as-array.R b/r/tests/testthat/test-as-array.R
index 8bb250d..6beff31 100644
--- a/r/tests/testthat/test-as-array.R
+++ b/r/tests/testthat/test-as-array.R
@@ -329,6 +329,89 @@ test_that("as_nanoarrow_array() errors for bad data.frame() -> na_struct()", {
)
})
+test_that("as_nanoarrow_array() works for Date -> na_date32()", {
+ array <- as_nanoarrow_array(as.Date(c("2000-01-01", "2023-02-03", NA)))
+
+ expect_identical(infer_nanoarrow_schema(array)$format, "tdD")
+ expect_identical(array$length, 3L)
+ expect_identical(array$null_count, 1L)
+
+ expect_identical(as.raw(array$buffers[[1]]), as.raw(0x03))
+ expect_identical(
+ as.raw(array$buffers[[2]]),
+ as.raw(as_nanoarrow_buffer(c(10957L, 19391L, NA)))
+ )
+})
+
+test_that("as_nanoarrow_array() works for Date -> na_date64()", {
+ array <- as_nanoarrow_array(
+ as.Date(c("2000-01-01", "2023-02-03", NA)),
+ schema = na_date64()
+ )
+
+ expect_identical(infer_nanoarrow_schema(array)$format, "tdm")
+ expect_identical(array$length, 3L)
+ expect_identical(array$null_count, 1L)
+
+ expect_identical(as.raw(array$buffers[[1]]), as.raw(0x03))
+ storage <- as_nanoarrow_array(
+ c(10957L, 19391L, NA) * 86400000,
+ schema = na_int64()
+ )
+
+ expect_identical(
+ as.raw(array$buffers[[2]]),
+ as.raw(storage$buffers[[2]])
+ )
+})
+
+test_that("as_nanoarrow_array() works for POSIXct -> na_timestamp()", {
+ array <- as_nanoarrow_array(
+ as.POSIXct(c("2000-01-01", "2023-02-03", NA), tz = "UTC"),
+ schema = na_timestamp("ms", timezone = "UTC")
+ )
+
+ expect_identical(infer_nanoarrow_schema(array)$format, "tsm:UTC")
+ expect_identical(array$length, 3L)
+ expect_identical(array$null_count, 1L)
+
+ expect_identical(as.raw(array$buffers[[1]]), as.raw(0x03))
+ storage <- as_nanoarrow_array(
+ c(10957L, 19391L, NA) * 86400000,
+ schema = na_int64()
+ )
+
+ expect_identical(
+ as.raw(array$buffers[[2]]),
+ as.raw(storage$buffers[[2]])
+ )
+})
+
+test_that("as_nanoarrow_array() works for difftime -> na_duration()", {
+ array <- as_nanoarrow_array(
+ as.difftime(c(1:5, NA), units = "secs"),
+ schema = na_duration("ms")
+ )
+
+ expect_identical(infer_nanoarrow_schema(array)$format, "tDm")
+ expect_identical(array$length, 6L)
+ expect_identical(array$null_count, 1L)
+
+ expect_identical(
+ as.raw(array$buffers[[1]]),
+ packBits(c(rep(TRUE, 5), FALSE, rep(FALSE, 2)))
+ )
+ storage <- as_nanoarrow_array(
+ c(1:5, NA) * 1000,
+ schema = na_int64()
+ )
+
+ expect_identical(
+ as.raw(array$buffers[[2]]),
+ as.raw(storage$buffers[[2]])
+ )
+})
+
test_that("as_nanoarrow_array() works for blob::blob() -> na_binary()", {
skip_if_not_installed("blob")
diff --git a/src/nanoarrow/schema.c b/src/nanoarrow/schema.c
index bdfb14a..3ce6518 100644
--- a/src/nanoarrow/schema.c
+++ b/src/nanoarrow/schema.c
@@ -838,12 +838,12 @@ static ArrowErrorCode ArrowSchemaViewParse(struct ArrowSchemaView* schema_view,
case 's':
switch (format[2]) {
case 's':
- ArrowSchemaViewSetPrimitive(schema_view, NANOARROW_TYPE_INT32);
+ ArrowSchemaViewSetPrimitive(schema_view, NANOARROW_TYPE_INT64);
schema_view->type = NANOARROW_TYPE_TIMESTAMP;
schema_view->time_unit = NANOARROW_TIME_UNIT_SECOND;
break;
case 'm':
- ArrowSchemaViewSetPrimitive(schema_view, NANOARROW_TYPE_INT32);
+ ArrowSchemaViewSetPrimitive(schema_view, NANOARROW_TYPE_INT64);
schema_view->type = NANOARROW_TYPE_TIMESTAMP;
schema_view->time_unit = NANOARROW_TIME_UNIT_MILLI;
break;
@@ -878,13 +878,13 @@ static ArrowErrorCode ArrowSchemaViewParse(struct ArrowSchemaView* schema_view,
case 'D':
switch (format[2]) {
case 's':
- ArrowSchemaViewSetPrimitive(schema_view, NANOARROW_TYPE_INT32);
+ ArrowSchemaViewSetPrimitive(schema_view, NANOARROW_TYPE_INT64);
schema_view->type = NANOARROW_TYPE_DURATION;
schema_view->time_unit = NANOARROW_TIME_UNIT_SECOND;
*format_end_out = format + 3;
return NANOARROW_OK;
case 'm':
- ArrowSchemaViewSetPrimitive(schema_view, NANOARROW_TYPE_INT32);
+ ArrowSchemaViewSetPrimitive(schema_view, NANOARROW_TYPE_INT64);
schema_view->type = NANOARROW_TYPE_DURATION;
schema_view->time_unit = NANOARROW_TIME_UNIT_MILLI;
*format_end_out = format + 3;
diff --git a/src/nanoarrow/schema_test.cc b/src/nanoarrow/schema_test.cc
index c03992f..9bcdd05 100644
--- a/src/nanoarrow/schema_test.cc
+++ b/src/nanoarrow/schema_test.cc
@@ -860,7 +860,7 @@ TEST(SchemaViewTest, SchemaViewInitTimeTimestamp) {
ARROW_EXPECT_OK(ExportType(*timestamp(TimeUnit::SECOND, "America/Halifax"), &schema));
EXPECT_EQ(ArrowSchemaViewInit(&schema_view, &schema, &error), NANOARROW_OK);
EXPECT_EQ(schema_view.type, NANOARROW_TYPE_TIMESTAMP);
- EXPECT_EQ(schema_view.storage_type, NANOARROW_TYPE_INT32);
+ EXPECT_EQ(schema_view.storage_type, NANOARROW_TYPE_INT64);
EXPECT_EQ(schema_view.time_unit, NANOARROW_TIME_UNIT_SECOND);
EXPECT_STREQ(schema_view.timezone, "America/Halifax");
EXPECT_EQ(ArrowSchemaToStdString(&schema), "timestamp('s', 'America/Halifax')");
@@ -869,7 +869,7 @@ TEST(SchemaViewTest, SchemaViewInitTimeTimestamp) {
ARROW_EXPECT_OK(ExportType(*timestamp(TimeUnit::MILLI, "America/Halifax"), &schema));
EXPECT_EQ(ArrowSchemaViewInit(&schema_view, &schema, &error), NANOARROW_OK);
EXPECT_EQ(schema_view.type, NANOARROW_TYPE_TIMESTAMP);
- EXPECT_EQ(schema_view.storage_type, NANOARROW_TYPE_INT32);
+ EXPECT_EQ(schema_view.storage_type, NANOARROW_TYPE_INT64);
EXPECT_EQ(schema_view.time_unit, NANOARROW_TIME_UNIT_MILLI);
EXPECT_STREQ(schema_view.timezone, "America/Halifax");
EXPECT_EQ(ArrowSchemaToStdString(&schema), "timestamp('ms', 'America/Halifax')");
@@ -902,7 +902,7 @@ TEST(SchemaViewTest, SchemaViewInitTimeDuration) {
ARROW_EXPECT_OK(ExportType(*duration(TimeUnit::SECOND), &schema));
EXPECT_EQ(ArrowSchemaViewInit(&schema_view, &schema, &error), NANOARROW_OK);
EXPECT_EQ(schema_view.type, NANOARROW_TYPE_DURATION);
- EXPECT_EQ(schema_view.storage_type, NANOARROW_TYPE_INT32);
+ EXPECT_EQ(schema_view.storage_type, NANOARROW_TYPE_INT64);
EXPECT_EQ(schema_view.time_unit, NANOARROW_TIME_UNIT_SECOND);
EXPECT_EQ(ArrowSchemaToStdString(&schema), "duration('s')");
schema.release(&schema);
@@ -910,7 +910,7 @@ TEST(SchemaViewTest, SchemaViewInitTimeDuration) {
ARROW_EXPECT_OK(ExportType(*duration(TimeUnit::MILLI), &schema));
EXPECT_EQ(ArrowSchemaViewInit(&schema_view, &schema, &error), NANOARROW_OK);
EXPECT_EQ(schema_view.type, NANOARROW_TYPE_DURATION);
- EXPECT_EQ(schema_view.storage_type, NANOARROW_TYPE_INT32);
+ EXPECT_EQ(schema_view.storage_type, NANOARROW_TYPE_INT64);
EXPECT_EQ(schema_view.time_unit, NANOARROW_TIME_UNIT_MILLI);
EXPECT_EQ(ArrowSchemaToStdString(&schema), "duration('ms')");
schema.release(&schema);