You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Hoeze (via GitHub)" <gi...@apache.org> on 2026/05/24 13:35:14 UTC

[PR] GH-50027: [Format][C++][Python] Add arrow.range canonical extension type [arrow]

Hoeze opened a new pull request, #50028:
URL: https://github.com/apache/arrow/pull/50028

   ### Rationale for this change
   This is a draft implementation for #50027, a new canonical `range` extension type.
   
   ### What changes are included in this PR?
   This PR provides the spec text, a C++ reference implementation, PyArrow bindings, and the supporting documentation.
   
   ### Are these changes tested?
   I let the tests run locally but did not try them in any other project yet.
   
   Note that I made heavy use of AI to create this PR and copied many structures from the fixed shape tensor extension type. I reviewed each change and hope the changes I made are meaningful.
   Nevertheless, I am not sure whether the C++ parts are comprehensive or if I missed anything; this is my first contribution to Arrow.
   
   ### Are there any user-facing changes?
   No, this is an addition of a new canonical extension 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


Re: [PR] GH-50027: [Format][C++][Python] Add fixed and variable closedness range canonical extension types [arrow]

Posted by "Hoeze (via GitHub)" <gi...@apache.org>.
Hoeze commented on code in PR #50028:
URL: https://github.com/apache/arrow/pull/50028#discussion_r3999603293


##########
cpp/src/arrow/extension/range.cc:
##########
@@ -0,0 +1,345 @@
+// 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.
+
+#include "arrow/extension/range.h"
+
+#include <sstream>
+#include <string_view>
+
+#include "arrow/json/rapidjson_defs.h"  // IWYU pragma: keep
+#include "arrow/util/logging_internal.h"
+
+#include <rapidjson/document.h>
+#include <rapidjson/error/en.h>
+#include <rapidjson/writer.h>
+
+namespace arrow::extension {
+
+namespace {
+
+/// Map RangeClosed -> the JSON string value used in serialization.
+std::string_view ClosedToString(RangeClosed closed) {
+  switch (closed) {
+    case RangeClosed::Left:
+      return "left";
+    case RangeClosed::Right:
+      return "right";
+    case RangeClosed::Both:
+      return "both";
+    case RangeClosed::Neither:
+      return "neither";
+  }
+  // unreachable
+  return "right";
+}
+
+/// Parse the JSON "closed" string into a RangeClosed enum.
+/// Returns an error if the string is not one of the four valid values.
+Result<RangeClosed> ClosedFromString(std::string_view s) {
+  if (s == "left") return RangeClosed::Left;
+  if (s == "right") return RangeClosed::Right;
+  if (s == "both") return RangeClosed::Both;
+  if (s == "neither") return RangeClosed::Neither;
+  return Status::Invalid(
+      "Invalid value for RangeType \"closed\" parameter: \"", s,
+      "\". Expected one of: \"left\", \"right\", \"both\", \"neither\".");
+}
+
+/// Build the storage Struct type for a given value subtype.
+std::shared_ptr<DataType> MakeStorageType(const std::shared_ptr<DataType>& value_type,
+                                          bool allow_unbounded) {
+  // Nullable bounds can represent an unbounded (infinite) endpoint; non-nullable
+  // bounds are always finite.
+  return struct_({field("lower", value_type, allow_unbounded),
+                  field("upper", value_type, allow_unbounded)});
+}
+
+}  // namespace
+
+// ---------------------------------------------------------------------------
+// RangeType
+
+std::shared_ptr<DataType> RangeType::value_type() const {
+  // storage_type() is a struct with two fields; both share the same type.
+  return internal::checked_cast<const StructType&>(*storage_type()).field(0)->type();
+}
+
+std::string RangeType::ToString(bool show_metadata) const {
+  std::stringstream ss;
+  ss << "extension<" << this->extension_name()
+     << "[value_type=" << value_type()->ToString(show_metadata)
+     << ", closed=" << ClosedToString(closed_) << "]>";
+  return ss.str();
+}
+
+bool RangeType::ExtensionEquals(const ExtensionType& other) const {
+  if (extension_name() != other.extension_name()) {
+    return false;
+  }
+  const auto& other_range = internal::checked_cast<const RangeType&>(other);
+  return storage_type()->Equals(*other_range.storage_type()) &&
+         closed_ == other_range.closed_;
+}
+
+std::string RangeType::Serialize() const {
+  rapidjson::Document document;
+  document.SetObject();
+  rapidjson::Document::AllocatorType& allocator = document.GetAllocator();
+
+  auto closed_str = ClosedToString(closed_);
+  rapidjson::Value closed_value(
+      closed_str.data(), static_cast<rapidjson::SizeType>(closed_str.size()), allocator);
+  document.AddMember(rapidjson::Value("closed", allocator), closed_value, allocator);
+
+  rapidjson::StringBuffer buffer;
+  rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
+  document.Accept(writer);
+  return buffer.GetString();
+}
+
+Result<std::shared_ptr<DataType>> RangeType::Deserialize(
+    std::shared_ptr<DataType> storage_type, const std::string& serialized_data) const {
+  // Validate storage type structure.
+  if (storage_type->id() != Type::STRUCT) {
+    return Status::Invalid("RangeType storage type must be a Struct, got ",
+                           storage_type->ToString());
+  }
+  const auto& struct_type = internal::checked_cast<const StructType&>(*storage_type);
+  if (struct_type.num_fields() != 2) {
+    return Status::Invalid("RangeType storage Struct must have exactly 2 fields, got ",
+                           struct_type.num_fields());
+  }
+  const auto& lower_field = struct_type.field(0);
+  const auto& upper_field = struct_type.field(1);
+  if (lower_field->name() != "lower") {
+    return Status::Invalid(
+        "RangeType storage Struct field 0 must be named \"lower\", got \"",
+        lower_field->name(), "\"");
+  }
+  if (upper_field->name() != "upper") {
+    return Status::Invalid(
+        "RangeType storage Struct field 1 must be named \"upper\", got \"",
+        upper_field->name(), "\"");
+  }
+  if (!lower_field->type()->Equals(*upper_field->type())) {
+    return Status::Invalid(
+        "RangeType storage Struct fields \"lower\" and \"upper\" must have the same "
+        "type, got \"",
+        lower_field->type()->ToString(), "\" and \"", upper_field->type()->ToString(),
+        "\"");
+  }
+
+  // Parse the required "closed" parameter from JSON metadata. The closedness
+  // is not defaulted on the wire: empty metadata or a missing key is invalid.
+  if (serialized_data.empty()) {
+    return Status::Invalid(
+        "RangeType metadata must be a JSON object with a required \"closed\" key, "
+        "got an empty string");
+  }
+  rapidjson::Document document;
+  const auto& parsed = document.Parse(serialized_data.data(), serialized_data.length());
+  if (parsed.HasParseError()) {
+    return Status::Invalid("Invalid serialized JSON data for RangeType: ",
+                           rapidjson::GetParseError_En(parsed.GetParseError()), ": ",
+                           serialized_data);
+  }
+  if (!document.IsObject()) {
+    return Status::Invalid("Invalid serialized JSON data for RangeType: not an object");
+  }
+  if (!document.HasMember("closed")) {
+    return Status::Invalid("RangeType metadata is missing the required \"closed\" key: ",
+                           serialized_data);
+  }
+  const auto& closed_val = document["closed"];
+  if (!closed_val.IsString()) {
+    return Status::Invalid(
+        "Invalid serialized JSON data for RangeType: \"closed\" is not a string");
+  }
+  ARROW_ASSIGN_OR_RAISE(RangeClosed closed,
+                        ClosedFromString(std::string_view(closed_val.GetString(),
+                                                          closed_val.GetStringLength())));
+
+  return std::make_shared<RangeType>(std::move(storage_type), closed);
+}
+
+std::shared_ptr<Array> RangeType::MakeArray(std::shared_ptr<ArrayData> data) const {
+  DCHECK_EQ(data->type->id(), Type::EXTENSION);
+  DCHECK_EQ("arrow.range",
+            internal::checked_cast<const ExtensionType&>(*data->type).extension_name());
+  return std::make_shared<RangeArray>(data);
+}
+
+Result<std::shared_ptr<DataType>> RangeType::Make(std::shared_ptr<DataType> value_type,
+                                                  RangeClosed closed,
+                                                  bool allow_unbounded) {
+  auto storage = MakeStorageType(value_type, allow_unbounded);
+  return std::make_shared<RangeType>(std::move(storage), closed);
+}
+
+// ---------------------------------------------------------------------------
+// Free factory function
+
+std::shared_ptr<DataType> range(std::shared_ptr<DataType> value_type, RangeClosed closed,
+                                bool allow_unbounded) {
+  auto result = RangeType::Make(std::move(value_type), closed, allow_unbounded);
+  ARROW_CHECK_OK(result.status());
+  return std::move(result).ValueOrDie();
+}
+
+// ---------------------------------------------------------------------------
+// RangeIncType

Review Comment:
   I renamed it to FixedClosednessRangeType and VariableClosednessRangeType for now



-- 
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


Re: [PR] GH-50027: [Format][C++][Python] Add arrow.range canonical extension type [arrow]

Posted by "rok (via GitHub)" <gi...@apache.org>.
rok commented on PR #50028:
URL: https://github.com/apache/arrow/pull/50028#issuecomment-4529791475

   See [comment](https://github.com/apache/arrow/issues/50027#issuecomment-4529789066).


-- 
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


Re: [PR] GH-50027: [Format][C++][Python] Add arrow.range canonical extension type [arrow]

Posted by "Hoeze (via GitHub)" <gi...@apache.org>.
Hoeze commented on code in PR #50028:
URL: https://github.com/apache/arrow/pull/50028#discussion_r3359205009


##########
cpp/src/arrow/extension/range.cc:
##########
@@ -0,0 +1,345 @@
+// 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.
+
+#include "arrow/extension/range.h"
+
+#include <sstream>
+#include <string_view>
+
+#include "arrow/json/rapidjson_defs.h"  // IWYU pragma: keep
+#include "arrow/util/logging_internal.h"
+
+#include <rapidjson/document.h>
+#include <rapidjson/error/en.h>
+#include <rapidjson/writer.h>
+
+namespace arrow::extension {
+
+namespace {
+
+/// Map RangeClosed -> the JSON string value used in serialization.
+std::string_view ClosedToString(RangeClosed closed) {
+  switch (closed) {
+    case RangeClosed::Left:
+      return "left";
+    case RangeClosed::Right:
+      return "right";
+    case RangeClosed::Both:
+      return "both";
+    case RangeClosed::Neither:
+      return "neither";
+  }
+  // unreachable
+  return "right";
+}
+
+/// Parse the JSON "closed" string into a RangeClosed enum.
+/// Returns an error if the string is not one of the four valid values.
+Result<RangeClosed> ClosedFromString(std::string_view s) {
+  if (s == "left") return RangeClosed::Left;
+  if (s == "right") return RangeClosed::Right;
+  if (s == "both") return RangeClosed::Both;
+  if (s == "neither") return RangeClosed::Neither;
+  return Status::Invalid(
+      "Invalid value for RangeType \"closed\" parameter: \"", s,
+      "\". Expected one of: \"left\", \"right\", \"both\", \"neither\".");
+}
+
+/// Build the storage Struct type for a given value subtype.
+std::shared_ptr<DataType> MakeStorageType(const std::shared_ptr<DataType>& value_type,
+                                          bool allow_unbounded) {
+  // Nullable bounds can represent an unbounded (infinite) endpoint; non-nullable
+  // bounds are always finite.
+  return struct_({field("lower", value_type, allow_unbounded),
+                  field("upper", value_type, allow_unbounded)});
+}
+
+}  // namespace
+
+// ---------------------------------------------------------------------------
+// RangeType
+
+std::shared_ptr<DataType> RangeType::value_type() const {
+  // storage_type() is a struct with two fields; both share the same type.
+  return internal::checked_cast<const StructType&>(*storage_type()).field(0)->type();
+}
+
+std::string RangeType::ToString(bool show_metadata) const {
+  std::stringstream ss;
+  ss << "extension<" << this->extension_name()
+     << "[value_type=" << value_type()->ToString(show_metadata)
+     << ", closed=" << ClosedToString(closed_) << "]>";
+  return ss.str();
+}
+
+bool RangeType::ExtensionEquals(const ExtensionType& other) const {
+  if (extension_name() != other.extension_name()) {
+    return false;
+  }
+  const auto& other_range = internal::checked_cast<const RangeType&>(other);
+  return storage_type()->Equals(*other_range.storage_type()) &&
+         closed_ == other_range.closed_;
+}
+
+std::string RangeType::Serialize() const {
+  rapidjson::Document document;
+  document.SetObject();
+  rapidjson::Document::AllocatorType& allocator = document.GetAllocator();
+
+  auto closed_str = ClosedToString(closed_);
+  rapidjson::Value closed_value(
+      closed_str.data(), static_cast<rapidjson::SizeType>(closed_str.size()), allocator);
+  document.AddMember(rapidjson::Value("closed", allocator), closed_value, allocator);
+
+  rapidjson::StringBuffer buffer;
+  rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
+  document.Accept(writer);
+  return buffer.GetString();
+}
+
+Result<std::shared_ptr<DataType>> RangeType::Deserialize(
+    std::shared_ptr<DataType> storage_type, const std::string& serialized_data) const {
+  // Validate storage type structure.
+  if (storage_type->id() != Type::STRUCT) {
+    return Status::Invalid("RangeType storage type must be a Struct, got ",
+                           storage_type->ToString());
+  }
+  const auto& struct_type = internal::checked_cast<const StructType&>(*storage_type);
+  if (struct_type.num_fields() != 2) {
+    return Status::Invalid("RangeType storage Struct must have exactly 2 fields, got ",
+                           struct_type.num_fields());
+  }
+  const auto& lower_field = struct_type.field(0);
+  const auto& upper_field = struct_type.field(1);
+  if (lower_field->name() != "lower") {
+    return Status::Invalid(
+        "RangeType storage Struct field 0 must be named \"lower\", got \"",
+        lower_field->name(), "\"");
+  }
+  if (upper_field->name() != "upper") {
+    return Status::Invalid(
+        "RangeType storage Struct field 1 must be named \"upper\", got \"",
+        upper_field->name(), "\"");
+  }
+  if (!lower_field->type()->Equals(*upper_field->type())) {
+    return Status::Invalid(
+        "RangeType storage Struct fields \"lower\" and \"upper\" must have the same "
+        "type, got \"",
+        lower_field->type()->ToString(), "\" and \"", upper_field->type()->ToString(),
+        "\"");
+  }
+
+  // Parse the required "closed" parameter from JSON metadata. The closedness
+  // is not defaulted on the wire: empty metadata or a missing key is invalid.
+  if (serialized_data.empty()) {
+    return Status::Invalid(
+        "RangeType metadata must be a JSON object with a required \"closed\" key, "
+        "got an empty string");
+  }
+  rapidjson::Document document;
+  const auto& parsed = document.Parse(serialized_data.data(), serialized_data.length());
+  if (parsed.HasParseError()) {
+    return Status::Invalid("Invalid serialized JSON data for RangeType: ",
+                           rapidjson::GetParseError_En(parsed.GetParseError()), ": ",
+                           serialized_data);
+  }
+  if (!document.IsObject()) {
+    return Status::Invalid("Invalid serialized JSON data for RangeType: not an object");
+  }
+  if (!document.HasMember("closed")) {
+    return Status::Invalid("RangeType metadata is missing the required \"closed\" key: ",
+                           serialized_data);
+  }
+  const auto& closed_val = document["closed"];
+  if (!closed_val.IsString()) {
+    return Status::Invalid(
+        "Invalid serialized JSON data for RangeType: \"closed\" is not a string");
+  }
+  ARROW_ASSIGN_OR_RAISE(RangeClosed closed,
+                        ClosedFromString(std::string_view(closed_val.GetString(),
+                                                          closed_val.GetStringLength())));
+
+  return std::make_shared<RangeType>(std::move(storage_type), closed);
+}
+
+std::shared_ptr<Array> RangeType::MakeArray(std::shared_ptr<ArrayData> data) const {
+  DCHECK_EQ(data->type->id(), Type::EXTENSION);
+  DCHECK_EQ("arrow.range",
+            internal::checked_cast<const ExtensionType&>(*data->type).extension_name());
+  return std::make_shared<RangeArray>(data);
+}
+
+Result<std::shared_ptr<DataType>> RangeType::Make(std::shared_ptr<DataType> value_type,
+                                                  RangeClosed closed,
+                                                  bool allow_unbounded) {
+  auto storage = MakeStorageType(value_type, allow_unbounded);
+  return std::make_shared<RangeType>(std::move(storage), closed);
+}
+
+// ---------------------------------------------------------------------------
+// Free factory function
+
+std::shared_ptr<DataType> range(std::shared_ptr<DataType> value_type, RangeClosed closed,
+                                bool allow_unbounded) {
+  auto result = RangeType::Make(std::move(value_type), closed, allow_unbounded);
+  ARROW_CHECK_OK(result.status());
+  return std::move(result).ValueOrDie();
+}
+
+// ---------------------------------------------------------------------------
+// RangeIncType

Review Comment:
   Absolutely :smile: 
   Some ideas:
   - VarRangeType
   - VariableClosedRangeType
   - PerValueRangeType
   
   What do you think?
   
   



-- 
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


Re: [PR] GH-50027: [Format][C++][Python] Add arrow.range canonical extension type [arrow]

Posted by "rok (via GitHub)" <gi...@apache.org>.
rok commented on code in PR #50028:
URL: https://github.com/apache/arrow/pull/50028#discussion_r3359490739


##########
cpp/src/arrow/extension/range.cc:
##########
@@ -0,0 +1,345 @@
+// 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.
+
+#include "arrow/extension/range.h"
+
+#include <sstream>
+#include <string_view>
+
+#include "arrow/json/rapidjson_defs.h"  // IWYU pragma: keep
+#include "arrow/util/logging_internal.h"
+
+#include <rapidjson/document.h>
+#include <rapidjson/error/en.h>
+#include <rapidjson/writer.h>
+
+namespace arrow::extension {
+
+namespace {
+
+/// Map RangeClosed -> the JSON string value used in serialization.
+std::string_view ClosedToString(RangeClosed closed) {
+  switch (closed) {
+    case RangeClosed::Left:
+      return "left";
+    case RangeClosed::Right:
+      return "right";
+    case RangeClosed::Both:
+      return "both";
+    case RangeClosed::Neither:
+      return "neither";
+  }
+  // unreachable
+  return "right";
+}
+
+/// Parse the JSON "closed" string into a RangeClosed enum.
+/// Returns an error if the string is not one of the four valid values.
+Result<RangeClosed> ClosedFromString(std::string_view s) {
+  if (s == "left") return RangeClosed::Left;
+  if (s == "right") return RangeClosed::Right;
+  if (s == "both") return RangeClosed::Both;
+  if (s == "neither") return RangeClosed::Neither;
+  return Status::Invalid(
+      "Invalid value for RangeType \"closed\" parameter: \"", s,
+      "\". Expected one of: \"left\", \"right\", \"both\", \"neither\".");
+}
+
+/// Build the storage Struct type for a given value subtype.
+std::shared_ptr<DataType> MakeStorageType(const std::shared_ptr<DataType>& value_type,
+                                          bool allow_unbounded) {
+  // Nullable bounds can represent an unbounded (infinite) endpoint; non-nullable
+  // bounds are always finite.
+  return struct_({field("lower", value_type, allow_unbounded),
+                  field("upper", value_type, allow_unbounded)});
+}
+
+}  // namespace
+
+// ---------------------------------------------------------------------------
+// RangeType
+
+std::shared_ptr<DataType> RangeType::value_type() const {
+  // storage_type() is a struct with two fields; both share the same type.
+  return internal::checked_cast<const StructType&>(*storage_type()).field(0)->type();
+}
+
+std::string RangeType::ToString(bool show_metadata) const {
+  std::stringstream ss;
+  ss << "extension<" << this->extension_name()
+     << "[value_type=" << value_type()->ToString(show_metadata)
+     << ", closed=" << ClosedToString(closed_) << "]>";
+  return ss.str();
+}
+
+bool RangeType::ExtensionEquals(const ExtensionType& other) const {
+  if (extension_name() != other.extension_name()) {
+    return false;
+  }
+  const auto& other_range = internal::checked_cast<const RangeType&>(other);
+  return storage_type()->Equals(*other_range.storage_type()) &&
+         closed_ == other_range.closed_;
+}
+
+std::string RangeType::Serialize() const {
+  rapidjson::Document document;
+  document.SetObject();
+  rapidjson::Document::AllocatorType& allocator = document.GetAllocator();
+
+  auto closed_str = ClosedToString(closed_);
+  rapidjson::Value closed_value(
+      closed_str.data(), static_cast<rapidjson::SizeType>(closed_str.size()), allocator);
+  document.AddMember(rapidjson::Value("closed", allocator), closed_value, allocator);
+
+  rapidjson::StringBuffer buffer;
+  rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
+  document.Accept(writer);
+  return buffer.GetString();
+}
+
+Result<std::shared_ptr<DataType>> RangeType::Deserialize(
+    std::shared_ptr<DataType> storage_type, const std::string& serialized_data) const {
+  // Validate storage type structure.
+  if (storage_type->id() != Type::STRUCT) {
+    return Status::Invalid("RangeType storage type must be a Struct, got ",
+                           storage_type->ToString());
+  }
+  const auto& struct_type = internal::checked_cast<const StructType&>(*storage_type);
+  if (struct_type.num_fields() != 2) {
+    return Status::Invalid("RangeType storage Struct must have exactly 2 fields, got ",
+                           struct_type.num_fields());
+  }
+  const auto& lower_field = struct_type.field(0);
+  const auto& upper_field = struct_type.field(1);
+  if (lower_field->name() != "lower") {
+    return Status::Invalid(
+        "RangeType storage Struct field 0 must be named \"lower\", got \"",
+        lower_field->name(), "\"");
+  }
+  if (upper_field->name() != "upper") {
+    return Status::Invalid(
+        "RangeType storage Struct field 1 must be named \"upper\", got \"",
+        upper_field->name(), "\"");
+  }
+  if (!lower_field->type()->Equals(*upper_field->type())) {
+    return Status::Invalid(
+        "RangeType storage Struct fields \"lower\" and \"upper\" must have the same "
+        "type, got \"",
+        lower_field->type()->ToString(), "\" and \"", upper_field->type()->ToString(),
+        "\"");
+  }
+
+  // Parse the required "closed" parameter from JSON metadata. The closedness
+  // is not defaulted on the wire: empty metadata or a missing key is invalid.
+  if (serialized_data.empty()) {
+    return Status::Invalid(
+        "RangeType metadata must be a JSON object with a required \"closed\" key, "
+        "got an empty string");
+  }
+  rapidjson::Document document;
+  const auto& parsed = document.Parse(serialized_data.data(), serialized_data.length());
+  if (parsed.HasParseError()) {
+    return Status::Invalid("Invalid serialized JSON data for RangeType: ",
+                           rapidjson::GetParseError_En(parsed.GetParseError()), ": ",
+                           serialized_data);
+  }
+  if (!document.IsObject()) {
+    return Status::Invalid("Invalid serialized JSON data for RangeType: not an object");
+  }
+  if (!document.HasMember("closed")) {
+    return Status::Invalid("RangeType metadata is missing the required \"closed\" key: ",
+                           serialized_data);
+  }
+  const auto& closed_val = document["closed"];
+  if (!closed_val.IsString()) {
+    return Status::Invalid(
+        "Invalid serialized JSON data for RangeType: \"closed\" is not a string");
+  }
+  ARROW_ASSIGN_OR_RAISE(RangeClosed closed,
+                        ClosedFromString(std::string_view(closed_val.GetString(),
+                                                          closed_val.GetStringLength())));
+
+  return std::make_shared<RangeType>(std::move(storage_type), closed);
+}
+
+std::shared_ptr<Array> RangeType::MakeArray(std::shared_ptr<ArrayData> data) const {
+  DCHECK_EQ(data->type->id(), Type::EXTENSION);
+  DCHECK_EQ("arrow.range",
+            internal::checked_cast<const ExtensionType&>(*data->type).extension_name());
+  return std::make_shared<RangeArray>(data);
+}
+
+Result<std::shared_ptr<DataType>> RangeType::Make(std::shared_ptr<DataType> value_type,
+                                                  RangeClosed closed,
+                                                  bool allow_unbounded) {
+  auto storage = MakeStorageType(value_type, allow_unbounded);
+  return std::make_shared<RangeType>(std::move(storage), closed);
+}
+
+// ---------------------------------------------------------------------------
+// Free factory function
+
+std::shared_ptr<DataType> range(std::shared_ptr<DataType> value_type, RangeClosed closed,
+                                bool allow_unbounded) {
+  auto result = RangeType::Make(std::move(value_type), closed, allow_unbounded);
+  ARROW_CHECK_OK(result.status());
+  return std::move(result).ValueOrDie();
+}
+
+// ---------------------------------------------------------------------------
+// RangeIncType

Review Comment:
   `Range` and `GranularRange`? Naming is hard.



-- 
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


Re: [PR] GH-50027: [Format][C++][Python] Add fixed and variable closedness range canonical extension types [arrow]

Posted by "Hoeze (via GitHub)" <gi...@apache.org>.
Hoeze commented on code in PR #50028:
URL: https://github.com/apache/arrow/pull/50028#discussion_r4177564327


##########
cpp/src/arrow/extension/range.h:
##########
@@ -0,0 +1,188 @@
+// 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.
+
+#pragma once
+
+#include "arrow/extension_type.h"
+#include "arrow/type.h"
+
+namespace arrow::extension {
+
+/// \brief Which bound(s) of an arrow.fixed_closedness_range interval are inclusive.
+///
+/// Null (infinite) bounds are always exclusive regardless of this value.
+enum class RangeClosed {
+  /// Lower bound is inclusive, upper bound is exclusive: [lower, upper)
+  Left,
+  /// Lower bound is exclusive, upper bound is inclusive: (lower, upper]
+  Right,
+  /// Both bounds are inclusive: [lower, upper]
+  Both,
+  /// Both bounds are exclusive: (lower, upper)
+  Neither,
+};
+
+/// \brief FixedClosednessRangeType represents a bounded set (mathematical interval) over
+/// an orderable Arrow type T.
+///
+/// Storage is a Struct with exactly two fields "lower" and "upper" of the same
+/// orderable type T. Each field may independently be nullable or not: a nullable
+/// bound can hold null to represent an unbounded (infinite) endpoint on that
+/// side, while a non-nullable bound is always finite.
+///   - "lower": T  (null, when nullable = unbounded below, i.e. -infinity)
+///   - "upper": T  (null, when nullable = unbounded above, i.e. +infinity)
+///
+/// The outer struct's validity bit marks a null/absent range.
+///
+/// The "closed" parameter controls which finite bounds are inclusive.
+/// Null (infinite) bounds are always treated as exclusive.
+class ARROW_EXPORT FixedClosednessRangeType : public ExtensionType {
+ public:
+  /// \brief Construct a FixedClosednessRangeType.
+  ///
+  /// \param[in] storage_type A two-field Struct type with nullable fields
+  ///   "lower" and "upper" of the same orderable Arrow type T.
+  /// \param[in] closed Which bound(s) are inclusive.
+  explicit FixedClosednessRangeType(std::shared_ptr<DataType> storage_type,
+                                    RangeClosed closed)
+      : ExtensionType(std::move(storage_type)), closed_(closed) {}
+
+  std::string extension_name() const override { return "arrow.fixed_closedness_range"; }

Review Comment:
   The names changed on purpose. The dev mailing-list discussion moved from one `arrow.range` type to the fixed/variable closedness pair, and #50027 now describes this design.



##########
docs/source/format/CanonicalExtensions.rst:
##########
@@ -573,6 +573,216 @@ This extension type is intended to be compatible with ANSI SQL's ``TIMESTAMP WIT
 
    It is also *permissible* for the ``offset_minutes`` field to be dictionary-encoded or run-end-encoded.
 
+.. _fixed_closedness_range_extension:
+
+Fixed closedness range
+======================
+
+Fixed closedness range represents a bounded set (mathematical interval)
+defined by a lower and an upper bound over an orderable Arrow type T.  Its
+closedness is a type parameter shared by all values.  It matches PostgreSQL's
+discrete `range types`_ (such as ``int4range`` and ``daterange``) and SQL:2011
+``PERIOD`` types.  Ranges whose closedness differs per value use
+:ref:`variable closedness range <variable_closedness_range_extension>`
+instead.
+
+.. note::
+
+   **Disambiguation from Arrow's calendar** ``Interval`` **type.**
+   Arrow already has an ``Interval`` type (``INTERVAL_MONTHS``,
+   ``INTERVAL_DAY_TIME``, ``INTERVAL_MONTH_DAY_NANO``) that represents a
+   *duration*: a signed difference between two points in time.  The
+   ``arrow.fixed_closedness_range`` and ``arrow.variable_closedness_range``
+   extension types are an entirely different concept: they represent a
+   *bounded set* with explicit lower and upper endpoints, analogous to a
+   closed or open interval in mathematics.  The naming follows database
+   convention: SQL uses ``INTERVAL`` for durations and ``RANGE`` (or
+   ``PERIOD``) for bounded sets.
+
+* Extension name: ``arrow.fixed_closedness_range``.

Review Comment:
   The two types and the per-value `lower_inc`/`upper_inc` storage come from the dev mailing-list discussion. #50027 was outdated and now describes this design.



-- 
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


Re: [PR] GH-50027: [Format][C++][Python] Add arrow.range canonical extension type [arrow]

Posted by "Hoeze (via GitHub)" <gi...@apache.org>.
Hoeze commented on code in PR #50028:
URL: https://github.com/apache/arrow/pull/50028#discussion_r3359240061


##########
cpp/src/arrow/extension/range.cc:
##########
@@ -0,0 +1,345 @@
+// 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.
+
+#include "arrow/extension/range.h"
+
+#include <sstream>
+#include <string_view>
+
+#include "arrow/json/rapidjson_defs.h"  // IWYU pragma: keep
+#include "arrow/util/logging_internal.h"
+
+#include <rapidjson/document.h>
+#include <rapidjson/error/en.h>
+#include <rapidjson/writer.h>
+
+namespace arrow::extension {
+
+namespace {
+
+/// Map RangeClosed -> the JSON string value used in serialization.
+std::string_view ClosedToString(RangeClosed closed) {
+  switch (closed) {
+    case RangeClosed::Left:
+      return "left";
+    case RangeClosed::Right:
+      return "right";
+    case RangeClosed::Both:
+      return "both";
+    case RangeClosed::Neither:
+      return "neither";
+  }
+  // unreachable
+  return "right";
+}
+
+/// Parse the JSON "closed" string into a RangeClosed enum.
+/// Returns an error if the string is not one of the four valid values.
+Result<RangeClosed> ClosedFromString(std::string_view s) {
+  if (s == "left") return RangeClosed::Left;
+  if (s == "right") return RangeClosed::Right;
+  if (s == "both") return RangeClosed::Both;
+  if (s == "neither") return RangeClosed::Neither;
+  return Status::Invalid(
+      "Invalid value for RangeType \"closed\" parameter: \"", s,
+      "\". Expected one of: \"left\", \"right\", \"both\", \"neither\".");
+}
+
+/// Build the storage Struct type for a given value subtype.
+std::shared_ptr<DataType> MakeStorageType(const std::shared_ptr<DataType>& value_type,
+                                          bool allow_unbounded) {
+  // Nullable bounds can represent an unbounded (infinite) endpoint; non-nullable
+  // bounds are always finite.
+  return struct_({field("lower", value_type, allow_unbounded),
+                  field("upper", value_type, allow_unbounded)});
+}
+
+}  // namespace
+
+// ---------------------------------------------------------------------------
+// RangeType
+
+std::shared_ptr<DataType> RangeType::value_type() const {
+  // storage_type() is a struct with two fields; both share the same type.
+  return internal::checked_cast<const StructType&>(*storage_type()).field(0)->type();
+}
+
+std::string RangeType::ToString(bool show_metadata) const {
+  std::stringstream ss;
+  ss << "extension<" << this->extension_name()
+     << "[value_type=" << value_type()->ToString(show_metadata)
+     << ", closed=" << ClosedToString(closed_) << "]>";
+  return ss.str();
+}
+
+bool RangeType::ExtensionEquals(const ExtensionType& other) const {
+  if (extension_name() != other.extension_name()) {
+    return false;
+  }
+  const auto& other_range = internal::checked_cast<const RangeType&>(other);
+  return storage_type()->Equals(*other_range.storage_type()) &&
+         closed_ == other_range.closed_;
+}
+
+std::string RangeType::Serialize() const {
+  rapidjson::Document document;
+  document.SetObject();
+  rapidjson::Document::AllocatorType& allocator = document.GetAllocator();
+
+  auto closed_str = ClosedToString(closed_);
+  rapidjson::Value closed_value(
+      closed_str.data(), static_cast<rapidjson::SizeType>(closed_str.size()), allocator);
+  document.AddMember(rapidjson::Value("closed", allocator), closed_value, allocator);
+
+  rapidjson::StringBuffer buffer;
+  rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
+  document.Accept(writer);
+  return buffer.GetString();
+}
+
+Result<std::shared_ptr<DataType>> RangeType::Deserialize(
+    std::shared_ptr<DataType> storage_type, const std::string& serialized_data) const {
+  // Validate storage type structure.
+  if (storage_type->id() != Type::STRUCT) {
+    return Status::Invalid("RangeType storage type must be a Struct, got ",
+                           storage_type->ToString());
+  }
+  const auto& struct_type = internal::checked_cast<const StructType&>(*storage_type);
+  if (struct_type.num_fields() != 2) {
+    return Status::Invalid("RangeType storage Struct must have exactly 2 fields, got ",
+                           struct_type.num_fields());
+  }
+  const auto& lower_field = struct_type.field(0);
+  const auto& upper_field = struct_type.field(1);
+  if (lower_field->name() != "lower") {
+    return Status::Invalid(
+        "RangeType storage Struct field 0 must be named \"lower\", got \"",
+        lower_field->name(), "\"");
+  }
+  if (upper_field->name() != "upper") {
+    return Status::Invalid(
+        "RangeType storage Struct field 1 must be named \"upper\", got \"",
+        upper_field->name(), "\"");
+  }
+  if (!lower_field->type()->Equals(*upper_field->type())) {
+    return Status::Invalid(
+        "RangeType storage Struct fields \"lower\" and \"upper\" must have the same "
+        "type, got \"",
+        lower_field->type()->ToString(), "\" and \"", upper_field->type()->ToString(),
+        "\"");
+  }
+
+  // Parse the required "closed" parameter from JSON metadata. The closedness
+  // is not defaulted on the wire: empty metadata or a missing key is invalid.
+  if (serialized_data.empty()) {
+    return Status::Invalid(
+        "RangeType metadata must be a JSON object with a required \"closed\" key, "
+        "got an empty string");
+  }
+  rapidjson::Document document;
+  const auto& parsed = document.Parse(serialized_data.data(), serialized_data.length());
+  if (parsed.HasParseError()) {
+    return Status::Invalid("Invalid serialized JSON data for RangeType: ",
+                           rapidjson::GetParseError_En(parsed.GetParseError()), ": ",
+                           serialized_data);
+  }
+  if (!document.IsObject()) {
+    return Status::Invalid("Invalid serialized JSON data for RangeType: not an object");
+  }
+  if (!document.HasMember("closed")) {
+    return Status::Invalid("RangeType metadata is missing the required \"closed\" key: ",
+                           serialized_data);
+  }
+  const auto& closed_val = document["closed"];
+  if (!closed_val.IsString()) {
+    return Status::Invalid(
+        "Invalid serialized JSON data for RangeType: \"closed\" is not a string");
+  }
+  ARROW_ASSIGN_OR_RAISE(RangeClosed closed,
+                        ClosedFromString(std::string_view(closed_val.GetString(),
+                                                          closed_val.GetStringLength())));
+
+  return std::make_shared<RangeType>(std::move(storage_type), closed);
+}
+
+std::shared_ptr<Array> RangeType::MakeArray(std::shared_ptr<ArrayData> data) const {
+  DCHECK_EQ(data->type->id(), Type::EXTENSION);
+  DCHECK_EQ("arrow.range",
+            internal::checked_cast<const ExtensionType&>(*data->type).extension_name());
+  return std::make_shared<RangeArray>(data);
+}
+
+Result<std::shared_ptr<DataType>> RangeType::Make(std::shared_ptr<DataType> value_type,
+                                                  RangeClosed closed,
+                                                  bool allow_unbounded) {
+  auto storage = MakeStorageType(value_type, allow_unbounded);
+  return std::make_shared<RangeType>(std::move(storage), closed);
+}
+
+// ---------------------------------------------------------------------------
+// Free factory function
+
+std::shared_ptr<DataType> range(std::shared_ptr<DataType> value_type, RangeClosed closed,
+                                bool allow_unbounded) {
+  auto result = RangeType::Make(std::move(value_type), closed, allow_unbounded);
+  ARROW_CHECK_OK(result.status());
+  return std::move(result).ValueOrDie();
+}
+
+// ---------------------------------------------------------------------------
+// RangeIncType

Review Comment:
   One more idea:
   `FixedRangeType` and `VariableRangeType` (similar to the tensor type naming



-- 
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


Re: [PR] GH-50027: [Format][C++][Python] Add fixed and variable closedness range canonical extension types [arrow]

Posted by "Hoeze (via GitHub)" <gi...@apache.org>.
Hoeze commented on PR #50028:
URL: https://github.com/apache/arrow/pull/50028#issuecomment-5979921270

   Hi all, could a maintainer please approve the CI workflows for this PR? So far only the bot checks have run.


-- 
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


Re: [PR] GH-50027: [Format][C++][Python] Add fixed and variable closedness range canonical extension types [arrow]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #50028:
URL: https://github.com/apache/arrow/pull/50028#discussion_r4065547599


##########
cpp/src/arrow/extension/range.h:
##########
@@ -0,0 +1,188 @@
+// 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.
+
+#pragma once
+
+#include "arrow/extension_type.h"
+#include "arrow/type.h"
+
+namespace arrow::extension {
+
+/// \brief Which bound(s) of an arrow.fixed_closedness_range interval are inclusive.
+///
+/// Null (infinite) bounds are always exclusive regardless of this value.
+enum class RangeClosed {
+  /// Lower bound is inclusive, upper bound is exclusive: [lower, upper)
+  Left,
+  /// Lower bound is exclusive, upper bound is inclusive: (lower, upper]
+  Right,
+  /// Both bounds are inclusive: [lower, upper]
+  Both,
+  /// Both bounds are exclusive: (lower, upper)
+  Neither,
+};
+
+/// \brief FixedClosednessRangeType represents a bounded set (mathematical interval) over
+/// an orderable Arrow type T.
+///
+/// Storage is a Struct with exactly two fields "lower" and "upper" of the same
+/// orderable type T. Each field may independently be nullable or not: a nullable
+/// bound can hold null to represent an unbounded (infinite) endpoint on that
+/// side, while a non-nullable bound is always finite.
+///   - "lower": T  (null, when nullable = unbounded below, i.e. -infinity)
+///   - "upper": T  (null, when nullable = unbounded above, i.e. +infinity)
+///
+/// The outer struct's validity bit marks a null/absent range.
+///
+/// The "closed" parameter controls which finite bounds are inclusive.
+/// Null (infinite) bounds are always treated as exclusive.
+class ARROW_EXPORT FixedClosednessRangeType : public ExtensionType {
+ public:
+  /// \brief Construct a FixedClosednessRangeType.
+  ///
+  /// \param[in] storage_type A two-field Struct type with nullable fields
+  ///   "lower" and "upper" of the same orderable Arrow type T.
+  /// \param[in] closed Which bound(s) are inclusive.
+  explicit FixedClosednessRangeType(std::shared_ptr<DataType> storage_type,
+                                    RangeClosed closed)
+      : ExtensionType(std::move(storage_type)), closed_(closed) {}
+
+  std::string extension_name() const override { return "arrow.fixed_closedness_range"; }

Review Comment:
   GH-50027 specifies the canonical wire extension name `arrow.range`, but this implementation publishes `arrow.fixed_closedness_range` (and the companion variable name). IPC consumers dispatch on `extension_name`, so these names are not interoperable with the requested canonical type; please resolve the name/design against the issue before standardizing the format.



##########
docs/source/format/CanonicalExtensions.rst:
##########
@@ -573,6 +573,216 @@ This extension type is intended to be compatible with ANSI SQL's ``TIMESTAMP WIT
 
    It is also *permissible* for the ``offset_minutes`` field to be dictionary-encoded or run-end-encoded.
 
+.. _fixed_closedness_range_extension:
+
+Fixed closedness range
+======================
+
+Fixed closedness range represents a bounded set (mathematical interval)
+defined by a lower and an upper bound over an orderable Arrow type T.  Its
+closedness is a type parameter shared by all values.  It matches PostgreSQL's
+discrete `range types`_ (such as ``int4range`` and ``daterange``) and SQL:2011
+``PERIOD`` types.  Ranges whose closedness differs per value use
+:ref:`variable closedness range <variable_closedness_range_extension>`
+instead.
+
+.. note::
+
+   **Disambiguation from Arrow's calendar** ``Interval`` **type.**
+   Arrow already has an ``Interval`` type (``INTERVAL_MONTHS``,
+   ``INTERVAL_DAY_TIME``, ``INTERVAL_MONTH_DAY_NANO``) that represents a
+   *duration*: a signed difference between two points in time.  The
+   ``arrow.fixed_closedness_range`` and ``arrow.variable_closedness_range``
+   extension types are an entirely different concept: they represent a
+   *bounded set* with explicit lower and upper endpoints, analogous to a
+   closed or open interval in mathematics.  The naming follows database
+   convention: SQL uses ``INTERVAL`` for durations and ``RANGE`` (or
+   ``PERIOD``) for bounded sets.
+
+* Extension name: ``arrow.fixed_closedness_range``.

Review Comment:
   The linked issue #50027 specifies the wire name `arrow.range` and a single `Struct<lower, upper>` type with required type-level `closed` metadata. This change instead publishes two different extension names and a per-value `lower_inc`/`upper_inc` storage contract, so consumers implementing the issue's format will not recognize or interoperate with these types. Please reconcile the canonical wire contract with the linked requirements before merging.



##########
python/pyarrow/types.pxi:
##########
@@ -2088,6 +2088,107 @@ cdef class Bool8Type(BaseExtensionType):
         return Bool8Scalar
 
 
+cdef class FixedClosednessRangeType(BaseExtensionType):
+    """
+    Concrete class for fixed closedness range extension type.
+
+    A fixed closedness range represents a bounded set (a mathematical interval)
+    over an orderable Arrow value type. The underlying storage is a Struct with
+    two fields "lower" and "upper" of the value type, each optionally nullable;
+    when a bound field is nullable, a null value denotes an unbounded (infinite)
+    side. The "closed" parameter controls which finite bounds are inclusive.
+
+    Examples
+    --------
+    Create an instance of fixed closedness range extension type:
+
+    >>> import pyarrow as pa
+    >>> pa.fixed_closedness_range(pa.int32(), "both")
+    FixedClosednessRangeType(extension<arrow.fixed_closedness_range[value_type=int32, closed=both]>)
+    """
+
+    cdef void init(self, const shared_ptr[CDataType]& type) except *:
+        BaseExtensionType.init(self, type)
+        self.range_ext_type = <const CFixedClosednessRangeType*> type.get()
+
+    @property
+    def value_type(self):
+        """
+        The Arrow value type of the "lower" and "upper" bounds.
+        """
+        return pyarrow_wrap_data_type(self.range_ext_type.value_type())
+
+    @property
+    def closed(self):
+        """
+        Which bound(s) are inclusive, as one of "left", "right", "both" or
+        "neither".
+        """
+        cdef CRangeClosed c_closed = self.range_ext_type.closed()
+        if c_closed == CRangeClosed.Left:
+            return "left"
+        elif c_closed == CRangeClosed.Right:
+            return "right"
+        elif c_closed == CRangeClosed.Both:
+            return "both"
+        else:
+            return "neither"
+
+    def __arrow_ext_class__(self):
+        return FixedClosednessRangeArray
+
+    def __reduce__(self):
+        return fixed_closedness_range, (self.value_type, self.closed)

Review Comment:
   `allow_unbounded` changes the storage nullability and is therefore part of this type's identity, but it is omitted from `__reduce__`. Pickling `fixed_closedness_range(..., allow_unbounded=False)` reconstructs the default nullable type (and cannot preserve independently nullable bounds accepted by C++ deserialization). Preserve the storage nullability in the pickle reconstruction and add coverage for finite-only types.



-- 
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


Re: [PR] GH-50027: [Format][C++][Python] Add arrow.range canonical extension type [arrow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #50028:
URL: https://github.com/apache/arrow/pull/50028#issuecomment-4528893053

   :warning: GitHub issue #50027 **has been automatically assigned in GitHub** to PR creator.


-- 
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


Re: [PR] GH-50027: [Format][C++][Python] Add arrow.range canonical extension type [arrow]

Posted by "rok (via GitHub)" <gi...@apache.org>.
rok commented on code in PR #50028:
URL: https://github.com/apache/arrow/pull/50028#discussion_r3359048766


##########
cpp/src/arrow/extension/range.cc:
##########
@@ -0,0 +1,345 @@
+// 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.
+
+#include "arrow/extension/range.h"
+
+#include <sstream>
+#include <string_view>
+
+#include "arrow/json/rapidjson_defs.h"  // IWYU pragma: keep
+#include "arrow/util/logging_internal.h"
+
+#include <rapidjson/document.h>
+#include <rapidjson/error/en.h>
+#include <rapidjson/writer.h>
+
+namespace arrow::extension {
+
+namespace {
+
+/// Map RangeClosed -> the JSON string value used in serialization.
+std::string_view ClosedToString(RangeClosed closed) {
+  switch (closed) {
+    case RangeClosed::Left:
+      return "left";
+    case RangeClosed::Right:
+      return "right";
+    case RangeClosed::Both:
+      return "both";
+    case RangeClosed::Neither:
+      return "neither";
+  }
+  // unreachable
+  return "right";
+}
+
+/// Parse the JSON "closed" string into a RangeClosed enum.
+/// Returns an error if the string is not one of the four valid values.
+Result<RangeClosed> ClosedFromString(std::string_view s) {
+  if (s == "left") return RangeClosed::Left;
+  if (s == "right") return RangeClosed::Right;
+  if (s == "both") return RangeClosed::Both;
+  if (s == "neither") return RangeClosed::Neither;
+  return Status::Invalid(
+      "Invalid value for RangeType \"closed\" parameter: \"", s,
+      "\". Expected one of: \"left\", \"right\", \"both\", \"neither\".");
+}
+
+/// Build the storage Struct type for a given value subtype.
+std::shared_ptr<DataType> MakeStorageType(const std::shared_ptr<DataType>& value_type,
+                                          bool allow_unbounded) {
+  // Nullable bounds can represent an unbounded (infinite) endpoint; non-nullable
+  // bounds are always finite.
+  return struct_({field("lower", value_type, allow_unbounded),
+                  field("upper", value_type, allow_unbounded)});
+}
+
+}  // namespace
+
+// ---------------------------------------------------------------------------
+// RangeType
+
+std::shared_ptr<DataType> RangeType::value_type() const {
+  // storage_type() is a struct with two fields; both share the same type.
+  return internal::checked_cast<const StructType&>(*storage_type()).field(0)->type();
+}
+
+std::string RangeType::ToString(bool show_metadata) const {
+  std::stringstream ss;
+  ss << "extension<" << this->extension_name()
+     << "[value_type=" << value_type()->ToString(show_metadata)
+     << ", closed=" << ClosedToString(closed_) << "]>";
+  return ss.str();
+}
+
+bool RangeType::ExtensionEquals(const ExtensionType& other) const {
+  if (extension_name() != other.extension_name()) {
+    return false;
+  }
+  const auto& other_range = internal::checked_cast<const RangeType&>(other);
+  return storage_type()->Equals(*other_range.storage_type()) &&
+         closed_ == other_range.closed_;
+}
+
+std::string RangeType::Serialize() const {
+  rapidjson::Document document;
+  document.SetObject();
+  rapidjson::Document::AllocatorType& allocator = document.GetAllocator();
+
+  auto closed_str = ClosedToString(closed_);
+  rapidjson::Value closed_value(
+      closed_str.data(), static_cast<rapidjson::SizeType>(closed_str.size()), allocator);
+  document.AddMember(rapidjson::Value("closed", allocator), closed_value, allocator);
+
+  rapidjson::StringBuffer buffer;
+  rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
+  document.Accept(writer);
+  return buffer.GetString();
+}
+
+Result<std::shared_ptr<DataType>> RangeType::Deserialize(
+    std::shared_ptr<DataType> storage_type, const std::string& serialized_data) const {
+  // Validate storage type structure.
+  if (storage_type->id() != Type::STRUCT) {
+    return Status::Invalid("RangeType storage type must be a Struct, got ",
+                           storage_type->ToString());
+  }
+  const auto& struct_type = internal::checked_cast<const StructType&>(*storage_type);
+  if (struct_type.num_fields() != 2) {
+    return Status::Invalid("RangeType storage Struct must have exactly 2 fields, got ",
+                           struct_type.num_fields());
+  }
+  const auto& lower_field = struct_type.field(0);
+  const auto& upper_field = struct_type.field(1);
+  if (lower_field->name() != "lower") {
+    return Status::Invalid(
+        "RangeType storage Struct field 0 must be named \"lower\", got \"",
+        lower_field->name(), "\"");
+  }
+  if (upper_field->name() != "upper") {
+    return Status::Invalid(
+        "RangeType storage Struct field 1 must be named \"upper\", got \"",
+        upper_field->name(), "\"");
+  }
+  if (!lower_field->type()->Equals(*upper_field->type())) {
+    return Status::Invalid(
+        "RangeType storage Struct fields \"lower\" and \"upper\" must have the same "
+        "type, got \"",
+        lower_field->type()->ToString(), "\" and \"", upper_field->type()->ToString(),
+        "\"");
+  }
+
+  // Parse the required "closed" parameter from JSON metadata. The closedness
+  // is not defaulted on the wire: empty metadata or a missing key is invalid.
+  if (serialized_data.empty()) {
+    return Status::Invalid(
+        "RangeType metadata must be a JSON object with a required \"closed\" key, "
+        "got an empty string");
+  }
+  rapidjson::Document document;
+  const auto& parsed = document.Parse(serialized_data.data(), serialized_data.length());
+  if (parsed.HasParseError()) {
+    return Status::Invalid("Invalid serialized JSON data for RangeType: ",
+                           rapidjson::GetParseError_En(parsed.GetParseError()), ": ",
+                           serialized_data);
+  }
+  if (!document.IsObject()) {
+    return Status::Invalid("Invalid serialized JSON data for RangeType: not an object");
+  }
+  if (!document.HasMember("closed")) {
+    return Status::Invalid("RangeType metadata is missing the required \"closed\" key: ",
+                           serialized_data);
+  }
+  const auto& closed_val = document["closed"];
+  if (!closed_val.IsString()) {
+    return Status::Invalid(
+        "Invalid serialized JSON data for RangeType: \"closed\" is not a string");
+  }
+  ARROW_ASSIGN_OR_RAISE(RangeClosed closed,
+                        ClosedFromString(std::string_view(closed_val.GetString(),
+                                                          closed_val.GetStringLength())));
+
+  return std::make_shared<RangeType>(std::move(storage_type), closed);
+}
+
+std::shared_ptr<Array> RangeType::MakeArray(std::shared_ptr<ArrayData> data) const {
+  DCHECK_EQ(data->type->id(), Type::EXTENSION);
+  DCHECK_EQ("arrow.range",
+            internal::checked_cast<const ExtensionType&>(*data->type).extension_name());
+  return std::make_shared<RangeArray>(data);
+}
+
+Result<std::shared_ptr<DataType>> RangeType::Make(std::shared_ptr<DataType> value_type,
+                                                  RangeClosed closed,
+                                                  bool allow_unbounded) {
+  auto storage = MakeStorageType(value_type, allow_unbounded);
+  return std::make_shared<RangeType>(std::move(storage), closed);
+}
+
+// ---------------------------------------------------------------------------
+// Free factory function
+
+std::shared_ptr<DataType> range(std::shared_ptr<DataType> value_type, RangeClosed closed,
+                                bool allow_unbounded) {
+  auto result = RangeType::Make(std::move(value_type), closed, allow_unbounded);
+  ARROW_CHECK_OK(result.status());
+  return std::move(result).ValueOrDie();
+}
+
+// ---------------------------------------------------------------------------
+// RangeIncType

Review Comment:
   Perhaps wwe can find a better name than `RangeInc`?



-- 
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


Re: [PR] GH-50027: [Format][C++][Python] Add arrow.range canonical extension type [arrow]

Posted by "Hoeze (via GitHub)" <gi...@apache.org>.
Hoeze commented on code in PR #50028:
URL: https://github.com/apache/arrow/pull/50028#discussion_r3359240061


##########
cpp/src/arrow/extension/range.cc:
##########
@@ -0,0 +1,345 @@
+// 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.
+
+#include "arrow/extension/range.h"
+
+#include <sstream>
+#include <string_view>
+
+#include "arrow/json/rapidjson_defs.h"  // IWYU pragma: keep
+#include "arrow/util/logging_internal.h"
+
+#include <rapidjson/document.h>
+#include <rapidjson/error/en.h>
+#include <rapidjson/writer.h>
+
+namespace arrow::extension {
+
+namespace {
+
+/// Map RangeClosed -> the JSON string value used in serialization.
+std::string_view ClosedToString(RangeClosed closed) {
+  switch (closed) {
+    case RangeClosed::Left:
+      return "left";
+    case RangeClosed::Right:
+      return "right";
+    case RangeClosed::Both:
+      return "both";
+    case RangeClosed::Neither:
+      return "neither";
+  }
+  // unreachable
+  return "right";
+}
+
+/// Parse the JSON "closed" string into a RangeClosed enum.
+/// Returns an error if the string is not one of the four valid values.
+Result<RangeClosed> ClosedFromString(std::string_view s) {
+  if (s == "left") return RangeClosed::Left;
+  if (s == "right") return RangeClosed::Right;
+  if (s == "both") return RangeClosed::Both;
+  if (s == "neither") return RangeClosed::Neither;
+  return Status::Invalid(
+      "Invalid value for RangeType \"closed\" parameter: \"", s,
+      "\". Expected one of: \"left\", \"right\", \"both\", \"neither\".");
+}
+
+/// Build the storage Struct type for a given value subtype.
+std::shared_ptr<DataType> MakeStorageType(const std::shared_ptr<DataType>& value_type,
+                                          bool allow_unbounded) {
+  // Nullable bounds can represent an unbounded (infinite) endpoint; non-nullable
+  // bounds are always finite.
+  return struct_({field("lower", value_type, allow_unbounded),
+                  field("upper", value_type, allow_unbounded)});
+}
+
+}  // namespace
+
+// ---------------------------------------------------------------------------
+// RangeType
+
+std::shared_ptr<DataType> RangeType::value_type() const {
+  // storage_type() is a struct with two fields; both share the same type.
+  return internal::checked_cast<const StructType&>(*storage_type()).field(0)->type();
+}
+
+std::string RangeType::ToString(bool show_metadata) const {
+  std::stringstream ss;
+  ss << "extension<" << this->extension_name()
+     << "[value_type=" << value_type()->ToString(show_metadata)
+     << ", closed=" << ClosedToString(closed_) << "]>";
+  return ss.str();
+}
+
+bool RangeType::ExtensionEquals(const ExtensionType& other) const {
+  if (extension_name() != other.extension_name()) {
+    return false;
+  }
+  const auto& other_range = internal::checked_cast<const RangeType&>(other);
+  return storage_type()->Equals(*other_range.storage_type()) &&
+         closed_ == other_range.closed_;
+}
+
+std::string RangeType::Serialize() const {
+  rapidjson::Document document;
+  document.SetObject();
+  rapidjson::Document::AllocatorType& allocator = document.GetAllocator();
+
+  auto closed_str = ClosedToString(closed_);
+  rapidjson::Value closed_value(
+      closed_str.data(), static_cast<rapidjson::SizeType>(closed_str.size()), allocator);
+  document.AddMember(rapidjson::Value("closed", allocator), closed_value, allocator);
+
+  rapidjson::StringBuffer buffer;
+  rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
+  document.Accept(writer);
+  return buffer.GetString();
+}
+
+Result<std::shared_ptr<DataType>> RangeType::Deserialize(
+    std::shared_ptr<DataType> storage_type, const std::string& serialized_data) const {
+  // Validate storage type structure.
+  if (storage_type->id() != Type::STRUCT) {
+    return Status::Invalid("RangeType storage type must be a Struct, got ",
+                           storage_type->ToString());
+  }
+  const auto& struct_type = internal::checked_cast<const StructType&>(*storage_type);
+  if (struct_type.num_fields() != 2) {
+    return Status::Invalid("RangeType storage Struct must have exactly 2 fields, got ",
+                           struct_type.num_fields());
+  }
+  const auto& lower_field = struct_type.field(0);
+  const auto& upper_field = struct_type.field(1);
+  if (lower_field->name() != "lower") {
+    return Status::Invalid(
+        "RangeType storage Struct field 0 must be named \"lower\", got \"",
+        lower_field->name(), "\"");
+  }
+  if (upper_field->name() != "upper") {
+    return Status::Invalid(
+        "RangeType storage Struct field 1 must be named \"upper\", got \"",
+        upper_field->name(), "\"");
+  }
+  if (!lower_field->type()->Equals(*upper_field->type())) {
+    return Status::Invalid(
+        "RangeType storage Struct fields \"lower\" and \"upper\" must have the same "
+        "type, got \"",
+        lower_field->type()->ToString(), "\" and \"", upper_field->type()->ToString(),
+        "\"");
+  }
+
+  // Parse the required "closed" parameter from JSON metadata. The closedness
+  // is not defaulted on the wire: empty metadata or a missing key is invalid.
+  if (serialized_data.empty()) {
+    return Status::Invalid(
+        "RangeType metadata must be a JSON object with a required \"closed\" key, "
+        "got an empty string");
+  }
+  rapidjson::Document document;
+  const auto& parsed = document.Parse(serialized_data.data(), serialized_data.length());
+  if (parsed.HasParseError()) {
+    return Status::Invalid("Invalid serialized JSON data for RangeType: ",
+                           rapidjson::GetParseError_En(parsed.GetParseError()), ": ",
+                           serialized_data);
+  }
+  if (!document.IsObject()) {
+    return Status::Invalid("Invalid serialized JSON data for RangeType: not an object");
+  }
+  if (!document.HasMember("closed")) {
+    return Status::Invalid("RangeType metadata is missing the required \"closed\" key: ",
+                           serialized_data);
+  }
+  const auto& closed_val = document["closed"];
+  if (!closed_val.IsString()) {
+    return Status::Invalid(
+        "Invalid serialized JSON data for RangeType: \"closed\" is not a string");
+  }
+  ARROW_ASSIGN_OR_RAISE(RangeClosed closed,
+                        ClosedFromString(std::string_view(closed_val.GetString(),
+                                                          closed_val.GetStringLength())));
+
+  return std::make_shared<RangeType>(std::move(storage_type), closed);
+}
+
+std::shared_ptr<Array> RangeType::MakeArray(std::shared_ptr<ArrayData> data) const {
+  DCHECK_EQ(data->type->id(), Type::EXTENSION);
+  DCHECK_EQ("arrow.range",
+            internal::checked_cast<const ExtensionType&>(*data->type).extension_name());
+  return std::make_shared<RangeArray>(data);
+}
+
+Result<std::shared_ptr<DataType>> RangeType::Make(std::shared_ptr<DataType> value_type,
+                                                  RangeClosed closed,
+                                                  bool allow_unbounded) {
+  auto storage = MakeStorageType(value_type, allow_unbounded);
+  return std::make_shared<RangeType>(std::move(storage), closed);
+}
+
+// ---------------------------------------------------------------------------
+// Free factory function
+
+std::shared_ptr<DataType> range(std::shared_ptr<DataType> value_type, RangeClosed closed,
+                                bool allow_unbounded) {
+  auto result = RangeType::Make(std::move(value_type), closed, allow_unbounded);
+  ARROW_CHECK_OK(result.status());
+  return std::move(result).ValueOrDie();
+}
+
+// ---------------------------------------------------------------------------
+// RangeIncType

Review Comment:
   One more idea:
   `FixedClosednessRangeType` and `VariableClosednessRangeType` (similar to the tensor type naming



-- 
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