GH-41055: [C++] Support flatten for combining nested list related types

[ZhangHuiGui]

Rationale for this change

Support flatten for combining nested list related types.

What changes are included in this PR?

Add the recursively flatten function for auto detect and flatten the combining nested list types.

Are these changes tested?

Yes

Are there any user-facing changes?

Yes, user can flatten a combining nested-list or related array by use Flatten API.

• GitHub Issue: [C++] ListArray's Flatten function should support recursive flattening of nested lists #41055

[github-actions]

⚠️ GitHub issue #41055 has been automatically assigned in GitHub to PR creator.

[ZhangHuiGui]

This TEST was forgotten to be added.

[ZhangHuiGui]

@felipecrv PTAL, thanks!

[mapleFU]

Would this better a new API?

[mapleFU]

what about list view?

[ZhangHuiGui]

List view related types are in FlattenListViewArray path.
You're right, it's better to extract them into an independent function.

[mapleFU]

apache/datafusion#6995

Seems ClickHouse flatten will flat nested list, other db in this issue doesn't flatten nested list

[ZhangHuiGui]

apache/arrow-datafusion#6995

Seems ClickHouse flatten will flat nested list, other db in this issue doesn't flatten nested list

Yes, i've thought about this just now.
I'm planing to add an option for Flatten api, let user decide which kind of flatten they want?

And also, this is actually a general requirement. If we allow users to generate such nested types as a general data platform(rather than db), we should provide corresponding flattening functions. At the same time, for some of our built-in compute functions, such as the scalar_hash kernel function we are working on recently , need to calculate hash values of list type, then the nested-array need to be completely flattened.

[mapleFU]

I don't know, I think if we want to implement that:

1. For function, we can support option
2. For underlying impl, a new interface

[ZhangHuiGui]

I don't know, I think if we want to implement that:

1. For function, we can support option
2. For underlying impl, a new interface

For 1, adder a new FlattenRecursion function with same underlying implementation as Flatten and keep the origin function for compatible with pyarrow python interface.

[felipecrv]

I think we should be explicit and list the types that we consider as logical lists here. It should include lists, list-views, fixed-size-lists, and map.

[felipecrv]

And you only have to declare it on VarLengthListLikeArray<> and FixedSizeListArray -- they can all simply call the same non-inline function that dispatches based on type->id(). That extra check won't be significant for a function that scans the entire array and allocates.

[ZhangHuiGui]

Agree! It's much clear.

[felipecrv]

Instead of doing this, you should have a while loop that successfully calls FlattenListArray. No bool with_recursion should be necessary.

The while loop means we don't stack overflow if someone passes a very deeply nested logical list array. In the future, we should come to this function and optimize it specifically for the recursive use-case. If depth==1, it's trivial to delegate to the non-recursive version, otherwise there are many interesting special cases that can be very efficient to flatten. But for now the while loop with successive flatten calls will do.

[ZhangHuiGui]

Yes, with_recursion is unnecessary. And also, it seems that we can easily complete flatten in one cycle with depth=1.

[felipecrv]

I don't know, I think if we want to implement that:

1. For function, we can support option
2. For underlying impl, a new interface

For 1, adder a new FlattenRecursion function with same underlying implementation as Flatten and keep the origin function for compatible with pyarrow python interface.

At the builder_nested.h level, I don't think bool parameters controlling recursion are necessary/desirable. Perhaps at the kernels API level (with the addition of an options_class for the "list_flatten" function) or at the pyarrow bindings. The default value for bool recursive on these 2 should be false so old behavior is preserved unless users opt-in to the recursive behavior explicitly.

arrow/cpp/src/arrow/compute/kernels/vector_nested.cc

Lines 108 to 113 in dbedcfc

	const FunctionDoc list_flatten_doc(
	"Flatten list values",
	("`lists` must have a list-like type.\n"
	"Return an array with the top list level flattened.\n"
	"Top-level null values in `lists` do not emit anything in the input."),
	{"lists"});

[felipecrv]

Looking much simpler now! But some small changes needed.

[felipecrv]

Don't move returned values. At this position, the compiler automatically turns the returned value into a && and passes that to the Result constructor. It took me a while to learn this!

[ZhangHuiGui]

Thank you so much for this!

[ZhangHuiGui]

@felipecrv Thank you so much for providing so many helpful suggestions to make codes cleaner!

[ZhangHuiGui]

is_list and is_list_view would exclude MAP type.

[felipecrv]

Indeed.

[felipecrv]

I will merge this soon if no one has concerns.

@ZhangHuiGui please create an issue (Enhancement Request) about improving the implementation of this function by making it create less intermediate array values. And another one about exposing it on the kernels API and Python API (cc @jorisvandenbossche).

[ZhangHuiGui]

@felipecrv Thanks for your review!
Two independent issues have been added to facilitate our subsequent tracking.

[mapleFU]

This looks nice as a first implementation ( without considering the performance )

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit b98763a.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them.