PARQUET-1193: [CPP] Implement ColumnOrder to support min_value and max_value#430
PARQUET-1193: [CPP] Implement ColumnOrder to support min_value and max_value#430majetideepak wants to merge 6 commits intoapache:masterfrom
Conversation
| auto column_chunk5 = ColumnChunkMetaData::Make( | ||
| reinterpret_cast<const uint8_t*>(&col_chunk), schema.Column(4), &version); | ||
| ASSERT_TRUE(column_chunk5->is_stats_set()); | ||
| ASSERT_FALSE(column_chunk5->is_stats_set()); |
There was a problem hiding this comment.
Why is this changed needed here?
There was a problem hiding this comment.
column_chunk5 is defined for LogicalType::INTERVAL, which has sort order UNKNOWN.
I modified this PR to not set stats for all UNKNOWN sort orders.
| }; | ||
|
|
||
| class ColumnOrder { | ||
| public: |
There was a problem hiding this comment.
From where stems the need to have a full class for the column order? From looking at the code, it seems to me like using the enum should be sufficient.
There was a problem hiding this comment.
Yes, enum is sufficient for this PR. I added the full class for further extensions. The Java implementation did something similar:
https://github.com/apache/parquet-mr/pull/435/files#diff-37183c50b2eaab3ca048f9c5954e5b52R26
xhochy
left a comment
xhochy
left a comment
There was a problem hiding this comment.
+1, thanks for the comments. Everything makes sense!
majetideepak
force-pushed
the
PARQUET-1193
branch
5 times, most recently
from
ff53935 to
7d4a888
Compare
majetideepak
force-pushed
the
PARQUET-1193
branch
from
7d4a888 to
d31df36
Compare
…x_value Changes: 1. Update parquet.thrift format 2. Add ColumnOrder Implementation 3. Make Int96 sort order UNKNOWN Author: Deepak Majeti <deepak.majeti@hpe.com> Closes apache#430 from majetideepak/PARQUET-1193 and squashes the following commits: d31df36 [Deepak Majeti] Fix unused function Warning 4ed405f [Deepak Majeti] Add comments dec58ca [Deepak Majeti] clang-format 2cd9f11 [Deepak Majeti] Make Int96 sort order UNKNOWN ff41b3c [Deepak Majeti] Add ColumnOrder Implementation 6221cba [Deepak Majeti] Pull updated parquet.thrift format
2 participants
Changes: