Skip to content

Conversation

@Ted-Jiang
Copy link
Member

@Ted-Jiang Ted-Jiang commented Jul 1, 2022

@Ted-Jiang
Copy link
Member Author

Ted-Jiang commented Jul 1, 2022

@pitrou @alamb PTAL

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me -- thank you @Ted-Jiang

data/README.md Outdated
|null_list.parquet | an empty list. Generated from this json `{"emptylist":[]}` and for the purposes of testing correct read/write behaviour of this base case. |
| null_list.parquet | an empty list. Generated from this json `{"emptylist":[]}` and for the purposes of testing correct read/write behaviour of this base case. |
| alltypes_tiny_pages.parquet | small page sizes with dictionary encoding with page index |
| alltypes_tiny_pages_plain.parquet | small page sizes without dictionary encoding with page index |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can also mention the file's lineage (that it came from impala, with link) in this comment

data/README.md Outdated
|null_list.parquet | an empty list. Generated from this json `{"emptylist":[]}` and for the purposes of testing correct read/write behaviour of this base case. |
| null_list.parquet | an empty list. Generated from this json `{"emptylist":[]}` and for the purposes of testing correct read/write behaviour of this base case. |
| alltypes_tiny_pages.parquet | small page sizes with dictionary encoding with page index |
| alltypes_tiny_pages_plain.parquet | small page sizes without dictionary encoding with page index |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| alltypes_tiny_pages_plain.parquet | small page sizes without dictionary encoding with page index |
| alltypes_tiny_pages_plain.parquet | small page sizes with plain encoding with page index |

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alamb Thanks, fix all comment.

@Ted-Jiang
Copy link
Member Author

@wesm could please take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants