[GitHub] [arrow] wgtmac commented on a diff in pull request #35930: GH-35926: [C++][Parquet] PageIndex allow only building Offset Index

["wgtmac (via GitHub)" <gi...@apache.org>]

wgtmac commented on code in PR #35930:
URL: https://github.com/apache/arrow/pull/35930#discussion_r1219168280


##########
cpp/src/parquet/properties.h:
##########
@@ -139,6 +139,7 @@ static constexpr Encoding::type DEFAULT_ENCODING = Encoding::PLAIN;
 static const char DEFAULT_CREATED_BY[] = CREATED_BY_VERSION;
 static constexpr Compression::type DEFAULT_COMPRESSION_TYPE = Compression::UNCOMPRESSED;
 static constexpr bool DEFAULT_IS_PAGE_INDEX_ENABLED = false;
+static constexpr bool DEFAULT_IS_COLUMN_INDEX_ENABLED = false;

Review Comment:
   This should be true IMO.



##########
cpp/src/parquet/properties.h:
##########
@@ -147,14 +148,16 @@ class PARQUET_EXPORT ColumnProperties {
                    bool dictionary_enabled = DEFAULT_IS_DICTIONARY_ENABLED,
                    bool statistics_enabled = DEFAULT_ARE_STATISTICS_ENABLED,
                    size_t max_stats_size = DEFAULT_MAX_STATISTICS_SIZE,
-                   bool page_index_enabled = DEFAULT_IS_PAGE_INDEX_ENABLED)
+                   bool page_index_enabled = DEFAULT_IS_PAGE_INDEX_ENABLED,
+                   bool column_index_enabled = DEFAULT_IS_COLUMN_INDEX_ENABLED)
       : encoding_(encoding),
         codec_(codec),
         dictionary_enabled_(dictionary_enabled),
         statistics_enabled_(statistics_enabled),
         max_stats_size_(max_stats_size),
         compression_level_(Codec::UseDefaultCompressionLevel()),
-        page_index_enabled_(DEFAULT_IS_PAGE_INDEX_ENABLED) {}
+        page_index_enabled_(page_index_enabled),

Review Comment:
   What about splitting `enable_page_index` into `enable_column_index` and `enable_offset_index`? This looks much cleaner to me. Though it may introduce a weird case where enable_column_index=true && enable_offset_index=false.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org