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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
