alamb commented on code in PR #3625:
URL: https://github.com/apache/arrow-rs/pull/3625#discussion_r1089940406


##########
parquet/src/file/page_index/index_reader.rs:
##########
@@ -25,8 +27,17 @@ use crate::format::{ColumnIndex, OffsetIndex, PageLocation};
 use std::io::{Cursor, Read};
 use thrift::protocol::{TCompactInputProtocol, TSerializable};
 
-/// Read on row group's all columns indexes and change into  [`Index`]
-/// If not the format not available return an empty vector.
+/// Reads per-column [`Index`] for all columns of a row group by

Review Comment:
   If I may rant a little, the use of the terms `Column Index` and `Page Index` 
by the parquet spec to refer to overlapping parts of this feature I find very 
confusing. Like the column index feature is made up of page indexes, maybe? Blah



##########
parquet/src/file/metadata.rs:
##########
@@ -50,7 +50,25 @@ use crate::schema::types::{
     Type as SchemaType,
 };
 
+/// [`Index`] page level for each row group of each column.

Review Comment:
   I found these two typedefs especially confusing which is why I propose 
expanding doc strings and add examples



-- 
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]

Reply via email to