Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions cpp/src/io/utilities/column_buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include <cudf/detail/null_mask.hpp>
#include <cudf/detail/structs/utilities.hpp>
#include <cudf/detail/unary.hpp>
#include <cudf/detail/utilities/vector_factories.hpp>
#include <cudf/strings/strings_column_view.hpp>
#include <cudf/types.hpp>
Expand Down Expand Up @@ -227,12 +228,21 @@ std::unique_ptr<column> make_column(column_buffer_base<string_policy>& buffer,
}
}

return make_lists_column(
num_rows,
std::move(col_content.children[strings_column_view::offsets_column_index]),
std::move(uint8_col),
null_count,
std::move(*col_content.null_mask));
// A strings column may carry 64-bit offsets, but a LIST column's offsets child is
// always 32-bit. The `char_size` check above bounds the chars by `size_type`, so
// every offset value is representable as an int32_t and this cannot lose data.
auto offsets_col =
std::move(col_content.children[strings_column_view::offsets_column_index]);
if (offsets_col->type().id() != type_id::INT32) {
offsets_col = cudf::detail::cast(
offsets_col->view(), data_type{type_id::INT32}, stream, buffer._mr);
}

return make_lists_column(num_rows,
std::move(offsets_col),
std::move(uint8_col),
null_count,
std::move(*col_content.null_mask));
Comment thread
davidwendt marked this conversation as resolved.
}
}

Expand Down
37 changes: 37 additions & 0 deletions cpp/tests/io/parquet_reader_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1684,6 +1684,43 @@ TEST_F(ParquetReaderTest, NestedByteArray)
CUDF_TEST_EXPECT_TABLES_EQUAL(expected, result.tbl->view());
}

// A binary column is read back as a strings column and then converted to `list<uint8>`.
// A strings column may carry 64-bit offsets, but a LIST column's offsets child is always
// 32-bit, so that conversion must not hand an INT64 offsets column to `make_lists_column`.
TEST_F(ParquetReaderTest, BinaryAsListLargeStringsThreshold)
{
// Force the intermediate strings column onto the 64-bit offsets path. The data below is far
// under `INT32_MAX` bytes, so every offset value stays representable as an int32_t.
tmp_env_var const large_strings_threshold{"LIBCUDF_LARGE_STRINGS_THRESHOLD", "8"};

cudf::test::lists_column_wrapper<uint8_t> list_int_col{
{'M', 'o', 'n', 'd', 'a', 'y'},
{'W', 'e', 'd', 'n', 'e', 's', 'd', 'a', 'y'},
{'F', 'r', 'i', 'd', 'a', 'y'},
{'F', 'u', 'n', 'd', 'a', 'y'}};

auto const expected = table_view{{list_int_col}};
cudf::io::table_input_metadata output_metadata(expected);
output_metadata.column_metadata[0].set_name("col_binary").set_output_as_binary(true);

auto filepath = temp_env->get_temp_filepath("BinaryAsListLargeStringsThreshold.parquet");
cudf::io::parquet_writer_options out_opts =
cudf::io::parquet_writer_options::builder(cudf::io::sink_info{filepath}, expected)
.metadata(std::move(output_metadata));
cudf::io::write_parquet(out_opts);

cudf::io::parquet_reader_options in_opts =
cudf::io::parquet_reader_options::builder(cudf::io::source_info{filepath})
.set_column_schema({cudf::io::reader_column_schema().set_convert_binary_to_strings(false)});
auto result = cudf::io::read_parquet(in_opts);

auto const col = result.tbl->view().column(0);
ASSERT_EQ(col.type().id(), cudf::type_id::LIST);
EXPECT_EQ(col.child(cudf::lists_column_view::offsets_column_index).type().id(),
cudf::type_id::INT32);
CUDF_TEST_EXPECT_TABLES_EQUAL(expected, result.tbl->view());
}
Comment thread
davidwendt marked this conversation as resolved.

TEST_F(ParquetReaderTest, StructByteArray)
{
constexpr auto num_rows = 100;
Expand Down
Loading