Support structs of lists in row lexicographic comparator

[ttnghia]

This fixes the lexicographic comparator that cannot handle the input having structs of lists. The new implementation mainly changes the helper functions decompose_structs. In particular:

• If a structs column has its first child is a lists column, the first column of the result table will no longer be Struct<Struct<...<List<SomeType>...> (i.e., nested structs ultimately having one child).
• Instead, the first output column will be nested empty structs: Struct<...Struct<>>...>. The innermost child column List<SomeType> is output as the second column in the result table.

Depends on:

• Fix gtest column utility comparator diff reporting #12995

Closes #11672.

[ttnghia]

A NULL in i does not mean a NULLs in S2.

No, if S1 is null then all its descendent must be considered as nulls. This is the property of structs: the inner levels cannot be non-nulls while the parent/higher level is null.

Note that the structs column may be "wrongly" constructed such that such property is not enforced. In such cases, such struct column must be corrected by structs::detail::push_down_nulls. I believe that the experimental comparator already calls that API during preprocessing.

Edit: I was wrong. The lex. comparator preprocessor doesn't call push_down_nulls because it is not needed. If the top level is null, the inner levels will not be considered at all. So whether they are nulls or not doesn't matter.

[ttnghia]

For the example you posted, you may get the correct structs column because the make_structs_column API calls structs::detail::superimpose_nulls to nullify the inner levels.

[divyegala]

In my example, it's not a top-level NULL though, is it? i and S2 are both children of S1. I'm not saying S1 is NULL, I'm saying i is NULL. Let me try to write an MRE for this, maybe I am wrong entirely.

[ttnghia]

For the example above, you will have this table row: [ {null}, {5.5}, 3 ], and depth when stopping at the first column is 1, not 0 because the depth value increases when going down:

cudf/cpp/include/cudf/table/experimental/row_operators.cuh

Line 406 in 02073e1

++depth;

.

[divyegala]

For the example above, you will have this table row: [ {null}, {5.5}, 3 ], and depth when stopping at the first column is 1, not 0 because the depth value increases when going down:

But depth is already returned before that, isn't it?

Regardless, it looks like I am wrong. This test passes:

  auto s1 = []() {
    auto i = cudf::test::fixed_width_column_wrapper<int>{{0, 0}, cudf::test::iterators::nulls_at({0, 1})};

    auto f = cudf::test::fixed_width_column_wrapper<float>{11.5, 5.5};
    auto d = cudf::test::fixed_width_column_wrapper<int>{3, 3};

    auto s2 = cudf::test::structs_column_wrapper{{f, d}};

    return cudf::test::structs_column_wrapper{{i, s2}};
  }();

  cudf::test::print(s1);
  cudf::test::fixed_width_column_wrapper<int32_t> expected{1, 0};

  auto s1_tview = cudf::table_view{{s1}};
  auto got = cudf::sorted_order(s1_tview);
  cudf::test::print(got->view());
  CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected, got->view());

I just don't understand how since according to the written example S1<i> is of depth 0 and S2<f> is of depth 1.

When you find a NULL in S<i> at depth 0, then S2<f> at depth 1 is supposed to be skipped according to the code, unless the example is marking them wrongly.

Anyway, thank you very much for walking me through this. Tomorrow I'll print the depths vector to see what the actual contents are.

[ttnghia]

Initially, the depth array is [0, 1, 2]. When processing the first column, depth = 0 as the initial value. Then ++depth is executed at

cudf/cpp/include/cudf/table/experimental/row_operators.cuh

Line 406 in 02073e1

++depth;

on the same column before returning due to null detected. So you will have depth=1 returned.

The returned depth is the depth value at which null is detected, not the original depth value of the top level.

[vyasr]

Couple of questions to make sure I follow the change.

[divyegala]

This PR looks very close, and I really like the test coverage. @ttnghia is there anything else that you are looking to finish?

[vyasr]

I have one open request to clarify a comment, otherwise LGTM!

[bdice]

Can the null count be known here?

[ttnghia]

It can be queried by temp_col.null_count() which may trigger a kernel launch (now). Good point, I think with our plan to remove UNKNOWN_NULL_COUNT then using temp_col.null_count() here would avoid modifying it again in the future.

[ttnghia]

Oh this is also addressed in #13102.

[ttnghia]

/merge