You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@parquet.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/05/19 11:51:58 UTC
[PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
alamb opened a new pull request, #245:
URL: https://github.com/apache/parquet-format/pull/245
https://issues.apache.org/jira/browse/PARQUET-2480
I have always found it very confusing that people refer to the term "page index" when referring to parquet, for example [this message](https://lists.apache.org/thread/o9nxbmv1z4hph3v5s2z63jsklywpkyyj,)
However, the term "page index" is not used in the the [parquet.thrift](https://github.com/apache/parquet-format/blob/master/src/main/thrift/parquet.thrift) file itself, but only appears as the name of the file that describes the `ColumnIndex` spec, [PageIndex.md](https://github.com/apache/parquet-format/blob/master/PageIndex.md)
This means I can't search for "page index" in the spec and find out what people are talking about
## Proposed Clarifications
1. Update the introductory paragraph of `PageIndex.md` to clarify use the term "page index" and explain that it is encoded as ColumnIndex
2. Update the description of `ColumnIndex` to include the term "page index" and clarify what it is
### Jira
- [x] My PR addresses the following [Parquet Jira](https://issues.apache.org/jira/browse/PARQUET/) issues and references them in the PR title. For example, "PARQUET-1234: My Parquet PR"
- https://issues.apache.org/jira/browse/PARQUET-2480
- In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
### Commits
- [x] My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation
- [x] In case of new functionality, my PR adds documentation that describes how to use it.
- All the public functions and the classes in the PR contain Javadoc that explain what it does
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac merged PR #245:
URL: https://github.com/apache/parquet-format/pull/245
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #245:
URL: https://github.com/apache/parquet-format/pull/245#discussion_r1606765182
##########
src/main/thrift/parquet.thrift:
##########
@@ -1017,8 +1020,10 @@ struct OffsetIndex {
}
/**
- * Description for ColumnIndex.
- * Each <array-field>[i] refers to the page at OffsetIndex.page_locations[i]
+ * "Page Index": Optional statistics for each data page in a ColumnChunk.
Review Comment:
Thank you. I think this is addressed in 0afdcd951b022eba0fdb81bde1ee390c502a329f
##########
src/main/thrift/parquet.thrift:
##########
@@ -738,10 +738,10 @@ struct PageHeader {
}
/**
- * Wrapper struct to specify sort order
+ * Sort order within a RowGroup
*/
struct SortingColumn {
- /** The column index (in this row group) **/
+ /** The ordinal position of the column (in this row group) **/
Review Comment:
I updated this to avoid potential confusion with `ColumnIndex` which is a different structure
##########
src/main/thrift/parquet.thrift:
##########
@@ -1017,8 +1020,10 @@ struct OffsetIndex {
}
/**
- * Description for ColumnIndex.
- * Each <array-field>[i] refers to the page at OffsetIndex.page_locations[i]
+ * "Page Index": Optional statistics for each data page in a ColumnChunk.
Review Comment:
I am glad I opened this PR as I was clearly confused about what people were talking about when they said page index !
I addressed in 0afdcd951b022eba0fdb81bde1ee390c502a329f (by using the term "page index"). Let me know what you think
##########
PageIndex.md:
##########
@@ -19,9 +19,11 @@
# ColumnIndex Layout to Support Page Skipping
-This document describes the format for column index pages in the Parquet
-footer. These pages contain statistics for DataPages and can be used to skip
-pages when scanning data in ordered and unordered columns.
+In Parquet, a "Page Index" is optional metadata for a
Review Comment:
After some thought, I think using "page index" (non capitalized) might be best because there while `ColumnIndex` and `OffsetIndex` are names of thrift structs, `PageIndex` is not. So referring to `PageIndex` as though it were a struct I think will cause more confusion
I tried to clarify this in 0afdcd9.
##########
src/main/thrift/parquet.thrift:
##########
@@ -738,10 +738,10 @@ struct PageHeader {
}
/**
- * Wrapper struct to specify sort order
+ * Sort order within a RowGroup
*/
struct SortingColumn {
- /** The column index (in this row group) **/
+ /** The ordinal position of the column (in this row group) **/
Review Comment:
in c7e3ae7
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #245:
URL: https://github.com/apache/parquet-format/pull/245#discussion_r1606818965
##########
PageIndex.md:
##########
@@ -17,11 +17,13 @@
- under the License.
-->
-# ColumnIndex Layout to Support Page Skipping
+# Parquet page index: Layout to Support Page Skipping
-This document describes the format for column index pages in the Parquet
-footer. These pages contain statistics for DataPages and can be used to skip
-pages when scanning data in ordered and unordered columns.
+In Parquet, a *page index* is optional metadata for a
+ColumnChunk, containing statistics for DataPages that can be used
+to skip those pages when scanning in ordered and unordered columns.
Review Comment:
Yes, ColumnIndex will not be created when there is no valid min/max statistics. In parquet-cpp, we can also disable creating ColumnIndex but enabling OffsetIndex.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #245:
URL: https://github.com/apache/parquet-format/pull/245#discussion_r1606156562
##########
PageIndex.md:
##########
@@ -19,9 +19,11 @@
# ColumnIndex Layout to Support Page Skipping
-This document describes the format for column index pages in the Parquet
-footer. These pages contain statistics for DataPages and can be used to skip
-pages when scanning data in ordered and unordered columns.
+In Parquet, a "Page Index" is optional metadata for a
Review Comment:
I think there is also confusion that sometimes we say PageIndex, sometimes ColumnIndex. However, PageIndex is composed of ColumnIndex and OffsetIndex. Is it better to use PageIndex wherever possible? If agreed, we need to modify the title of this section as well.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #245:
URL: https://github.com/apache/parquet-format/pull/245#discussion_r1606856543
##########
PageIndex.md:
##########
@@ -17,11 +17,13 @@
- under the License.
-->
-# ColumnIndex Layout to Support Page Skipping
+# Parquet page index: Layout to Support Page Skipping
-This document describes the format for column index pages in the Parquet
-footer. These pages contain statistics for DataPages and can be used to skip
-pages when scanning data in ordered and unordered columns.
+In Parquet, a *page index* is optional metadata for a
+ColumnChunk, containing statistics for DataPages that can be used
+to skip those pages when scanning in ordered and unordered columns.
Review Comment:
Yeah, I guess OffsetIndex might be used separate
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on PR #245:
URL: https://github.com/apache/parquet-format/pull/245#issuecomment-2119741896
I also think that currently PageIndex means "offset index and column index". They're all page-level index
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #245:
URL: https://github.com/apache/parquet-format/pull/245#discussion_r1606856543
##########
PageIndex.md:
##########
@@ -17,11 +17,13 @@
- under the License.
-->
-# ColumnIndex Layout to Support Page Skipping
+# Parquet page index: Layout to Support Page Skipping
-This document describes the format for column index pages in the Parquet
-footer. These pages contain statistics for DataPages and can be used to skip
-pages when scanning data in ordered and unordered columns.
+In Parquet, a *page index* is optional metadata for a
+ColumnChunk, containing statistics for DataPages that can be used
+to skip those pages when scanning in ordered and unordered columns.
Review Comment:
Yeah, I guess OffsetIndex might be used separately ( But personally I don't think Column Index might be used like that, lol)
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #245:
URL: https://github.com/apache/parquet-format/pull/245#discussion_r1606777041
##########
PageIndex.md:
##########
@@ -17,11 +17,13 @@
- under the License.
-->
-# ColumnIndex Layout to Support Page Skipping
+# Parquet page index: Layout to Support Page Skipping
-This document describes the format for column index pages in the Parquet
-footer. These pages contain statistics for DataPages and can be used to skip
-pages when scanning data in ordered and unordered columns.
+In Parquet, a *page index* is optional metadata for a
+ColumnChunk, containing statistics for DataPages that can be used
+to skip those pages when scanning in ordered and unordered columns.
Review Comment:
I am sorry @mapleFU -- I don't quite follow this suggestion
Do you mean that some writers may write only `OffsetIndex` and that both structures may not be present?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #245:
URL: https://github.com/apache/parquet-format/pull/245#discussion_r1606762494
##########
PageIndex.md:
##########
@@ -19,9 +19,11 @@
# ColumnIndex Layout to Support Page Skipping
-This document describes the format for column index pages in the Parquet
-footer. These pages contain statistics for DataPages and can be used to skip
-pages when scanning data in ordered and unordered columns.
+In Parquet, a "Page Index" is optional metadata for a
Review Comment:
After some thought, I think using "page index" (non capitalized) might be best because while `ColumnIndex` and `OffsetIndex` are names of thrift structs, `PageIndex` is not. So referring to `PageIndex` as though it were a struct I think will cause more confusion
I tried to clarify this in 0afdcd9.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #245:
URL: https://github.com/apache/parquet-format/pull/245#issuecomment-2120446395
Thank you for the comments @mapleFU @tustvold and @wgtmac - I implemented your suggestions and I think the PR is much clearer because of it.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #245:
URL: https://github.com/apache/parquet-format/pull/245#discussion_r1607433284
##########
PageIndex.md:
##########
@@ -17,11 +17,13 @@
- under the License.
-->
-# ColumnIndex Layout to Support Page Skipping
+# Parquet page index: Layout to Support Page Skipping
-This document describes the format for column index pages in the Parquet
-footer. These pages contain statistics for DataPages and can be used to skip
-pages when scanning data in ordered and unordered columns.
+In Parquet, a *page index* is optional metadata for a
+ColumnChunk, containing statistics for DataPages that can be used
+to skip those pages when scanning in ordered and unordered columns.
Review Comment:
I added
```diff
--- a/src/main/thrift/parquet.thrift
+++ b/src/main/thrift/parquet.thrift
@@ -1005,6 +1005,8 @@ struct PageLocation {
* Optional offsets for each data page in a ColumnChunk.
*
* Forms part of the page index, along with ColumnIndex.
+ *
+ * OffsetIndex may be present even if ColumnIndex is not.
*/
struct OffsetIndex {
/**
@@ -1026,6 +1028,8 @@ struct OffsetIndex {
*
* Forms part the page index, along with OffsetIndex.
*
+ * If this structure is present, OffsetIndex must also be present.
+ *
* For each field in this structure, <field>[i] refers to the page at
* OffsetIndex.page_locations[i]
*/
```
in 8d59c7d to try and clarify this
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #245:
URL: https://github.com/apache/parquet-format/pull/245#issuecomment-2122757296
@gszadovszky @pitrou @emkornfield @julienledem Would you like to take a look?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #245:
URL: https://github.com/apache/parquet-format/pull/245#discussion_r1606157508
##########
src/main/thrift/parquet.thrift:
##########
@@ -1017,8 +1020,10 @@ struct OffsetIndex {
}
/**
- * Description for ColumnIndex.
- * Each <array-field>[i] refers to the page at OffsetIndex.page_locations[i]
+ * "Page Index": Optional statistics for each data page in a ColumnChunk.
Review Comment:
I think this is the confusion I have mentioned above. As we can see in https://github.com/apache/parquet-format/blob/master/PageIndex.md, the title is `ColumnIndex xxx` but actually it mentions both ColumnIndex and OffsetIndex as PageIndex.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #245:
URL: https://github.com/apache/parquet-format/pull/245#discussion_r1606156924
##########
src/main/thrift/parquet.thrift:
##########
@@ -738,10 +738,10 @@ struct PageHeader {
}
/**
- * Wrapper struct to specify sort order
+ * Sort order within a RowGroup
*/
struct SortingColumn {
- /** The column index (in this row group) **/
+ /** The ordinal position of the column (in this row group) **/
Review Comment:
I'd suggest to explicitly say it is a leaf column.
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #245:
URL: https://github.com/apache/parquet-format/pull/245#discussion_r1606673705
##########
src/main/thrift/parquet.thrift:
##########
@@ -1017,8 +1020,10 @@ struct OffsetIndex {
}
/**
- * Description for ColumnIndex.
- * Each <array-field>[i] refers to the page at OffsetIndex.page_locations[i]
+ * "Page Index": Optional statistics for each data page in a ColumnChunk.
Review Comment:
The page index is a collective term for both the column and offset indexes, this here is the column index
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] PARQUET-2480: Clarify what "page index" means in Parquet.thrift [parquet-format]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #245:
URL: https://github.com/apache/parquet-format/pull/245#discussion_r1606773522
##########
PageIndex.md:
##########
@@ -17,11 +17,13 @@
- under the License.
-->
-# ColumnIndex Layout to Support Page Skipping
+# Parquet page index: Layout to Support Page Skipping
-This document describes the format for column index pages in the Parquet
-footer. These pages contain statistics for DataPages and can be used to skip
-pages when scanning data in ordered and unordered columns.
+In Parquet, a *page index* is optional metadata for a
+ColumnChunk, containing statistics for DataPages that can be used
+to skip those pages when scanning in ordered and unordered columns.
Review Comment:
With statistics enabled, only OffsetIndex is presented, with can be used to planning IO and get first_row_ordinal for ColumnChunk?
--
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: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org