apache / apache/arrow

[C++] Choose the Correct Value in ArrayBuilder::AppendEmptyValue() for Floating-Point Types

Open
#50,509 6 comments 0 reactions 1 assignee Claimed by @goel-skd View on GitHub
Component: C++ Type: enhancement
Dominant language
C++
Stars
17.1k
Forks
4.3k
Avg merge
3d 13h
Merged PRs (30d)
88

Description

### Describe the enhancement requested

`ArrayBuilder::AppendEmptyValue()` was introduced and implemented in [ARROW-25393](https://github.com/apache/arrow/issues/25393) to write an arbitrary, well-defined value during `StructBuilder::AppendNull()`, as discussed [here](https://github.com/apache/arrow/pull/7887#discussion_r470062219). In that context, the value written to the child builder is irrelevant because the parent entry is null.

At that time, `RunEndEncodedBuilder` did not yet exist; it was implemented about three years later in [ARROW-32688](https://github.com/apache/arrow/issues/32688). As a result, the current implementation does not appear to consider that the value written by `AppendEmptyValue()` may later become part of the value stream observed by `RunEndEncodedBuilder`.

this can be problematic for RunEndEncodedBuilder with floating-point values, as it can produce a zero value, which cannot be considered an empty value (Case 1). Even worse, it can be mistakenly confused with actual floating-point values (see the example below).

### Case 1

```c++
auto ree_type = run_end_encoded(int32(), float32());
auto int32_builder = std::make_shared(pool_);
auto float_builder = std::make_shared(pool_);
RunEndEncodedBuilder builder(pool_, int32_builder, float_builder, ree_type);

ASSERT_OK(builder.AppendEmptyValues(3));
ASSERT_OK(builder.AppendScalar(**MakeScalar(float32(), 3), 3));

ASSERT_OK_AND_ASSIGN(auto array, builder.Finish());
ARROW_LOGGER_INFO("", array->ToString());
```

Output:

```text
-- run_ends:
[
3,
6
]
-- values:
[
0,
3
]
```

### Case 2

```c++
auto ree_type = run_end_encoded(int32(), float32());
auto int32_builder = std::make_shared(pool_);
auto float_builder = std::make_shared(pool_);
RunEndEncodedBuilder builder(pool_, int32_builder, float_builder, ree_type);

ASSERT_OK(builder.AppendEmptyValues(3));
ASSERT_OK(builder.AppendScalar(**MakeScalar(float32(), 0), 3));

ASSERT_OK_AND_ASSIGN(auto array, builder.Finish());
ARROW_LOGGER_INFO("", array->ToString());
```

Output:

```text
-- run_ends:
[
3,
6
]
-- values:
[
0,
0
]
```

One possible solution would be to use `NaN` as the placeholder value for floating-point builders instead of `0.0f`.

### Component(s)

C++

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.