From f6ed071416c84bcd4e29fc249015a4e9ae7264ac Mon Sep 17 00:00:00 2001 From: David Sisson Date: Wed, 26 Jul 2023 19:24:04 -0700 Subject: [PATCH] Switched back to string from string_view to make it easier to use Google's zero copy stream. --- src/substrait/common/Io.cpp | 2 +- src/substrait/common/tests/IoTest.cpp | 1 + src/substrait/textplan/converter/LoadBinary.cpp | 6 +++--- src/substrait/textplan/converter/LoadBinary.h | 7 ++++--- src/substrait/textplan/parser/LoadText.cpp | 2 +- src/substrait/textplan/parser/LoadText.h | 3 +-- 6 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/substrait/common/Io.cpp b/src/substrait/common/Io.cpp index 51b2ec61..d017a58c 100644 --- a/src/substrait/common/Io.cpp +++ b/src/substrait/common/Io.cpp @@ -37,7 +37,7 @@ PlanFileEncoding detectEncoding(std::string_view content) { absl::StatusOr<::substrait::proto::Plan> loadPlanWithUnknownEncoding( std::string_view input_filename) { - auto contentOrError = textplan::readFromFile(input_filename); + auto contentOrError = textplan::readFromFile(input_filename.data()); if (!contentOrError.ok()) { return contentOrError.status(); } diff --git a/src/substrait/common/tests/IoTest.cpp b/src/substrait/common/tests/IoTest.cpp index 157cc5cf..a9e7f13f 100644 --- a/src/substrait/common/tests/IoTest.cpp +++ b/src/substrait/common/tests/IoTest.cpp @@ -25,6 +25,7 @@ constexpr const char* planFileEncodingToString(PlanFileEncoding e) noexcept { case PlanFileEncoding::kText: return "kText"; } + return "IMPOSSIBLE"; } } // namespace diff --git a/src/substrait/textplan/converter/LoadBinary.cpp b/src/substrait/textplan/converter/LoadBinary.cpp index 3c6b315b..65e5694c 100644 --- a/src/substrait/textplan/converter/LoadBinary.cpp +++ b/src/substrait/textplan/converter/LoadBinary.cpp @@ -52,7 +52,7 @@ absl::StatusOr readFromFile(std::string_view msgPath) { return buffer.str(); } -absl::StatusOr<::substrait::proto::Plan> loadFromJson(std::string_view json) { +absl::StatusOr<::substrait::proto::Plan> loadFromJson(const std::string& json) { if (json.empty()) { return absl::InternalError("Provided JSON string was empty."); } @@ -76,7 +76,7 @@ absl::StatusOr<::substrait::proto::Plan> loadFromJson(std::string_view json) { } absl::StatusOr<::substrait::proto::Plan> loadFromProtoText( - std::string_view text) { + const std::string& text) { ::substrait::proto::Plan plan; ::google::protobuf::TextFormat::Parser parser; StringErrorCollector collector; @@ -89,7 +89,7 @@ absl::StatusOr<::substrait::proto::Plan> loadFromProtoText( } absl::StatusOr<::substrait::proto::Plan> loadFromBinary( - std::string_view bytes) { + const std::string& bytes) { ::substrait::proto::Plan plan; if (!plan.ParseFromString(bytes)) { return absl::InternalError("Failed to parse as a binary Substrait plan."); diff --git a/src/substrait/textplan/converter/LoadBinary.h b/src/substrait/textplan/converter/LoadBinary.h index 8c8bdb5c..a4d4e38f 100644 --- a/src/substrait/textplan/converter/LoadBinary.h +++ b/src/substrait/textplan/converter/LoadBinary.h @@ -17,15 +17,16 @@ absl::StatusOr 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. -absl::StatusOr<::substrait::proto::Plan> loadFromJson(std::string_view json); +absl::StatusOr<::substrait::proto::Plan> loadFromJson(const std::string& json); // Reads a plan encoded as a text protobuf. // Returns a list of errors if the file cannot be parsed. absl::StatusOr<::substrait::proto::Plan> loadFromProtoText( - std::string_view text); + const std::string& text); // Reads a plan serialized as a binary protobuf. // Returns a list of errors if the file cannot be parsed. -absl::StatusOr<::substrait::proto::Plan> loadFromBinary(std::string_view bytes); +absl::StatusOr<::substrait::proto::Plan> loadFromBinary( + const std::string& bytes); } // namespace io::substrait::textplan diff --git a/src/substrait/textplan/parser/LoadText.cpp b/src/substrait/textplan/parser/LoadText.cpp index ee2c7078..0850befe 100644 --- a/src/substrait/textplan/parser/LoadText.cpp +++ b/src/substrait/textplan/parser/LoadText.cpp @@ -9,7 +9,7 @@ namespace io::substrait::textplan { -absl::StatusOr<::substrait::proto::Plan> loadFromText(std::string_view text) { +absl::StatusOr<::substrait::proto::Plan> loadFromText(const std::string& text) { auto stream = loadTextString(text); auto parseResult = io::substrait::textplan::parseStream(stream); if (!parseResult.successful()) { diff --git a/src/substrait/textplan/parser/LoadText.h b/src/substrait/textplan/parser/LoadText.h index a5894731..3b2f900b 100644 --- a/src/substrait/textplan/parser/LoadText.h +++ b/src/substrait/textplan/parser/LoadText.h @@ -3,7 +3,6 @@ #pragma once #include -#include namespace substrait::proto { class Plan; @@ -13,6 +12,6 @@ namespace io::substrait::textplan { // Reads a plan encoded as a text protobuf. // Returns a list of errors if the text cannot be parsed. -absl::StatusOr<::substrait::proto::Plan> loadFromText(std::string_view text); +absl::StatusOr<::substrait::proto::Plan> loadFromText(const std::string& text); } // namespace io::substrait::textplan