This is an automated email from the ASF dual-hosted git repository.
kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 5b1af01098 MINOR: [C++][Compute] Remove resolved TODO comments in
case_when null validation (#48749)
5b1af01098 is described below
commit 5b1af01098f9281e7c04c5017a7f5232a683f646
Author: Hyukjin Kwon <[email protected]>
AuthorDate: Thu Jan 8 18:19:02 2026 +0900
MINOR: [C++][Compute] Remove resolved TODO comments in case_when null
validation (#48749)
### Rationale for this change
https://github.com/apache/arrow/blob/8ccdbe78063ad4b43872b8826aba37a1a73dc951/cpp/src/arrow/compute/kernels/scalar_if_else.cc#L1771
https://github.com/apache/arrow/blob/8ccdbe78063ad4b43872b8826aba37a1a73dc951/cpp/src/arrow/compute/kernels/scalar_if_else.cc#L1819
https://github.com/apache/arrow/blob/8ccdbe78063ad4b43872b8826aba37a1a73dc951/cpp/src/arrow/compute/kernels/scalar_if_else.cc#L1861
https://github.com/apache/arrow/blob/8ccdbe78063ad4b43872b8826aba37a1a73dc951/cpp/src/arrow/compute/kernels/scalar_if_else.cc#L1903
https://github.com/apache/arrow/blob/8ccdbe78063ad4b43872b8826aba37a1a73dc951/cpp/src/arrow/compute/kernels/scalar_if_else.cc#L1921
https://github.com/apache/arrow/blob/8ccdbe78063ad4b43872b8826aba37a1a73dc951/cpp/src/arrow/compute/kernels/scalar_if_else.cc#L1939
https://github.com/apache/arrow/blob/8ccdbe78063ad4b43872b8826aba37a1a73dc951/cpp/src/arrow/compute/kernels/scalar_if_else.cc#L1967
https://github.com/apache/arrow/blob/8ccdbe78063ad4b43872b8826aba37a1a73dc951/cpp/src/arrow/compute/kernels/scalar_if_else.cc#L1981
are all user-facing checks so cannot be `DCHECK`.
(Generated by ChatGPT)
```python
import pyarrow as pa
import pyarrow.compute as pc
print("=" * 60)
print("PyArrow case_when Validation Examples")
print("=" * 60)
# Helper function to test case_when
def test_case_when(name, cond_array, value1, value2, line_number):
try:
result = pc.case_when(cond_array, value1, value2)
print(f"✅ {name}: Success")
return True
except pa.ArrowInvalid as e:
print(f"❌ {name} (line {line_number}): {e}")
return False
# Create condition structs
valid_cond = pa.array([
{'c1': True}, {'c1': False}
], type=pa.struct([pa.field('c1', pa.bool_())]))
invalid_cond = pa.array([
{'c1': True}, None # ← OUTER NULL triggers validation!
], type=pa.struct([pa.field('c1', pa.bool_())]))
print("\n1. BINARY TYPE (line 1771 - enable_if_base_binary)")
print("-" * 60)
binary1 = pa.array([b'v1', b'v1'])
binary2 = pa.array([b'v2', b'v2'])
test_case_when("Valid binary", valid_cond, binary1, binary2, 1771)
test_case_when("Invalid binary", invalid_cond, binary1, binary2, 1771)
print("\n2. LIST TYPE (line 1818 - enable_if_var_size_list)")
print("-" * 60)
list1 = pa.array([[1, 2], [3, 4]])
list2 = pa.array([[5, 6], [7, 8]])
test_case_when("Valid list", valid_cond, list1, list2, 1818)
test_case_when("Invalid list", invalid_cond, list1, list2, 1818)
print("\n3. LIST_VIEW TYPE (line 1859 - enable_if_list_view)")
print("-" * 60)
listview1 = pa.array([[1, 2], [3, 4]], type=pa.list_view(pa.int64()))
listview2 = pa.array([[5, 6], [7, 8]], type=pa.list_view(pa.int64()))
test_case_when("Valid list_view", valid_cond, listview1, listview2, 1859)
test_case_when("Invalid list_view", invalid_cond, listview1, listview2,
1859)
print("\n4. MAP TYPE (line 1900)")
print("-" * 60)
map1 = pa.array([[('k1', 100)], [('k2', 200)]], type=pa.map_(pa.string(),
pa.int64()))
map2 = pa.array([[('k3', 300)], [('k4', 400)]], type=pa.map_(pa.string(),
pa.int64()))
test_case_when("Valid map", valid_cond, map1, map2, 1900)
test_case_when("Invalid map", invalid_cond, map1, map2, 1900)
print("\n5. STRUCT TYPE (line 1917)")
print("-" * 60)
struct1 = pa.array([{'x': 1}, {'x': 2}], type=pa.struct([pa.field('x',
pa.int64())]))
struct2 = pa.array([{'x': 3}, {'x': 4}], type=pa.struct([pa.field('x',
pa.int64())]))
test_case_when("Valid struct", valid_cond, struct1, struct2, 1917)
test_case_when("Invalid struct", invalid_cond, struct1, struct2, 1917)
print("\n6. FIXED_SIZE_LIST TYPE (line 1934)")
print("-" * 60)
fsl1 = pa.array([[1, 2], [3, 4]], type=pa.list_(pa.int64(), 2))
fsl2 = pa.array([[5, 6], [7, 8]], type=pa.list_(pa.int64(), 2))
test_case_when("Valid fixed_size_list", valid_cond, fsl1, fsl2, 1934)
test_case_when("Invalid fixed_size_list", invalid_cond, fsl1, fsl2, 1934)
print("\n7. DICTIONARY TYPE (line 1974)")
print("-" * 60)
dict1 = pa.array(['a', 'b']).dictionary_encode()
dict2 = pa.array(['c', 'd']).dictionary_encode()
test_case_when("Valid dictionary", valid_cond, dict1, dict2, 1974)
test_case_when("Invalid dictionary", invalid_cond, dict1, dict2, 1974)
print("\n" + "=" * 60)
print("KEY POINT: Outer nulls (None in struct) trigger validation")
print(" Inner nulls ({'c1': None}) are allowed")
print("=" * 60)
```
### What changes are included in this PR?
This PR removes obsolete TODOs. They cannot be DCHECK.
### Are these changes tested?
Yes, as described above.
### Are there any user-facing changes?
No, dev-only.
Authored-by: Hyukjin Kwon <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
---
cpp/src/arrow/compute/kernels/scalar_if_else.cc | 8 --------
1 file changed, 8 deletions(-)
diff --git a/cpp/src/arrow/compute/kernels/scalar_if_else.cc
b/cpp/src/arrow/compute/kernels/scalar_if_else.cc
index d885db4cd9..d32070f5c0 100644
--- a/cpp/src/arrow/compute/kernels/scalar_if_else.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_if_else.cc
@@ -1768,7 +1768,6 @@ struct CaseWhenFunctor<Type, enable_if_base_binary<Type>>
{
using offset_type = typename Type::offset_type;
using BuilderType = typename TypeTraits<Type>::BuilderType;
static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
- /// TODO(wesm): should this be a DCHECK? Or checked elsewhere
if (batch[0].null_count() > 0) {
return Status::Invalid("cond struct must not have outer nulls");
}
@@ -1816,7 +1815,6 @@ struct CaseWhenFunctor<Type,
enable_if_var_size_list<Type>> {
using offset_type = typename Type::offset_type;
using BuilderType = typename TypeTraits<Type>::BuilderType;
static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
- /// TODO(wesm): should this be a DCHECK? Or checked elsewhere
if (batch[0].null_count() > 0) {
return Status::Invalid("cond struct must not have outer nulls");
}
@@ -1858,7 +1856,6 @@ struct CaseWhenFunctor<Type, enable_if_list_view<Type>> {
using offset_type = typename Type::offset_type;
using BuilderType = typename TypeTraits<Type>::BuilderType;
static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
- /// TODO(wesm): should this be a DCHECK? Or checked elsewhere
if (batch[0].null_count() > 0) {
return Status::Invalid("cond struct must not have outer nulls");
}
@@ -1900,7 +1897,6 @@ Status ReserveNoData(ArrayBuilder*) { return
Status::OK(); }
template <>
struct CaseWhenFunctor<MapType> {
static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
- /// TODO(wesm): should this be a DCHECK? Or checked elsewhere
if (batch[0].null_count() > 0) {
return Status::Invalid("cond struct must not have outer nulls");
}
@@ -1918,7 +1914,6 @@ struct CaseWhenFunctor<MapType> {
template <>
struct CaseWhenFunctor<StructType> {
static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
- /// TODO(wesm): should this be a DCHECK? Or checked elsewhere
if (batch[0].null_count() > 0) {
return Status::Invalid("cond struct must not have outer nulls");
}
@@ -1936,7 +1931,6 @@ struct CaseWhenFunctor<StructType> {
template <>
struct CaseWhenFunctor<FixedSizeListType> {
static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
- /// TODO(wesm): should this be a DCHECK? Or checked elsewhere
if (batch[0].null_count() > 0) {
return Status::Invalid("cond struct must not have outer nulls");
}
@@ -1964,7 +1958,6 @@ struct CaseWhenFunctor<FixedSizeListType> {
template <typename Type>
struct CaseWhenFunctor<Type, enable_if_union<Type>> {
static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
- /// TODO(wesm): should this be a DCHECK? Or checked elsewhere
if (batch[0].null_count() > 0) {
return Status::Invalid("cond struct must not have outer nulls");
}
@@ -1978,7 +1971,6 @@ struct CaseWhenFunctor<Type, enable_if_union<Type>> {
template <>
struct CaseWhenFunctor<DictionaryType> {
static Status Exec(KernelContext* ctx, const ExecSpan& batch, ExecResult*
out) {
- /// TODO(wesm): should this be a DCHECK? Or checked elsewhere
if (batch[0].null_count() > 0) {
return Status::Invalid("cond struct must not have outer nulls");
}
