kou commented on code in PR #47164:
URL: https://github.com/apache/arrow/pull/47164#discussion_r2233298756


##########
cpp/src/arrow/table.h:
##########
@@ -207,7 +207,8 @@ class ARROW_EXPORT Table {
   ///
   /// Two tables can be equal only if they have equal schemas.
   /// However, they may be equal even if they have different chunkings.
-  bool Equals(const Table& other, bool check_metadata = false) const;
+  bool Equals(const Table& other, bool check_metadata = false,
+              const EqualOptions& = EqualOptions::Defaults()) const;

Review Comment:
   I think that the following + test is only needed for #46835 . We don't need 
to change `arrow::Table` for #46835 because `arrow::Table::Equals()` doesn't 
use `arrow::EqualOptions` yet.
   
   ```diff
   diff --git a/cpp/src/arrow/compare.h b/cpp/src/arrow/compare.h
   index 4d2282c982..4de7807ebb 100644
   --- a/cpp/src/arrow/compare.h
   +++ b/cpp/src/arrow/compare.h
   @@ -72,6 +72,16 @@ class EqualOptions {
        return res;
      }
    
   +  /// TODO
   +  bool check_metadata() const { return check_metadata_; }
   +
   +  /// TODO
   +  EqualOptions check_metadata(bool v) const {
   +    auto res = EqualOptions(*this);
   +    res.check_metadata = v;
   +    return res;
   +  }
   +
      /// The absolute tolerance for approximate comparisons of floating-point 
values.
      /// Note that this option is ignored if "use_atol" is set to false.
      double atol() const { return atol_; }
   @@ -103,6 +113,7 @@ class EqualOptions {
      bool nans_equal_ = false;
      bool signed_zeros_equal_ = true;
      bool use_atol_ = false;
   +  bool check_metadata_ = false;
    
      std::ostream* diff_sink_ = NULLPTR;
    };
   diff --git a/cpp/src/arrow/record_batch.cc b/cpp/src/arrow/record_batch.cc
   index 941fd3f002..0ca6b05792 100644
   --- a/cpp/src/arrow/record_batch.cc
   +++ b/cpp/src/arrow/record_batch.cc
   @@ -349,6 +349,11 @@ bool CanIgnoreNaNInEquality(const RecordBatch& batch, 
const EqualOptions& opts)
    
    bool RecordBatch::Equals(const RecordBatch& other, bool check_metadata,
                             const EqualOptions& opts) const {
   +  return RecordBatch::Equals(other, opts.check_metadata(check_metadata));
   +}
   +
   +bool RecordBatch::Equals(const RecordBatch& other,
   +                         const EqualOptions& opts) const {
      if (this == &other) {
        if (CanIgnoreNaNInEquality(*this, opts)) {
          return true;
   @@ -356,7 +361,7 @@ bool RecordBatch::Equals(const RecordBatch& other, bool 
check_metadata,
      } else {
        if (num_columns() != other.num_columns() || num_rows_ != 
other.num_rows()) {
          return false;
   -    } else if (!schema_->Equals(*other.schema(), check_metadata)) {
   +    } else if (!schema_->Equals(*other.schema(), opts.check_metadata())) {
          return false;
        } else if (device_type() != other.device_type()) {
          return false;
   diff --git a/cpp/src/arrow/record_batch.h b/cpp/src/arrow/record_batch.h
   index 43a8ee63b5..0a9a91a556 100644
   --- a/cpp/src/arrow/record_batch.h
   +++ b/cpp/src/arrow/record_batch.h
   @@ -127,6 +127,9 @@ class ARROW_EXPORT RecordBatch {
      bool Equals(const RecordBatch& other, bool check_metadata = false,
                  const EqualOptions& opts = EqualOptions::Defaults()) const;
    
   +  /// TODO
   +  bool Equals(const RecordBatch& other, const EqualOptions& opts) const;
   +
      /// \brief Determine if two record batches are approximately equal
      ///
      /// \param[in] other the RecordBatch to compare with
   ```



-- 
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-unsubscr...@arrow.apache.org

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

Reply via email to