kou commented on a change in pull request #11975:
URL: https://github.com/apache/arrow/pull/11975#discussion_r771057423
##########
File path: c_glib/arrow-glib/basic-data-type.cpp
##########
@@ -1287,6 +1296,137 @@ garrow_time64_data_type_new(GArrowTimeUnit unit, GError
**error)
}
+G_DEFINE_ABSTRACT_TYPE(GArrowIntervalDataType,
+ garrow_interval_data_type,
+ GARROW_TYPE_TEMPORAL_DATA_TYPE)
+
+static void
+garrow_interval_data_type_init(GArrowIntervalDataType *object)
+{
+}
+
+static void
+garrow_interval_data_type_class_init(GArrowIntervalDataTypeClass *klass)
+{
+}
+
+/**
+ * garrow_interval_data_type_get_interval_type:
+ * @type: #GArrowIntervalDataType.
+ *
+ * Returns: GArrowIntervalType
Review comment:
We need more description here.
##########
File path: c_glib/arrow-glib/basic-data-type.cpp
##########
@@ -97,6 +97,15 @@ G_BEGIN_DECLS
* #GArrowTime64DataType is a class for the number of microseconds or
* nanoseconds since midnight in the 64-bit signed integer data type.
*
+ * #GArrowIntervalDataType is an abstract class for interval related data type
+ * such as #GArrowIntervalMonthsDataType.
+ *
+ * #GArrowIntervalMonthsDataType is a class for the month intarval data type.
Review comment:
We should use `GArrowXXXIntervalDataType` for consistency.
For example, we already have `GArrowListDataType` and
`GArrowLargeListDataType`.
For `arrow::IntervalType::MONTHS`, let's use `MONTH` not `MONTHS` in GLib
for consistency. Other interval types don't have `S`.
We can discuss C++ API (`arrow::IntervalType::MONTHS` or `...::MONTH`, which
is better?) later with C++ developers on Jira or mailing list.
##########
File path: c_glib/arrow-glib/basic-data-type.cpp
##########
@@ -1287,6 +1296,137 @@ garrow_time64_data_type_new(GArrowTimeUnit unit, GError
**error)
}
+G_DEFINE_ABSTRACT_TYPE(GArrowIntervalDataType,
+ garrow_interval_data_type,
+ GARROW_TYPE_TEMPORAL_DATA_TYPE)
+
+static void
+garrow_interval_data_type_init(GArrowIntervalDataType *object)
+{
+}
+
+static void
+garrow_interval_data_type_class_init(GArrowIntervalDataTypeClass *klass)
+{
+}
+
+/**
+ * garrow_interval_data_type_get_interval_type:
+ * @type: #GArrowIntervalDataType.
+ *
+ * Returns: GArrowIntervalType
+ *
+ * Since: 7.0.0
+ */
+GArrowIntervalType
+garrow_interval_data_type_get_interval_type(GArrowIntervalDataType *type) {
Review comment:
We use the following style:
```c++
TYPE
FUNCTION_NAME(ARGS)
{
```
##########
File path: c_glib/arrow-glib/type.h
##########
@@ -94,6 +95,7 @@ typedef enum {
GARROW_TYPE_TIME64,
GARROW_TYPE_INTERVAL_MONTHS,
GARROW_TYPE_INTERVAL_DAY_TIME,
+ GARROW_TYPE_INTERVAL_MONTH_DAY_NANO,
Review comment:
We should not put this here.
We need to use the same order as `arrow::Type::type` to use the same integer
value.
##########
File path: c_glib/test/test-interval-day-time-data-type.rb
##########
@@ -0,0 +1,38 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+class TestIntervalDayTimeDataType < Test::Unit::TestCase
+ def test_type
+ data_type = Arrow::IntervalDayTimeDataType.new
Review comment:
We can use `setup` for common setup code:
```ruby
def setup
@data_type = Arrow::IntervalDayTimeDataType.new
end
def test_type
assert_equal(..., @data_type.id)
end
def test_xxx
assert_equal(..., @data_type.xxx)
end
```
##########
File path: c_glib/arrow-glib/basic-data-type.cpp
##########
@@ -1287,6 +1296,137 @@ garrow_time64_data_type_new(GArrowTimeUnit unit, GError
**error)
}
+G_DEFINE_ABSTRACT_TYPE(GArrowIntervalDataType,
+ garrow_interval_data_type,
+ GARROW_TYPE_TEMPORAL_DATA_TYPE)
+
+static void
+garrow_interval_data_type_init(GArrowIntervalDataType *object)
+{
+}
+
+static void
+garrow_interval_data_type_class_init(GArrowIntervalDataTypeClass *klass)
+{
+}
+
+/**
+ * garrow_interval_data_type_get_interval_type:
+ * @type: #GArrowIntervalDataType.
+ *
+ * Returns: GArrowIntervalType
+ *
+ * Since: 7.0.0
+ */
+GArrowIntervalType
+garrow_interval_data_type_get_interval_type(GArrowIntervalDataType *type) {
+ const auto arrow_data_type =
+ garrow_data_type_get_raw(GARROW_DATA_TYPE(type));
+ const auto arrow_interval_type =
+ std::static_pointer_cast<arrow::IntervalType>(arrow_data_type);
+ return garrow_interval_type_from_raw(arrow_interval_type->interval_type());
+}
+
+
+G_DEFINE_TYPE(GArrowIntervalMonthsDataType,
+ garrow_interval_months_data_type,
+ GARROW_TYPE_INTERVAL_DATA_TYPE)
+static void
+garrow_interval_months_data_type_init(GArrowIntervalMonthsDataType *object)
+{
+}
+
+static void
+garrow_interval_months_data_type_class_init(GArrowIntervalMonthsDataTypeClass
*klass)
+{
+}
+
+/**
+ * garrow_interval_months_data_type_new:
+ *
+ * Returns: The newly created interval months data type.
+ *
+ * Since: 7.0.0
+ */
+GArrowIntervalMonthsDataType *
+garrow_interval_months_data_type_new(void)
+{
+ auto arrow_data_type = arrow::month_interval();
+
+ GArrowIntervalMonthsDataType *data_type =
Review comment:
We can use `auto` here.
##########
File path: c_glib/arrow-glib/type.h
##########
@@ -126,4 +128,18 @@ typedef enum {
GARROW_TIME_UNIT_NANO
} GArrowTimeUnit;
+/**
+ * GArrowIntervalType:
+ * @GARROW_INTERVAL_TYPE_MONTHS: Months.
+ * @GARROW_INTERVAL_TYPE_DAY_TIME: Days milliseconds.
+ * @GARROW_INTERVAL_TYPE_MONTH_DAY_NANO: Months days nanoseconds.
+ *
+ * They are corresponding to `arrow::IntervalType::type` values.
+ */
Review comment:
`Since: 7.0.0` is missing.
##########
File path: c_glib/arrow-glib/basic-data-type.h
##########
@@ -457,6 +457,65 @@ GArrowTime64DataType *garrow_time64_data_type_new
(GArrowTimeUnit unit,
GError **error);
+#define GARROW_TYPE_INTERVAL_DATA_TYPE (garrow_interval_data_type_get_type())
+G_DECLARE_DERIVABLE_TYPE(GArrowIntervalDataType,
+ garrow_interval_data_type,
+ GARROW,
+ INTERVAL_DATA_TYPE,
+ GArrowTimeDataType)
+struct _GArrowIntervalDataTypeClass
+{
+ GArrowTimeDataTypeClass parent_class;
+};
+
+GArrowIntervalType
garrow_interval_data_type_get_interval_type(GArrowIntervalDataType *type);
Review comment:
We need to add `GARROW_AVAILABLE_IN_7_0` annotation for new functions.
--
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]
