Copilot commented on code in PR #50049:
URL: https://github.com/apache/arrow/pull/50049#discussion_r3328565610
##########
cpp/src/arrow/array/builder_binary.h:
##########
@@ -390,6 +400,32 @@ class BaseBinaryBuilder
}
protected:
+ template <typename LogicalType>
+ static std::shared_ptr<DataType> ValidateType(std::shared_ptr<DataType>
type) {
+ ARROW_CHECK(type != nullptr) << "Cannot construct binary builder with null
type";
+ const auto expected_type = TypeTraits<LogicalType>::type_singleton();
Review Comment:
ValidateType() uses ARROW_CHECK for user-supplied constructor inputs (both
null type and mismatched logical/storage types). ARROW_CHECK will terminate the
process in release builds; for public builders, prefer returning a Status error
(e.g. validate in FinishInternal / Finish) or otherwise avoid a fatal check on
invalid inputs.
##########
cpp/src/arrow/array/array_binary_test.cc:
##########
@@ -48,6 +50,35 @@ namespace arrow {
using internal::checked_cast;
+class BinaryExtensionType : public ExtensionType {
+ public:
+ BinaryExtensionType(std::shared_ptr<DataType> storage_type, std::string
extension_name)
+ : ExtensionType(std::move(storage_type)),
+ extension_name_(std::move(extension_name)) {}
+
+ std::string extension_name() const override { return extension_name_; }
+
+ bool ExtensionEquals(const ExtensionType& other) const override {
+ return other.extension_name() == this->extension_name() &&
+ storage_type()->Equals(*other.storage_type());
+ }
+
+ std::shared_ptr<Array> MakeArray(std::shared_ptr<ArrayData> data) const
override {
+ return std::make_shared<ExtensionArray>(std::move(data));
+ }
+
+ Result<std::shared_ptr<DataType>> Deserialize(
+ std::shared_ptr<DataType> storage_type,
+ const std::string& serialized) const override {
+ return Status::NotImplemented("BinaryExtensionType::Deserialize");
Review Comment:
BinaryExtensionType::Deserialize() doesn't use its parameters, which can
trigger -Wunused-parameter warnings in some build configurations. Consider
omitting the parameter names (or explicitly marking them unused) to keep tests
warning-clean.
--
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]
