andishgar commented on issue #50509: URL: https://github.com/apache/arrow/issues/50509#issuecomment-5677272706
> > One possible solution would be to use NaN as the placeholder value for floating-point builders instead of 0.0f. > > That would only fix the issue until [[#50517](https://github.com/apache/arrow/issues/50517)](https://github.com/apache/arrow/issues/50517) is fixed, right? Yes, that's correct. However, at this stage, I would like to know whether this suggestion is useful as a possible solution or direction. > > Even worse, it can be mistakenly confused with actual floating-point values (see the example below). > > That would be a bug in `RunEndEncodedBuilder::AppendEmptyValues` then? Let's make sure that "empty" values don't get compressed with a run of non-empty values. Sorry, I think I conveyed my suggestion incorrectly. The current logic in `RunEndEncodedBuilder` already prevents merging `AppendEmptyValues` with non-empty values. The problem is that the value produced by `AppendEmptyValues` is indistinguishable from a legitimate value with the same value. For example, both an empty value and an explicitly appended `0.0` are represented as `0.0` in different consecutive runs, so the run-end value stream alone cannot tell whether a particular value came from `AppendEmptyValues` or from a normal append. For example: ```cpp TEST(TestRunEndEncodedBuilder, AppendScalar) { auto ree_type = run_end_encoded(int32(), float32()); auto run_end_builder = std::make_shared<Int32Builder>(); auto value_builder = std::make_shared<FloatBuilder>(); RunEndEncodedBuilder ree_builder(default_memory_pool(), run_end_builder, value_builder, ree_type); ASSERT_OK(ree_builder.AppendEmptyValues(10)); ASSERT_OK(ree_builder.AppendScalar(**MakeScalar(float32(), 0), 10)); ASSERT_OK_AND_ASSIGN(auto result, ree_builder.Finish()); ARROW_LOGGER_INFO("", result->ToString()); } ``` This produces: ```text -- run_ends: [ 10, 20 ] -- values: [ 0, 0 ] ``` The two runs are kept separate, but looking at the values alone, there is no way to distinguish the first `0` produced by `AppendEmptyValues` from the second `0` produced by `AppendScalar`. > > 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). > > This is the same behavior as other numeric types, so I don't understand the problem. Integer types do not have NaN, so there is no analogous special value that could be used as a placeholder, and `0` is currently used. Floating-point types, however, do have NaN, which could potentially serve as a distinguishable placeholder. As I mentioned above, `AppendEmptyValue()` was originally introduced for `StructBuilder::AppendNull()`, where the parent validity bitmap marks the value as null, so the value written to the child builder does not matter. In `RunEndEncodedBuilder::AppendEmptyValue()`, however, no validity bitmap is set for the value, so it is important to write a value that can distinguish an empty value from a regular value. For floating-point types, NaN could potentially serve this purpose. -- 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]
