ARROW-8198: [C++] Format Diff of NullArrays

Also don't DCHECK in Array::Equals Closes #6763 from bkietz/8198-Diffing-should-handle-nul Authored-by: Benjamin Kietzman <bengilgit@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
apache · Mar 31, 2020 · 4560ef5 · 4560ef5
1 parent e16da50
commit 4560ef5
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 1 deletion.
diff --git a/cpp/src/arrow/array/diff.cc b/cpp/src/arrow/array/diff.cc
@@ -822,6 +822,17 @@ class UnifiedDiffFormatter {
 
 Result<std::function<Status(const Array& edits, const Array& base, const Array& target)>>
 MakeUnifiedDiffFormatter(const DataType& type, std::ostream* os) {
+  if (type.id() == Type::NA) {
+    return [os](const Array& edits, const Array& base, const Array& target) {
+      if (base.length() != target.length()) {
+        *os << "# Null arrays differed" << std::endl
+            << "-" << base.length() << " nulls" << std::endl
+            << "+" << target.length() << " nulls" << std::endl;
+      }
+      return Status::OK();
+    };
+  }
+
   ARROW_ASSIGN_OR_RAISE(auto formatter, MakeFormatter(type));
   return UnifiedDiffFormatter(os, std::move(formatter));
 }

diff --git a/cpp/src/arrow/compare.cc b/cpp/src/arrow/compare.cc
@@ -961,7 +961,7 @@ Status PrintDiff(const Array& left, const Array& right, std::ostream* os) {
 bool ArrayEquals(const Array& left, const Array& right, const EqualOptions& opts) {
   bool are_equal = ArrayEqualsImpl<ArrayEqualsVisitor>(left, right, opts);
   if (!are_equal) {
-    DCHECK_OK(PrintDiff(left, right, opts.diff_sink()));
+    ARROW_IGNORE_EXPR(PrintDiff(left, right, opts.diff_sink()));
   }
   return are_equal;
 }

diff --git a/cpp/src/arrow/compare.h b/cpp/src/arrow/compare.h
@@ -63,6 +63,8 @@ class EqualOptions {
   std::ostream* diff_sink() const { return diff_sink_; }
 
   /// Return a new EqualOptions object with the "diff_sink" property changed.
+  /// This option will be ignored if diff formatting of the types of compared arrays is
+  /// not supported.
   EqualOptions diff_sink(std::ostream* diff_sink) const {
     auto res = EqualOptions(*this);
     res.diff_sink_ = diff_sink;