bkietz commented on a change in pull request #8894:
URL: https://github.com/apache/arrow/pull/8894#discussion_r551978453



##########
File path: cpp/src/arrow/dataset/partition_test.cc
##########
@@ -21,52 +21,51 @@
 #include <gtest/gtest.h>
 
 #include <cstdint>
-#include <map>
 #include <memory>
 #include <regex>
 #include <string>
 #include <vector>
 
-#include "arrow/dataset/file_base.h"
 #include "arrow/dataset/scanner_internal.h"
 #include "arrow/dataset/test_util.h"
-#include "arrow/filesystem/localfs.h"
 #include "arrow/filesystem/path_util.h"
 #include "arrow/status.h"
 #include "arrow/testing/gtest_util.h"
-#include "arrow/util/io_util.h"
 
 namespace arrow {
 using internal::checked_pointer_cast;
 
 namespace dataset {
 
-using E = TestExpression;
-
 class TestPartitioning : public ::testing::Test {
  public:
   void AssertParseError(const std::string& path) {
     ASSERT_RAISES(Invalid, partitioning_->Parse(path));
   }
 
-  void AssertParse(const std::string& path, E expected) {
+  void AssertParse(const std::string& path, Expression expected) {
     ASSERT_OK_AND_ASSIGN(auto parsed, partitioning_->Parse(path));
-    ASSERT_EQ(E{parsed}, expected);
+    ASSERT_EQ(parsed, expected);
   }
 
   template <StatusCode code = StatusCode::Invalid>
-  void AssertFormatError(E expr) {
-    ASSERT_EQ(partitioning_->Format(*expr.expression).status().code(), code);
+  void AssertFormatError(Expression expr) {
+    ASSERT_EQ(partitioning_->Format(expr).status().code(), code);
   }
 
-  void AssertFormat(E expr, const std::string& expected) {
-    ASSERT_OK_AND_ASSIGN(auto formatted, 
partitioning_->Format(*expr.expression));
+  void AssertFormat(Expression expr, const std::string& expected) {
+    // formatted partition expressions are bound to the schema of the dataset 
being
+    // written
+    ASSERT_OK_AND_ASSIGN(auto formatted, partitioning_->Format(expr));
     ASSERT_EQ(formatted, expected);
 
     // ensure the formatted path round trips the relevant components of the 
partition
     // expression: roundtripped should be a subset of expr
-    ASSERT_OK_AND_ASSIGN(auto roundtripped, partitioning_->Parse(formatted));
-    ASSERT_EQ(E{roundtripped->Assume(*expr.expression)}, E{scalar(true)});
+    ASSERT_OK_AND_ASSIGN(Expression roundtripped, 
partitioning_->Parse(formatted));
+
+    ASSERT_OK_AND_ASSIGN(roundtripped, roundtripped.Bind(*written_schema_));

Review comment:
       Hmm, actually not: hive partitioning can parse arbitrarily ordered field 
values, but will always format them in schema order.




----------------------------------------------------------------
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.

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


Reply via email to