[BUG] corruption in string column after contig split + partition

[abellina]

This issue happens after a combination of cudf calls are applied and started occurring after this PR was merged: #10673.

If we first call contiguous_split with 0 splits (an operation that should return the original data but packed in a single buffer) and later call partition on a row that has an empty string "", the result is a null string instead of the original empty string.

Before the linked PR this wasn't an issue (as we noticed this in tests that broke NVIDIA/spark-rapids#5286). If contiguous_split is removed from the sequence of calls, the issue doesn't manifest.

Here's a repro case, and thanks to @davidwendt for taking a cuDF java example I had and converting it to the C++ equivalent.

TEST_F(ContiguousSplitTableCornerCases, SplitEmptyColumn)
{
  cudf::test::strings_column_wrapper a{""};
  cudf::table_view t({a});
  auto cs = cudf::contiguous_split(t, {});
  std::cout << "csplit size = " << cs.size() << std::endl;
  cudf::test::print(cs.front().table.column(0));
  std::cout << "csplit columns = " << cs.front().table.num_columns() << "\n";
  std::cout << "csplit rows = " << cs.front().table.num_rows() << "\n";
  cudf::test::print(cs.back().table.column(0));

  auto map    = cudf::test::fixed_width_column_wrapper<int32_t>{0};
  auto result = cudf::partition(cs.front().table, map, 2);
  std::cout << "partition size = " << result.second.size() << "\n";
  std::cout << "partition table columns = " << result.first->num_columns() << "\n";
  std::cout << "partition table rows = " << result.first->num_rows() << "\n";
  cudf::test::print(result.first->view().column(0));
}

In this case, the last print: cudf::test::print(result.first->view().column(0));, prints NULL when it should be the empty string.

Thanks to @jlowe and @revans2 for pointing me in the right direction.

[abellina]

@davidwendt graciously offered to take a look at this issue, so I've assigned it to him.

[abellina]

This seems like another case of: #4480, where data() for the empty string is null, so when we compare it with the null placeholder it converts the value to null when the scatter code is trying to create the resulting column.

This patch fixes this particular instance, but I am not 100% sure if this is the correct place, but essentially I believe we should detect that the element is the empty string, and return string_view{"", 0} instead of string_view{nullptr, 0}.

If this is the correct place (column_device_view::element(size_type) which is overloaded for string_view) , I am happy to put up a PR + test.

diff --git a/cpp/include/cudf/column/column_device_view.cuh b/cpp/include/cudf/column/column_device_view.cuh
index 070ca80..6508d6e 100644
--- a/cpp/include/cudf/column/column_device_view.cuh
+++ b/cpp/include/cudf/column/column_device_view.cuh
@@ -395,8 +395,14 @@ class alignas(16) column_device_view : public detail::column_device_view_base {
     size_type index       = element_index + offset();  // account for this view's _offset
     const auto* d_offsets = d_children[strings_column_view::offsets_column_index].data<int32_t>();
     const char* d_strings = d_children[strings_column_view::chars_column_index].data<char>();
-    size_type offset      = d_offsets[index];
-    return string_view{d_strings + offset, d_offsets[index + 1] - offset};
+    size_type offset      = d_offsets[index];   
+    const char* element   = d_strings + offset;
+    // special case empty string
+    if (element == nullptr and !is_null(element_index)) {
+      return string_view{"", 0};
+    }
+
+    return string_view{element, d_offsets[index + 1] - offset};
   }

[davidwendt]

@abellina I don't agree with this solution. I want to keep this function as efficient as possible. I think there is a better way to fix this where the specific factory functions are used (which are rarely). I will work on this tomorrow.