GH-40868: [C++][Parquet] Minor: Remove "Experimental" for parquet::RecordReader
Rationale for this change
Remove experimental for parquet::internal::RecordReader
What changes are included in this PR?
Remove experimental for parquet::internal::RecordReader
Are these changes tested?
no
Are there any user-facing changes?
no
- GitHub Issue: #40868
:warning: GitHub issue #40868 has been automatically assigned in GitHub to PR creator.
I just remove the experimental but not internal::, the later one will introduce lots compilation error. But will do it if you like
I would prefer fixing those compile errors at the same time. We have to do that any way.
/arrow/cpp/src/parquet/file_reader.h:65:33: error: declaration of 'std::shared_ptr<parquet::RecordReader> parquet::RowGroupReader::RecordReader(int, bool)' changes meaning of 'RecordReader' [-fpermissive]
65 | std::shared_ptr<RecordReader> RecordReader(int i, bool read_dictionary = false);
| ^~~~~~~~~~~~
/arrow/cpp/src/parquet/file_reader.h:40:7: note: 'RecordReader' declared here as 'class parquet::RecordReader'
40 | class RecordReader;
| ^~~~~~~~~~~~
/arrow/cpp/src/parquet/file_reader.h:90:31: error: type/value mismatch at argument 1 in template parameter list for 'template<class _Tp> class std::shared_ptr'
90 | std::shared_ptr<RecordReader> RecordReaderWithExposeEncoding(
| ^
Hmmm
@wgtmac I tried to removed the internal, but RowGroupReader::RecordReader is a function, which conflicts here. This patch I'll first remove experimental here.
IMHO, it is literally weird to remove experimental before removing internal namespace. The expectation is that the API is stable after removing experimental except that we will include the namespace change in the same release.
Then we may need to change this interface:
// EXPERIMENTAL: Construct a RecordReader for the indicated column of the row group.
// Ownership is shared with the RowGroupReader.
std::shared_ptr<internal::RecordReader> RecordReader(int i,
bool read_dictionary = false);
I'm also ok to change this to GetRecordReader