Skip to content

[C++][Parquet] Clarify the destructor implementation of parquet::Page #49408

@HuaHuaY

Description

@HuaHuaY

Describe the enhancement requested

At cpp/src/parquet/column_page.h, Page has no destructor. I don't know whether it's by design or it's a bug. I think it's better to add an explicit destructor to expose the design intent. We don't meet memory leaks here because we always use std::shared_ptr<Page> and its destructor is saved in the subclass std::shared_ptr<T> object.

  • If it's by design, I think it's better to add protected: ~Page() = default; to avoiding freeing subclass object by Page's pointer.
  • If it's a bug, we need to add public: virtual ~Page() = default;.
class Page {
 public:
  Page(const std::shared_ptr<Buffer>& buffer, PageType::type type)
      : buffer_(buffer), type_(type) {}

  ......

 private:
  std::shared_ptr<Buffer> buffer_;
  PageType::type type_;
};

/// \brief Base type for DataPageV1 and DataPageV2 including common attributes
class DataPage : public Page {
  ......

  int32_t num_values_;
  Encoding::type encoding_;
  int64_t uncompressed_size_;
  EncodedStatistics statistics_;
  /// Row ordinal within the row group to the first row in the data page.
  std::optional<int64_t> first_row_index_;
  SizeStatistics size_statistics_;
};

Component(s)

C++, Parquet

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions