Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: [C++][Parquet] Add RowRanges API #45234

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions cpp/src/parquet/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ set(PARQUET_SRCS
platform.cc
printer.cc
properties.cc
row_range.cc
schema.cc
size_statistics.cc
statistics.cc
Expand Down
225 changes: 225 additions & 0 deletions cpp/src/parquet/row_range.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

#include "parquet/row_range.h"

#include "arrow/util/bitmap_ops.h"
#include "arrow/util/unreachable.h"
#include "parquet/exception.h"

namespace parquet {

class IteratorImpl : public RowRanges::Iterator {
public:
explicit IteratorImpl(const RowRanges& ranges)
: iter_(ranges.ranges_.cbegin()), end_(ranges.ranges_.cend()) {}

~IteratorImpl() override = default;

std::variant<RowRanges::IntervalRange, RowRanges::BitmapRange, RowRanges::End>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking that would variant being slow in storage? What would an internal:

enum class Type {
   IntervalRange,
   BitmapRange
};

struct Range {
   int64_t begin;
   /// for Interval this means range
   /// for bitmap this is a 64-bits map
   int64_t payload; 
};
std::vector<Type> types_;
std::vector<Range> ranges_;

I think this is more space efficient than current storage api...

NextRange() override {
if (iter_ == end_) {
return RowRanges::End();
}
if (std::holds_alternative<RowRanges::IntervalRange>(*iter_)) {
return std::get<RowRanges::IntervalRange>(*iter_);
}
if (std::holds_alternative<RowRanges::BitmapRange>(*iter_)) {
return std::get<RowRanges::BitmapRange>(*iter_);
}
arrow::Unreachable("Invalid row ranges type");
}

private:
decltype(RowRanges::ranges_.cbegin()) iter_;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we using IterT = decltype(RowRanges::ranges_.cbegin()); here?

decltype(RowRanges::ranges_.cend()) end_;
};

std::unique_ptr<RowRanges::Iterator> RowRanges::NewIterator() const {
return std::make_unique<IteratorImpl>(*this);
}

void RowRanges::Validate() const {
int64_t last_end = -1;
for (const auto& range : ranges_) {
if (std::holds_alternative<RowRanges::IntervalRange>(range)) {
const auto& interval = std::get<RowRanges::IntervalRange>(range);
if (interval.start <= last_end) {
throw ParquetException("Row ranges are not in ascending order");
}
if (interval.end < interval.start) {
throw ParquetException("Invalid interval range");
}
last_end = interval.end;
continue;
}
if (std::holds_alternative<RowRanges::BitmapRange>(range)) {
const auto& bitmap = std::get<RowRanges::BitmapRange>(range);
if (bitmap.offset <= last_end) {
throw ParquetException("Row ranges are not in ascending order");
}
last_end = bitmap.offset + sizeof(bitmap.bitmap) - 1;
continue;
}
arrow::Unreachable("Invalid row ranges type");
}
}

int64_t RowRanges::row_count() const {
int64_t count = 0;
for (const auto& range : ranges_) {
if (std::holds_alternative<RowRanges::IntervalRange>(range)) {
const auto& interval = std::get<RowRanges::IntervalRange>(range);
count += interval.end - interval.start + 1;
}
if (std::holds_alternative<RowRanges::BitmapRange>(range)) {
const auto& bitmap = std::get<RowRanges::BitmapRange>(range);
count += arrow::internal::CountSetBits(
reinterpret_cast<const uint8_t*>(&bitmap.bitmap), 0, sizeof(bitmap.bitmap));
}
arrow::Unreachable("Invalid row ranges type");
}
return count;
}

RowRanges RowRanges::Intersect(const RowRanges& lhs, const RowRanges& rhs) {
RowRanges result;
auto lhs_iter = lhs.NewIterator();
auto rhs_iter = rhs.NewIterator();
auto lhs_range = lhs_iter->NextRange();
auto rhs_range = rhs_iter->NextRange();

while (!std::holds_alternative<End>(lhs_range) &&
!std::holds_alternative<End>(rhs_range)) {
if (!std::holds_alternative<IntervalRange>(lhs_range) ||
!std::holds_alternative<IntervalRange>(rhs_range)) {
throw ParquetException("Bitmap range is not yet supported");
}

auto& left = std::get<IntervalRange>(lhs_range);
auto& right = std::get<IntervalRange>(rhs_range);

// Find overlapping region
int64_t start = std::max(left.start, right.start);
int64_t end = std::min(left.end, right.end);

// If there is an overlap, add it to results
if (start <= end) {
result.ranges_.push_back(IntervalRange{start, end});
}

// Advance the iterator with smaller end
if (left.end < right.end) {
lhs_range = lhs_iter->NextRange();
} else {
rhs_range = rhs_iter->NextRange();
}
}

return result;
}

RowRanges RowRanges::Union(const RowRanges& lhs, const RowRanges& rhs) {
RowRanges result;
auto lhs_iter = lhs.NewIterator();
auto rhs_iter = rhs.NewIterator();
auto lhs_range = lhs_iter->NextRange();
auto rhs_range = rhs_iter->NextRange();

if (std::holds_alternative<End>(lhs_range)) {
return rhs;
}
if (std::holds_alternative<End>(rhs_range)) {
return lhs;
}

if (std::holds_alternative<BitmapRange>(lhs_range)) {
throw ParquetException("Bitmap range is not yet supported");
}
IntervalRange current = std::get<IntervalRange>(lhs_range);
lhs_range = lhs_iter->NextRange();

while (!std::holds_alternative<End>(lhs_range) ||
!std::holds_alternative<End>(rhs_range)) {
IntervalRange next;

if (std::holds_alternative<End>(rhs_range)) {
// Only lhs ranges remain
if (std::holds_alternative<BitmapRange>(lhs_range)) {
throw ParquetException("Bitmap range is not yet supported");
}
next = std::get<IntervalRange>(lhs_range);
lhs_range = lhs_iter->NextRange();
} else if (std::holds_alternative<End>(lhs_range)) {
// Only rhs ranges remain
if (std::holds_alternative<BitmapRange>(rhs_range)) {
throw ParquetException("Bitmap range is not yet supported");
}
next = std::get<IntervalRange>(rhs_range);
rhs_range = rhs_iter->NextRange();
} else {
// Both iterators have ranges - pick the one with smaller start
if (std::holds_alternative<BitmapRange>(lhs_range) ||
std::holds_alternative<BitmapRange>(rhs_range)) {
throw ParquetException("Bitmap range is not yet supported");
}
const auto& left = std::get<IntervalRange>(lhs_range);
const auto& right = std::get<IntervalRange>(rhs_range);

if (left.start <= right.start) {
next = left;
lhs_range = lhs_iter->NextRange();
} else {
next = right;
rhs_range = rhs_iter->NextRange();
}
}

if (current.end + 1 >= next.start) {
// Concatenate overlapping or adjacent ranges
current.end = std::max(current.end, next.end);
} else {
// Gap between current and next range
result.ranges_.push_back(current);
current = next;
}
}

result.ranges_.push_back(current);
return result;
}

RowRanges RowRanges::MakeSingle(int64_t row_count) {
RowRanges rowRanges;
rowRanges.ranges_.push_back(IntervalRange{0, row_count - 1});
return rowRanges;
}

RowRanges RowRanges::MakeSingle(int64_t start, int64_t end) {
RowRanges rowRanges;
rowRanges.ranges_.push_back(IntervalRange{start, end});
return rowRanges;
}

RowRanges RowRanges::MakeIntervals(const std::vector<IntervalRange>& intervals) {
RowRanges rowRanges;
rowRanges.ranges_.reserve(intervals.size());
rowRanges.ranges_.insert(rowRanges.ranges_.end(), intervals.cbegin(), intervals.cend());
return rowRanges;
}

} // namespace parquet
87 changes: 87 additions & 0 deletions cpp/src/parquet/row_range.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

#pragma once

#include <variant>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also need to include <cstdint>

#include <vector>

#include "parquet/platform.h"

namespace parquet {

/// RowRanges is a collection of non-overlapping and ascendingly ordered row ranges.
class PARQUET_EXPORT RowRanges {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we mark internal or experimental first? I think this might change fastly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plural class names are always a bit weird, should we perhaps name it RowSelection?

public:
/// \brief A range of contiguous rows represented by an interval.
struct IntervalRange {
/// Start row of the range (inclusive).
int64_t start;
/// End row of the range (inclusive).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest either a length or a exclusive end. Inclusive is more confusing and error-prone IMHO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We commonly use the (start, length) convention in other places.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(start, length) is good for API but not good for computation because we have to transform it back and forth to (start, start + length).

int64_t end;
};

/// \brief A range of contiguous rows represented by a bitmap.
struct BitmapRange {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually convenient to use? I see it's not supported yet in the implementation...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are other possible representations, for example:

/// \brief A range of 8 rows represented by a start offset and small indices
struct IndexedRange {
  /// Start row of the range
  int64_t offset;
  /// Indices from `offset` that point to selected rows, in ascending order.
  std::array<uint8_t, 8> indices;
};

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, two comments:

  1. this shows that a Google Doc is really not the right medium to discuss API changes
  2. I would ask that any API comes with an actual implementation; if it's too complicated to make use of the proposed BitmapRange efficiently, then it shows that BitmapRange is not the right abstraction

Copy link
Member Author

@wgtmac wgtmac Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also a little bit skeptical about the effectiveness of mixing BitmapRange and IntervalRange. According to my experience, I need either a list of IntervalRange (like the RowRanges API in parquet-java) or simply a roaring bitmap, but not mixed. The former is used for intermediate result of filter pushdown (e.g. compute selected row ranges of a row group when evaluating different filters) and the latter is for row-level selection/deselection (e.g. apply iceberg position delete file to a base file).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The former is used for intermediate result of filter pushdown (e.g. compute selected row ranges of a row group when evaluating different filters) and the latter is for row-level selection/deselection (e.g. apply iceberg position delete file to a base file).

Ok, so several questions follow:

  1. at which use case is this API targeted?
  2. what is the typical bitmap length this is meant to represent?
  3. do we want a structure to select across an entire row group, or is it better to have one selection per data page, or perhaps a two-level selection (with a page bitmap and then a per-page bitmap for those pages that haven't been pruned)?

For example, given the typical size of a page, an uncompressed bitmap might be fine if selecting inside a page.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. at which use case is this API targeted?

a. Specify the required row ranges when creating a Parquet reader (be it a ParquetFileReader or RowGroupReader). This applies to both interval and bitmap forms depending on the use case. Perhaps this is the main reason to use a mixed form.
b. Used internally when computing matched row ranges from filter pushdown evaluation result with the help of page index. This applies to the interval range because operations are usually based on the page level.
c. Advanced row-level filtering discussed in #37559. This should be bitmap range used by ColumnReader and Decoder.

  1. what is the typical bitmap length this is meant to represent?

It depends on the use case above and can be any of num_rows_in_file, num_rows_in_a_row_group and batch_size.

  1. do we want a structure to select across an entire row group, or is it better to have one selection per data page, or perhaps a two-level selection (with a page bitmap and then a per-page bitmap for those pages that haven't been pruned)?

I don't think we need an intra-page selection since page boundaries are usually not aligned across different columns. I think file-level selection is easy to use for users and row-group-level selection is required by the RowGroupReader implementation.

/// Start row of the range (inclusive).
int64_t offset;
/// Zero appended if there are less than 64 elements.
uint64_t bitmap;
};

/// \brief An end marker for the row range iterator.
struct End {};

/// \brief An iterator for accessing row ranges in order.
class Iterator {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have iteration facilities in arrow/util/iterator.h, should they be reused instead of defining our own here?

public:
virtual ~Iterator() = default;
virtual std::variant<IntervalRange, BitmapRange, End> NextRange() = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this virtual function being slow?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that doesn't look terrific. Perhaps something like:

/// Return a batch of ranges. The returned batch has size 0 iff the iterator is exhausted.
virtual util::span<std::variant<IntervalRange, BitmapRange>> NextRanges() = 0;

};

/// \brief Create a new iterator for accessing row ranges in order.
std::unique_ptr<Iterator> NewIterator() const;

/// \brief Validate the row ranges.
/// \throws ParquetException if the row ranges are not in ascending order or
/// overlapped.
void Validate() const;

/// \brief Get the total number of rows in the row ranges.
int64_t row_count() const;

/// \brief Compute the intersection of two row ranges.
static RowRanges Intersect(const RowRanges& lhs, const RowRanges& rhs);

/// \brief Compute the union of two row ranges.
static RowRanges Union(const RowRanges& lhs, const RowRanges& rhs);

/// \brief Make a single row range of [0, row_count - 1].
static RowRanges MakeSingle(int64_t row_count);

/// \brief Make a single row range of [start, end].
static RowRanges MakeSingle(int64_t start, int64_t end);
Comment on lines +73 to +77
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is basically a simpler version of the IntervalRange-taking constructor?


/// \brief Make a row range from a list of intervals.
static RowRanges MakeIntervals(const std::vector<IntervalRange>& intervals);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could take a span<const IntervalRange> instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, perhaps name this FromIntervals.


private:
friend class IteratorImpl;
std::vector<std::variant<IntervalRange, BitmapRange>> ranges_;
};

} // namespace parquet
Loading