Copilot commented on code in PR #50422:
URL: https://github.com/apache/arrow/pull/50422#discussion_r3553413928
##########
cpp/src/parquet/column_reader.cc:
##########
@@ -191,6 +203,25 @@ int LevelDecoder::Decode(int batch_size, int16_t* levels) {
return num_decoded;
}
+int LevelDecoder::Skip(int batch_size) {
+ const int num_values = std::min(num_values_remaining_, batch_size);
+ const int num_advanced = impl_->Advance(num_values);
+ ARROW_DCHECK_EQ(num_values, num_advanced);
+ num_values_remaining_ -= num_advanced;
+ return num_advanced;
+}
Review Comment:
`LevelDecoder::Skip` only uses `ARROW_DCHECK_EQ(num_values, num_advanced)`.
In release builds, if the underlying decoder advances fewer values than
expected (e.g. truncated/corrupt levels buffer), the reader state can silently
desynchronize. This should fail fast with a `ParquetException` when
`num_advanced != num_values`.
##########
cpp/src/parquet/column_reader.cc:
##########
@@ -191,6 +203,25 @@ int LevelDecoder::Decode(int batch_size, int16_t* levels) {
return num_decoded;
}
+int LevelDecoder::Skip(int batch_size) {
+ const int num_values = std::min(num_values_remaining_, batch_size);
+ const int num_advanced = impl_->Advance(num_values);
+ ARROW_DCHECK_EQ(num_values, num_advanced);
+ num_values_remaining_ -= num_advanced;
+ return num_advanced;
+}
+
+auto LevelDecoder::CountUpTo(int16_t value, int batch_size) -> CountUpToResult
{
+ const int num_values = std::min(num_values_remaining_, batch_size);
+ const auto result = impl_->CountUpTo(value, num_values);
+ ARROW_DCHECK_EQ(num_values, result.processed_count);
+ num_values_remaining_ -= result.processed_count;
+ return {
+ .matching_count = result.matching_count,
+ .processed_count = result.processed_count,
+ };
+}
Review Comment:
`LevelDecoder::CountUpTo` only asserts that `processed_count == num_values`
via `ARROW_DCHECK_EQ`. In release builds, a short read would silently make
`num_values_remaining_` inconsistent with callers (notably skip paths). Prefer
throwing a `ParquetException` when fewer than `num_values` are processed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
