Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++][Parquet] Minor: making parquet TypedComparator operation as const method #38874

Closed
mapleFU opened this issue Nov 24, 2023 · 0 comments · Fixed by #38875
Closed

[C++][Parquet] Minor: making parquet TypedComparator operation as const method #38874

mapleFU opened this issue Nov 24, 2023 · 0 comments · Fixed by #38875

Comments

@mapleFU
Copy link
Member

mapleFU commented Nov 24, 2023

Describe the enhancement requested

/// \brief Interface for comparison of physical types according to the
/// semantics of a particular logical type.
template <typename DType>
class TypedComparator : public Comparator {
 public:
  using T = typename DType::c_type;

  /// \brief Scalar comparison of two elements, return true if first
  /// is strictly less than the second
  virtual bool Compare(const T& a, const T& b) = 0;

  /// \brief Compute maximum and minimum elements in a batch of
  /// elements without any nulls
  virtual std::pair<T, T> GetMinMax(const T* values, int64_t length) = 0;

  /// \brief Compute minimum and maximum elements from an Arrow array. Only
  /// valid for certain Parquet Type / Arrow Type combinations, like BYTE_ARRAY
  /// / arrow::BinaryArray
  virtual std::pair<T, T> GetMinMax(const ::arrow::Array& values) = 0;

  /// \brief Compute maximum and minimum elements in a batch of
  /// elements with accompanying bitmap indicating which elements are
  /// included (bit set) and excluded (bit not set)
  ///
  /// \param[in] values the sequence of values
  /// \param[in] length the length of the sequence
  /// \param[in] valid_bits a bitmap indicating which elements are
  /// included (1) or excluded (0)
  /// \param[in] valid_bits_offset the bit offset into the bitmap of
  /// the first element in the sequence
  virtual std::pair<T, T> GetMinMaxSpaced(const T* values, int64_t length,
                                          const uint8_t* valid_bits,
                                          int64_t valid_bits_offset) = 0;
};

Current they're all non-const functions. So we need to making them as const.

Component(s)

C++, Parquet

mapleFU added a commit that referenced this issue Nov 28, 2023
…ion as const method (#38875)

### Rationale for this change

`parquet::TypedComparator<DType>` is not const method, which should be const

### What changes are included in this PR?

Change `Compare`, `GetMinMax`, `GetMinMaxSpaced` to const

### Are these changes tested?

No

### Are there any user-facing changes?

No

* Closes: #38874

Authored-by: mwish <maplewish117@gmail.com>
Signed-off-by: mwish <maplewish117@gmail.com>
@mapleFU mapleFU added this to the 15.0.0 milestone Nov 28, 2023
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
…operation as const method (apache#38875)

### Rationale for this change

`parquet::TypedComparator<DType>` is not const method, which should be const

### What changes are included in this PR?

Change `Compare`, `GetMinMax`, `GetMinMaxSpaced` to const

### Are these changes tested?

No

### Are there any user-facing changes?

No

* Closes: apache#38874

Authored-by: mwish <maplewish117@gmail.com>
Signed-off-by: mwish <maplewish117@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant