Added a convenience function for joining lines of text.

src/substrait/common/Io.cpp

@@ -5,6 +5,7 @@
 #include <regex>
 #include <string_view>
 
+#include "substrait/proto/plan.pb.h"
 #include "substrait/textplan/converter/LoadBinary.h"
 #include "substrait/textplan/converter/SaveBinary.h"
 #include "substrait/textplan/parser/LoadText.h"

src/substrait/textplan/StringManipulation.cpp

@@ -2,18 +2,33 @@
 
 #include "StringManipulation.h"
 
+#include <numeric>
+#include <vector>
+#include <string>
+#include <string_view>
+
 namespace io::substrait::textplan {
 
-// Yields true if the string 'haystack' starts with the string 'needle'.
 bool startsWith(std::string_view haystack, std::string_view needle) {
  return haystack.size() > needle.size() &&
  haystack.substr(0, needle.size()) == needle;
 }
 
-// Returns true if the string 'haystack' ends with the string 'needle'.
 bool endsWith(std::string_view haystack, std::string_view needle) {
  return haystack.size() > needle.size() &&
  haystack.substr(haystack.size() - needle.size(), needle.size()) == needle;
 }
 
+std::string joinLines(
+ std::vector<std::string> lines,
+ std::string_view separator) {
+ auto concatWithSeparator = [separator](std::string a, const std::string& b) {
+ return std::move(a) + std::string(separator) + b;
+ };
+
+ auto result = std::accumulate(
+ std::next(lines.begin()), lines.end(), lines[0], concatWithSeparator);
+ return result;
+}
+
 } // namespace io::substrait::textplan

src/substrait/textplan/StringManipulation.h

@@ -12,4 +12,9 @@ bool startsWith(std::string_view haystack, std::string_view needle);
 // Returns true if the string 'haystack' ends with the string 'needle'.
 bool endsWith(std::string_view haystack, std::string_view needle);
 
+// Joins a vector of strings into a single string separated by separator.
+std::string joinLines(
+ std::vector<std::string> lines,
+ std::string_view separator="\n");
+
 } // namespace io::substrait::textplan

src/substrait/textplan/converter/LoadBinary.cpp

@@ -14,6 +14,7 @@
 #include <vector>
 
 #include "substrait/proto/plan.pb.h"
+#include "substrait/textplan/StringManipulation.h"
 
 namespace io::substrait::textplan {
 
@@ -82,8 +83,7 @@ absl::StatusOr<::substrait::proto::Plan> loadFromProtoText(
  parser.RecordErrorsTo(&collector);
  if (!parser.ParseFromString(text, &plan)) {
  auto errors = collector.getErrors();
- return absl::InternalError(
- std::accumulate(errors.begin(), errors.end(), std::string("\n")));
+ return absl::InternalError(joinLines(errors));
  }
  return plan;
 }

src/substrait/textplan/converter/SaveBinary.cpp

@@ -9,6 +9,7 @@
 
 #include <google/protobuf/text_format.h>
 #include <google/protobuf/util/json_util.h>
+#include "substrait/textplan/StringManipulation.h"
 #include "substrait/textplan/SymbolTablePrinter.h"
 #include "substrait/textplan/converter/ParseBinary.h"
 
@@ -72,8 +73,7 @@ absl::Status savePlanToText(
  auto result = parseBinaryPlan(plan);
  auto errors = result.getAllErrors();
  if (!errors.empty()) {
- return absl::UnknownError(
- std::accumulate(errors.begin(), errors.end(), std::string("\n")));
+ return absl::UnknownError(joinLines(errors));
  }
  stream << SymbolTablePrinter::outputToText(result.getSymbolTable());
  stream.close();

src/substrait/textplan/converter/tests/BinaryToTextPlanConversionTest.cpp

@@ -597,9 +597,10 @@ std::vector<TestCase> getTestCases() {
 TEST_P(BinaryToTextPlanConverterTestFixture, Parse) {
  auto [name, input, matcher] = GetParam();
 
- auto planOrError = loadFromText(input);
+ auto planOrError = loadFromProtoText(input);
  if (!planOrError.ok()) {
- ParseResult result(SymbolTable(), planOrError.errors(), {});
+ ParseResult result(
+ SymbolTable(), {std::string(planOrError.status().message())}, {});
  ASSERT_THAT(result, matcher);
  return;
  }
@@ -627,13 +628,15 @@ INSTANTIATE_TEST_SUITE_P(
 class BinaryToTextPlanConversionTest : public ::testing::Test {};
 
 TEST_F(BinaryToTextPlanConversionTest, FullSample) {
- std::string json = readFromFile("data/q6_first_stage.json");
- auto planOrError = loadFromJson(json);
+ auto jsonOrError = readFromFile("data/q6_first_stage.json");
+ ASSERT_TRUE(jsonOrError.ok());
+ auto planOrError = loadFromJson(*jsonOrError);
  ASSERT_TRUE(planOrError.ok());
  auto plan = *planOrError;
  EXPECT_THAT(plan.extensions_size(), ::testing::Eq(7));
 
- std::string expectedOutput = readFromFile("data/q6_first_stage.golden.splan");
+ auto expectedOutputOrError = readFromFile("data/q6_first_stage.golden.splan");
+ ASSERT_TRUE(expectedOutputOrError.ok());
 
  auto result = parseBinaryPlan(plan);
  auto symbols = result.getSymbolTable().getSymbols();
@@ -668,7 +671,7 @@ TEST_F(BinaryToTextPlanConversionTest, FullSample) {
  SymbolType::kSource,
  SymbolType::kSchema,
  }),
- WhenSerialized(EqSquashingWhitespace(expectedOutput))))
+ WhenSerialized(EqSquashingWhitespace(*expectedOutputOrError))))
  << result.getSymbolTable().toDebugString();
 }
 

src/substrait/textplan/parser/LoadText.cpp

@@ -3,6 +3,7 @@
 #include "substrait/textplan/parser/LoadText.h"
 
 #include "substrait/proto/plan.pb.h"
+#include "substrait/textplan/StringManipulation.h"
 #include "substrait/textplan/SymbolTablePrinter.h"
 #include "substrait/textplan/parser/ParseText.h"
 
@@ -13,8 +14,7 @@ absl::StatusOr<::substrait::proto::Plan> loadFromText(std::string_view text) {
  auto parseResult = io::substrait::textplan::parseStream(stream);
  if (!parseResult.successful()) {
  auto errors = parseResult.getAllErrors();
- return absl::UnknownError(
- std::accumulate(errors.begin(), errors.end(), std::string("\n")));
+ return absl::UnknownError(joinLines(errors));
  }
 
  return SymbolTablePrinter::outputToBinaryPlan(parseResult.getSymbolTable());