Always load binary

Author: mortbopet

include/substrait/common/Io.h

@@ -30,15 +30,12 @@ enum class PlanFileFormat {
  * amount of memory that it consumed on disk.
  *
  * \param input_filename The filename containing the plan to convert.
- * \param force_binary If true, the plan will be opened as a binary file.
- *  Required on Windows to avoid text mode line-ending translation.
  * \return If loading was successful, returns a plan.  If loading was not
  * successful this is a status containing a list of parse errors in the status's
  * message.
  */
 absl::StatusOr<::substrait::proto::Plan> loadPlan(
-    std::string_view input_filename,
-    bool force_binary = false);
+    std::string_view input_filename);
 
 /*
  * \brief Writes the provided plan to disk.

src/substrait/common/Io.cpp

@@ -37,10 +37,8 @@ PlanFileFormat detectFormat(std::string_view content) {
 } // namespace
 
 absl::StatusOr<::substrait::proto::Plan> loadPlan(
-    std::string_view input_filename,
-    bool forceBinary) {
-  auto contentOrError =
-      textplan::readFromFile(input_filename.data(), forceBinary);
+    std::string_view input_filename) {
+  auto contentOrError = textplan::readFromFile(input_filename.data());
   if (!contentOrError.ok()) {
     return contentOrError.status();
   }

src/substrait/common/tests/IoTest.cpp

@@ -87,16 +87,7 @@ TEST_P(SaveAndLoadTestFixture, SaveAndLoad) {
   read->mutable_named_table()->add_names("table_name");
   auto status = ::io::substrait::savePlan(plan, tempFilename, encoding);
   ASSERT_TRUE(status.ok()) << "Save failed.\n" << status;
-
-#ifdef _WIN32
-  // Windows cannot rely on io::substrait::loadPlan to detect the file format,
-  // since it needs to a-priori specify how the file should be loaded.
-  bool forceBinary = encoding == PlanFileFormat::kBinary;
-  auto result = ::io::substrait::loadPlan(tempFilename, forceBinary);
-#else
   auto result = ::io::substrait::loadPlan(tempFilename);
-#endif
-
   ASSERT_TRUE(result.ok()) << "Load failed.\n" << result.status();
   ASSERT_THAT(
       *result,

src/substrait/textplan/converter/LoadBinary.cpp

@@ -42,15 +42,8 @@ class StringErrorCollector : public google::protobuf::io::ErrorCollector {
 
 } // namespace
 
-absl::StatusOr<std::string> readFromFile(
-    std::string_view msgPath,
-    bool forceBinary) {
-  std::ifstream file;
-  if (forceBinary)
-    file.open(std::string{msgPath}, std::ios::binary);
-  else
-    file.open(std::string{msgPath}, std::ios::in);
-
+absl::StatusOr<std::string> readFromFile(std::string_view msgPath) {
+  std::ifstream file(std::string{msgPath}, std::ios::binary);
   if (file.fail()) {
     auto currDir = std::filesystem::current_path().string();
     return absl::ErrnoToStatus(

src/substrait/textplan/converter/LoadBinary.h

@@ -14,9 +14,7 @@ namespace io::substrait::textplan {
 
 // Read the contents of a file from disk. 'forceBinary' enables file reading in
 // binary mode.
-absl::StatusOr<std::string> readFromFile(
-    std::string_view msgPath,
-    bool forceBinary = false);
+absl::StatusOr<std::string> readFromFile(std::string_view msgPath);
 
 // Reads a plan from a json-encoded text proto.
 // Returns a list of errors if the file cannot be parsed.