Skip to content

Commit ffcfd4d

Browse files
committed
fix(python): keep bound nullability when pickling range types
__reduce__ rebuilt both range types from their parameters only, so a type with non-nullable bounds came back nullable after unpickling. Rebuild them from the storage type through the C++ Deserialize instead. This also keeps a type where only one bound is nullable, and tests cover both cases.
1 parent a3c9f4d commit ffcfd4d

3 files changed

Lines changed: 80 additions & 4 deletions

File tree

‎python/pyarrow/includes/libarrow.pxd‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3160,6 +3160,10 @@ cdef extern from "arrow/extension/range.h" namespace "arrow::extension" nogil:
31603160
CRangeClosed closed,
31613161
c_bool allow_unbounded)
31623162

3163+
CResult[shared_ptr[CDataType]] Deserialize(
3164+
shared_ptr[CDataType] storage_type,
3165+
const c_string& serialized_data) const
3166+
31633167
CRangeClosed closed()
31643168
shared_ptr[CDataType] value_type()
31653169

@@ -3174,6 +3178,10 @@ cdef extern from "arrow/extension/range.h" namespace "arrow::extension" nogil:
31743178
CResult[shared_ptr[CDataType]] Make(shared_ptr[CDataType] value_type,
31753179
c_bool allow_unbounded)
31763180

3181+
CResult[shared_ptr[CDataType]] Deserialize(
3182+
shared_ptr[CDataType] storage_type,
3183+
const c_string& serialized_data) const
3184+
31773185
shared_ptr[CDataType] value_type()
31783186

31793187
cdef cppclass CVariableClosednessRangeArray \

‎python/pyarrow/tests/test_extension_type.py‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2147,7 +2147,7 @@ def test_fixed_closedness_range_type_invalid_closed():
21472147
pa.fixed_closedness_range(pa.int32(), "")
21482148

21492149

2150-
def test_fixed_closedness_range_type_allow_unbounded():
2150+
def test_fixed_closedness_range_type_allow_unbounded(pickle_module):
21512151
# Default: bounds are nullable (can represent an unbounded / infinite side).
21522152
nullable = pa.fixed_closedness_range(pa.int32(), "both")
21532153
assert nullable.storage_type.field("lower").nullable
@@ -2163,12 +2163,28 @@ def test_fixed_closedness_range_type_allow_unbounded():
21632163
# Distinct types: storage nullability differs.
21642164
assert finite != nullable
21652165

2166+
# Pickling keeps the storage nullability.
2167+
assert pickle_module.loads(pickle_module.dumps(finite)) == finite
2168+
21662169
# A non-nullable-bounds range round-trips through its storage.
21672170
storage = pa.array([{"lower": 1, "upper": 5}], finite.storage_type)
21682171
arr = pa.ExtensionArray.from_storage(finite, storage)
21692172
assert arr.type == finite
21702173

21712174

2175+
def test_fixed_closedness_range_type_from_storage(pickle_module):
2176+
# C++ accepts one nullable and one non-nullable bound; pickling keeps it.
2177+
storage = pa.struct([pa.field("lower", pa.int32(), nullable=True),
2178+
pa.field("upper", pa.int32(), nullable=False)])
2179+
range_type = pa.lib._fixed_closedness_range_from_storage(storage, "right")
2180+
assert range_type.storage_type == storage
2181+
assert range_type.closed == "right"
2182+
assert pickle_module.loads(pickle_module.dumps(range_type)) == range_type
2183+
2184+
with pytest.raises(pa.ArrowInvalid, match="must be a Struct"):
2185+
pa.lib._fixed_closedness_range_from_storage(pa.int32(), "left")
2186+
2187+
21722188
@pytest.mark.parametrize("value_type,rows", [
21732189
(pa.int32(), [
21742190
{"lower": 1, "upper": 5, "lower_inc": True, "upper_inc": False},
@@ -2235,7 +2251,7 @@ def test_variable_closedness_range_type(pickle_module, value_type, rows):
22352251
assert inner == storage
22362252

22372253

2238-
def test_variable_closedness_range_type_allow_unbounded():
2254+
def test_variable_closedness_range_type_allow_unbounded(pickle_module):
22392255
# Default: bounds are nullable (can represent an unbounded / infinite side).
22402256
nullable = pa.variable_closedness_range(pa.int32())
22412257
assert nullable.storage_type.field("lower").nullable
@@ -2256,6 +2272,9 @@ def test_variable_closedness_range_type_allow_unbounded():
22562272
# Distinct types: storage nullability differs.
22572273
assert finite != nullable
22582274

2275+
# Pickling keeps the storage nullability.
2276+
assert pickle_module.loads(pickle_module.dumps(finite)) == finite
2277+
22592278
# A variable closedness range with non-nullable bounds round-trips through
22602279
# its storage.
22612280
storage = pa.array(
@@ -2266,6 +2285,20 @@ def test_variable_closedness_range_type_allow_unbounded():
22662285
assert arr.type == finite
22672286

22682287

2288+
def test_variable_closedness_range_type_from_storage(pickle_module):
2289+
# C++ accepts one nullable and one non-nullable bound; pickling keeps it.
2290+
storage = pa.struct([pa.field("lower", pa.float64(), nullable=False),
2291+
pa.field("upper", pa.float64(), nullable=True),
2292+
pa.field("lower_inc", pa.bool_(), nullable=False),
2293+
pa.field("upper_inc", pa.bool_(), nullable=False)])
2294+
range_type = pa.lib._variable_closedness_range_from_storage(storage)
2295+
assert range_type.storage_type == storage
2296+
assert pickle_module.loads(pickle_module.dumps(range_type)) == range_type
2297+
2298+
with pytest.raises(pa.ArrowInvalid, match="must be a Struct"):
2299+
pa.lib._variable_closedness_range_from_storage(pa.int32())
2300+
2301+
22692302
def test_bool8_type(pickle_module):
22702303
bool8_type = pa.bool8()
22712304
storage_type = pa.int8()

‎python/pyarrow/types.pxi‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2138,7 +2138,10 @@ cdef class FixedClosednessRangeType(BaseExtensionType):
21382138
return FixedClosednessRangeArray
21392139

21402140
def __reduce__(self):
2141-
return fixed_closedness_range, (self.value_type, self.closed)
2141+
# Rebuild from the storage type, which keeps the nullability of each
2142+
# bound; the value type and closed alone cannot express that.
2143+
return _fixed_closedness_range_from_storage, (self.storage_type,
2144+
self.closed)
21422145

21432146
def __arrow_ext_scalar_class__(self):
21442147
return FixedClosednessRangeScalar
@@ -2183,7 +2186,9 @@ cdef class VariableClosednessRangeType(BaseExtensionType):
21832186
return VariableClosednessRangeArray
21842187

21852188
def __reduce__(self):
2186-
return variable_closedness_range, (self.value_type,)
2189+
# Rebuild from the storage type, which keeps the nullability of each
2190+
# bound; the value type alone cannot express that.
2191+
return _variable_closedness_range_from_storage, (self.storage_type,)
21872192

21882193
def __arrow_ext_scalar_class__(self):
21892194
return VariableClosednessRangeScalar
@@ -5941,6 +5946,36 @@ def variable_closedness_range(DataType value_type not None, allow_unbounded=True
59415946
return out
59425947

59435948

5949+
def _fixed_closedness_range_from_storage(DataType storage_type not None,
5950+
str closed not None):
5951+
"""
5952+
Rebuild a fixed closedness range type from its storage type.
5953+
5954+
Used for pickling. The storage type is validated like extension type
5955+
metadata read from IPC.
5956+
"""
5957+
cdef:
5958+
FixedClosednessRangeType prototype = fixed_closedness_range(int32())
5959+
c_string c_metadata = tobytes(f'{{"closed": "{closed}"}}')
5960+
shared_ptr[CDataType] c_type = GetResultValue(
5961+
prototype.range_ext_type.Deserialize(storage_type.sp_type, c_metadata))
5962+
return pyarrow_wrap_data_type(c_type)
5963+
5964+
5965+
def _variable_closedness_range_from_storage(DataType storage_type not None):
5966+
"""
5967+
Rebuild a variable closedness range type from its storage type.
5968+
5969+
Used for pickling. The storage type is validated like extension type
5970+
metadata read from IPC.
5971+
"""
5972+
cdef:
5973+
VariableClosednessRangeType prototype = variable_closedness_range(int32())
5974+
shared_ptr[CDataType] c_type = GetResultValue(
5975+
prototype.range_ext_type.Deserialize(storage_type.sp_type, b"{}"))
5976+
return pyarrow_wrap_data_type(c_type)
5977+
5978+
59445979
def opaque(DataType storage_type, str type_name not None, str vendor_name not None):
59455980
"""
59465981
Create instance of opaque extension type.

0 commit comments

Comments
 (0)