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
12 changes: 10 additions & 2 deletions velox/dwio/parquet/reader/StringDecoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,16 @@ class StringDecoder {
if (hasNulls) {
numValues = bits::countNonNulls(nulls, current, current + numValues);
}
for (auto i = 0; i < numValues; ++i) {
bufferStart_ += lengthAt(bufferStart_) + sizeof(int32_t);
if (fixedLength_ > 0) {
// FIXED_LEN_BYTE_ARRAY values carry no length prefix, so the byte
// offset is a constant multiple of the fixed width. This is both an
// O(1) skip and a correctness fix: the variable-length branch below
// would read the value bytes as a bogus 4-byte length.
bufferStart_ += static_cast<int64_t>(numValues) * fixedLength_;
} else {
for (auto i = 0; i < numValues; ++i) {
bufferStart_ += lengthAt(bufferStart_) + sizeof(int32_t);
}
}
}

Expand Down
10 changes: 10 additions & 0 deletions velox/dwio/parquet/tests/examples/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,16 @@ annotation, definition level, repetition level, and compression when useful.
- Purpose: Tests reading FIXED_LEN_BYTE_ARRAY as VARBINARY and validates mixed
primitive physical type handling.

### `flba_skip.parquet`

- Metadata: `created_by=parquet-cpp-arrow version 24.0.0`, 40 rows, 1 row group,
columns `key: INT32` and `value: FIXED_LEN_BYTE_ARRAY` (4-byte width), both
optional, uncompressed. `value` equals the row index encoded big-endian.
- Purpose: Regression fixture for skipping FIXED_LEN_BYTE_ARRAY values. Filtering
`key` to the even rows forces the FLBA decoder to skip the odd rows, which must
advance by a constant multiple of the fixed width. A skip that instead read the
value bytes as a 4-byte length prefix would misalign or read out of bounds.

### `uuid.parquet`

- Metadata: `created_by=parquet-mr version 1.12.2`, 3 rows, 1 row group,
Expand Down
Binary file not shown.
44 changes: 44 additions & 0 deletions velox/dwio/parquet/tests/reader/ParquetReaderTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1422,6 +1422,50 @@ TEST_F(ParquetReaderTest, readVarbinaryFromFLBA) {
->valueAt(0));
}

// Regression test for skipping FIXED_LEN_BYTE_ARRAY values under a filter on a
// sibling column. See flba_skip.parquet in examples/README.md for the fixture.
TEST_F(ParquetReaderTest, fixedLenByteArraySkipWithFilter) {
const std::string filename("flba_skip.parquet");
const auto fileSchema = ROW({"key", "value"}, {INTEGER(), VARBINARY()});

constexpr int32_t kNumRows = 40;
std::vector<int64_t> evenKeys;
for (int64_t i = 0; i < kNumRows; i += 2) {
evenKeys.push_back(i);
}
const auto kNumSelected = static_cast<vector_size_t>(evenKeys.size());
FilterMap filters;
filters.insert(
{"key",
std::make_unique<common::BigintValuesUsingBitmask>(
0, kNumRows - 2, std::move(evenKeys), false)});

// Backing storage for the expected 4-byte big-endian values.
std::vector<std::string> valueStore;
for (int32_t i = 0; i < kNumRows; i += 2) {
const auto u = static_cast<uint32_t>(i);
std::string bytes(4, '\0');
bytes[0] = static_cast<char>((u >> 24) & 0xffU);
bytes[1] = static_cast<char>((u >> 16) & 0xffU);
bytes[2] = static_cast<char>((u >> 8) & 0xffU);
bytes[3] = static_cast<char>(u & 0xffU);
valueStore.push_back(std::move(bytes));
}
auto expected = makeRowVector(
{"key", "value"},
{
makeFlatVector<int32_t>(
kNumSelected, [](auto row) { return row * 2; }),
makeFlatVector<StringView>(
kNumSelected,
[&](auto row) { return StringView(valueStore[row]); },
nullptr,
VARBINARY()),
});

assertReadWithFilters(filename, fileSchema, std::move(filters), expected);
}

TEST_F(ParquetReaderTest, readBinaryAsStringFromNation) {
const std::string filename("nation.parquet");

Expand Down
Loading