pitrou commented on code in PR #47922:
URL: https://github.com/apache/arrow/pull/47922#discussion_r2758444680
##########
cpp/src/arrow/type_test.cc:
##########
@@ -49,6 +51,66 @@ TEST(TestTypeId, AllTypeIds) {
ASSERT_EQ(static_cast<int>(all_ids.size()), Type::MAX_ID);
}
+TEST(TestTypeSingleton, ParameterFreeTypes) {
+ // Test successful cases - parameter-free types
+ std::vector<std::pair<Type::type, std::shared_ptr<DataType>>> cases = {
Review Comment:
If we use `TypeTraits` in the implementation then we don't need to test
these exhaustively, we can just check a couple examples.
##########
cpp/src/arrow/type_traits.cc:
##########
@@ -17,10 +17,73 @@
#include "arrow/type_traits.h"
+#include "arrow/result.h"
+#include "arrow/status.h"
+#include "arrow/type.h"
#include "arrow/util/logging_internal.h"
namespace arrow {
+Result<std::shared_ptr<DataType>> type_singleton(Type::type id) {
+ switch (id) {
+ case Type::NA:
+ return null();
+ case Type::BOOL:
+ return boolean();
+ case Type::INT8:
+ return int8();
+ case Type::INT16:
+ return int16();
+ case Type::INT32:
+ return int32();
+ case Type::INT64:
+ return int64();
+ case Type::UINT8:
+ return uint8();
+ case Type::UINT16:
+ return uint16();
+ case Type::UINT32:
+ return uint32();
+ case Type::UINT64:
+ return uint64();
+ case Type::HALF_FLOAT:
+ return float16();
+ case Type::FLOAT:
+ return float32();
+ case Type::DOUBLE:
+ return float64();
+ case Type::STRING:
+ return utf8();
+ case Type::BINARY:
+ return binary();
+ case Type::LARGE_STRING:
+ return large_utf8();
+ case Type::LARGE_BINARY:
+ return large_binary();
+ case Type::DATE32:
+ return date32();
+
+ // Explicitly handle known parameterized types
+ case Type::TIMESTAMP:
+ case Type::TIME32:
+ case Type::TIME64:
+ case Type::DURATION:
+ case Type::FIXED_SIZE_BINARY:
+ case Type::DECIMAL128:
+ case Type::LIST:
+ case Type::LARGE_LIST:
+ case Type::FIXED_SIZE_LIST:
+ case Type::STRUCT:
+ case Type::DICTIONARY:
+ case Type::MAP:
+ case Type::EXTENSION:
+ return Status::Invalid("Type ", id, " is not a parameter-free singleton
type.");
Review Comment:
Should probably `TypeError` instead.
##########
cpp/src/arrow/type_traits.h:
##########
@@ -596,6 +598,19 @@ struct TypeTraits<ExtensionType> {
};
/// @}
+/// \brief Create a data type instance from a type ID for parameter-free types
+///
+/// This function creates a data type instance for types that don't require
+/// additional parameters (where TypeTraits<T>::is_parameter_free is true).
+/// For types that require parameters (like TimestampType or ListType),
+/// this function will return an error.
+///
+/// \param[in] id The type ID to create a type instance for
+/// \return Result with a shared pointer to the created DataType instance,
+/// or an error if the type requires parameters
+ARROW_EXPORT
+Result<std::shared_ptr<DataType>> type_singleton(Type::type id);
Review Comment:
I would put this in `type.h` since that's where data types constructors live.
##########
cpp/src/arrow/type_traits.cc:
##########
@@ -17,10 +17,73 @@
#include "arrow/type_traits.h"
+#include "arrow/result.h"
+#include "arrow/status.h"
+#include "arrow/type.h"
#include "arrow/util/logging_internal.h"
namespace arrow {
+Result<std::shared_ptr<DataType>> type_singleton(Type::type id) {
+ switch (id) {
+ case Type::NA:
Review Comment:
We don't need to handle these explicitly, we can use `VisitTypeIdInline` and
then use something like `TypeTraits<T>::type_singleton`. This will ensure that
this function works with all existing types at all times.
--
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]
