professor-moody opened a new pull request, #55384: URL: https://github.com/apache/arrow/pull/55384
### Rationale for this change `ColumnChunkReader::SetPageReader` in `cpp/src/parquet/column_reader.cc` clears the current page, the decoders and current_decoder_, but keeps `num_buffered_values_` and `num_decoded_values_` from the previous page. When a `RecordReader` is moved to the next row group before the previous page was read to the end, EnsureDataPage() still sees values left in the old page and does not read a new one, so the next `ReadRecords` decodes with the cleared current_decoder_ and crashes. `TypedRecordReader` also keeps levels that were decoded from the previous column chunk but not consumed yet. parquet::arrow::FileReader reads each row group to the end before calling `SetPageReader`, so it is not affected; direct users of `RecordReader` are. ### What changes are included in this PR? - `ColumnChunkReader::SetPageReader` also resets `num_buffered_values_` and `num_decoded_values_`, so the next read starts from the first page of the new column chunk. - `TypedRecordReader::SetPageReader` drops the levels between `levels_position_` and `levels_written()` that were decoded from the previous column chunk. Levels that were already consumed are kept. ### Are these changes tested? Yes, in `cpp/src/parquet/reader_test.cc`, on a file with a required and an optional dictionary-encoded `INT32` column in two row groups with different values: - `RecordReaderSetPageReaderAfterFullReadSucceeds` reads the first row group to the end, calls `SetPageReader` for the second one and checks that `ReadDictionary` and `ReadRecords` return the second row group's dictionary and values. - `RecordReaderSetPageReaderAfterPartialReadDoesNotReusePage` reads 2 of 8 records of the first row group, calls `SetPageReader` for the second one and checks that `ReadRecords` returns the second row group's values. The Parquet reader and Arrow reader/writer test suites pass with the change. ### Are there any user-facing changes? `ReadRecords` on a `RecordReader` returns the new row group's values after `SetPageReader`, also when the previous row group was not read to the end. **This PR contains a "Critical Fix".** Without it, this call sequence crashes in `ReadRecords`. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [x] Human - [x] AI **Reviewed before submission by:** - [x] Human - [x] AI - [ ] Not reviewed -- 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]
