apache / arrow

Apache Arrow is a multi-language toolbox for accelerated data interchange and in-memory processing
https://arrow.apache.org/
Apache License 2.0
14.27k stars 3.47k forks source link

[C++] (eventually) abandon scalar columns of an ExecBatch in favor of RLE encoded arrays #31680

Open asfimport opened 2 years ago

asfimport commented 2 years ago

This JIRA is a proposal / discussion. I am not asserting this is the way to go but I would like to consider it.

From the execution engine's perspective an exec batch's columns are always either arrays or scalars. The only time we make use of scalars today is for the four augmented columns (e.g. __filename). Once we have support for RLE arrays a scalar could easily be encoded as an RLE array and there would be no need to use scalars here.

The advantage would be reducing the complexity in exec nodes and avoiding issues like ARROW-16288. It is already rather difficult to explain the idea of a "scalar" and "vector" function and then have to turn around and explain that the word "scalar" has an entirely different meaning when talking about field shape.

I think it's worth considering taking this even further and removing the concept from the compute layer entirely. Kernel functions that want to have special logic for scalars could do so using the RLE array. This would be a significant change to many kernels which currently declare the ANY shape and determine which logic to apply within the kernel itself (e.g. there is one array OR scalar kernel and not one kernel for each).

Admittedly there is probably a few instructions and a few bytes more to handle an RLE scalar than the scalar we have today. However, this is just different flavors of O(1) and not likely to have significant impact.

Reporter: Weston Pace / @westonpace

Related issues:

Note: This issue was originally created as ARROW-16289. Please see the migration documentation for further details.

asfimport commented 2 years ago

Weston Pace / @westonpace: CC @lidavidm @edponce @pitrou @michalursa @cyb70289

asfimport commented 2 years ago

David Li / @lidavidm: The concept of scalars would still exist (e.g. in expressions, options) so there's still potential for confusion though this would reduce it. Aggregations would presumably still return scalars, too.

It does seem being able to accept scalars is more confusing than it's worth, though.

asfimport commented 2 years ago

Eduardo Ponce / @edponce: The term Scalar is used in different (but related) contexts. For example, the notion of a Scalar value, Scalar kernels, Scalar expressions, etc.

I recall from an ad-hoc conversation last year where it was discussed that we should consider treating Scalars as a 1-element Array to making the compute layer logic more straightforward. The front-end API would still have the concept of a Scalar but it would be disguised as an Array for execution purposes.

I think such a proposal has its merits, but we should ensure where the concept of Scalar will remain and make these distinctions clear.

asfimport commented 2 years ago

Antoine Pitrou / @pitrou: RLE arrays is a pie in the sky feature for now so I'm not sure this is really worth discussing.

Notice that ExecBatch is supposed to support a selection vector that has never been implemented in any kernel that I know of.