Parquet: normalize dictionary encoding to use RLE_DICTIONARY

apache · May 24, 2024 · 4d664b5 · 4d664b5
1 parent 3a4fcff
commit 4d664b5
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 18 deletions.
diff --git a/cpp/src/parquet/column_reader.cc b/cpp/src/parquet/column_reader.cc
@@ -642,12 +642,6 @@ namespace {
 // ----------------------------------------------------------------------
 // Impl base class for TypedColumnReader and RecordReader
 
-// PLAIN_DICTIONARY is deprecated but used to be used as a dictionary index
-// encoding.
-static bool IsDictionaryIndexEncoding(const Encoding::type& e) {
-  return e == Encoding::RLE_DICTIONARY || e == Encoding::PLAIN_DICTIONARY;
-}
-
 template <typename DType>
 class ColumnReaderImplBase {
  public:
@@ -876,8 +870,9 @@ class ColumnReaderImplBase {
     }
 
     Encoding::type encoding = page.encoding();
-
     if (IsDictionaryIndexEncoding(encoding)) {
+      // Normalizing the PLAIN_DICTIONARY to RLE_DICTIONARY encoding
+      // in decoder.
       encoding = Encoding::RLE_DICTIONARY;
     }
 
@@ -950,7 +945,7 @@ class ColumnReaderImplBase {
 
   /// Flag to signal when a new dictionary has been set, for the benefit of
   /// DictionaryRecordReader
-  bool new_dictionary_;
+  bool new_dictionary_ = false;
 
   // The exposed encoding
   ExposedEncoding exposed_encoding_ = ExposedEncoding::NO_ENCODING;

diff --git a/cpp/src/parquet/column_writer.cc b/cpp/src/parquet/column_writer.cc
@@ -1205,10 +1205,6 @@ Status ConvertDictionaryToDense(const ::arrow::Array& array, MemoryPool* pool,
   return Status::OK();
 }
 
-static inline bool IsDictionaryEncoding(Encoding::type encoding) {
-  return encoding == Encoding::PLAIN_DICTIONARY;
-}
-
 template <typename DType>
 class TypedColumnWriterImpl : public ColumnWriterImpl, public TypedColumnWriter<DType> {
  public:
@@ -1565,7 +1561,7 @@ class TypedColumnWriterImpl : public ColumnWriterImpl, public TypedColumnWriter<
   }
 
   void FallbackToPlainEncoding() {
-    if (IsDictionaryEncoding(current_encoder_->encoding())) {
+    if (IsDictionaryIndexEncoding(current_encoder_->encoding())) {
       WriteDictionaryPage();
       // Serialize the buffered Dictionary Indices
       FlushBufferedDataPages();
@@ -1661,7 +1657,7 @@ Status TypedColumnWriterImpl<DType>::WriteArrowDictionary(
                            maybe_parent_nulls);
   };
 
-  if (!IsDictionaryEncoding(current_encoder_->encoding()) ||
+  if (!IsDictionaryIndexEncoding(current_encoder_->encoding()) ||
       !DictionaryDirectWriteSupported(array)) {
     // No longer dictionary-encoding for whatever reason, maybe we never were
     // or we decided to stop. Note that WriteArrow can be invoked multiple

diff --git a/cpp/src/parquet/encoding.cc b/cpp/src/parquet/encoding.cc
@@ -442,12 +442,12 @@ class DictEncoderImpl : public EncoderImpl, virtual public DictEncoder<DType> {
   constexpr static int32_t kDataPageBitWidthBytes = 1;
 
   explicit DictEncoderImpl(const ColumnDescriptor* desc, MemoryPool* pool)
-      : EncoderImpl(desc, Encoding::PLAIN_DICTIONARY, pool),
+      : EncoderImpl(desc, Encoding::RLE_DICTIONARY, pool),
         buffered_indices_(::arrow::stl::allocator<int32_t>(pool)),
         dict_encoded_size_(0),
         memo_table_(pool, kInitialHashTableSize) {}
 
-  ~DictEncoderImpl() = default;
+  ~DictEncoderImpl() override = default;
 
   int dict_encoded_size() const override { return dict_encoded_size_; }
 

diff --git a/cpp/src/parquet/file_reader.cc b/cpp/src/parquet/file_reader.cc
@@ -71,8 +71,7 @@ bool IsColumnChunkFullyDictionaryEncoded(const ColumnChunkMetaData& col) {
   }
   // The following pages should be dictionary encoded data pages.
   for (size_t idx = 1; idx < encoding_stats.size(); ++idx) {
-    if ((encoding_stats[idx].encoding != Encoding::RLE_DICTIONARY &&
-         encoding_stats[idx].encoding != Encoding::PLAIN_DICTIONARY) ||
+    if (!IsDictionaryIndexEncoding(encoding_stats[idx].encoding) ||
         (encoding_stats[idx].page_type != PageType::DATA_PAGE &&
          encoding_stats[idx].page_type != PageType::DATA_PAGE_V2)) {
       // Return false if any following page is not a dictionary encoded data

diff --git a/cpp/src/parquet/properties.h b/cpp/src/parquet/properties.h
@@ -1180,4 +1180,10 @@ struct ArrowWriteContext {
 PARQUET_EXPORT
 std::shared_ptr<ArrowWriterProperties> default_arrow_writer_properties();
 
+// PLAIN_DICTIONARY is deprecated but used to be used as a dictionary index
+// encoding.
+constexpr bool IsDictionaryIndexEncoding(Encoding::type e) {
+  return e == Encoding::RLE_DICTIONARY || e == Encoding::PLAIN_DICTIONARY;
+}
+
 }  // namespace parquet