YT: Switch TQueryBuilder to std::string

commit_hash:b9ba9e739dc3af4d562ef56bd81774e2c8919eed

5 files changed, 139 insertions, 77 deletions

diff --git a/yt/yt/client/query_client/query_builder.cpp b/yt/yt/client/query_client/query_builder.cpp
index dd453632916..bd47e87fac3 100644
--- a/yt/yt/client/query_client/query_builder.cpp
+++ b/yt/yt/client/query_client/query_builder.cpp
@@ -9,26 +9,25 @@ namespace NYT::NQueryClient {
 
 ////////////////////////////////////////////////////////////////////////////////
 
-static std::vector<TString> Parenthesize(std::vector<TString> strings)
+static void Parenthesize(TStringBuilderBase* builder, const std::string& str)
 {
-    for (auto& string : strings) {
-        string.prepend('(').append(')');
-    }
-    return strings;
+    builder->AppendChar('(');
+    builder->AppendString(str);
+    builder->AppendChar(')');
 }
 
-void TQueryBuilder::SetSource(TString source)
+void TQueryBuilder::SetSource(std::string source)
 {
     Source_ = std::move(source);
 }
 
-void TQueryBuilder::SetSource(TString source, TString alias)
+void TQueryBuilder::SetSource(std::string source, std::string alias)
 {
     Source_ = std::move(source);
     SourceAlias_ = std::move(alias);
 }
 
-int TQueryBuilder::AddSelectExpression(TString expression)
+int TQueryBuilder::AddSelectExpression(std::string expression)
 {
     SelectEntries_.push_back(TEntryWithAlias{
         std::move(expression),
@@ -37,7 +36,7 @@ int TQueryBuilder::AddSelectExpression(TString expression)
     return SelectEntries_.size() - 1;
 }
 
-int TQueryBuilder::AddSelectExpression(TString expression, TString alias)
+int TQueryBuilder::AddSelectExpression(std::string expression, std::string alias)
 {
     SelectEntries_.push_back(TEntryWithAlias{
         std::move(expression),
@@ -46,12 +45,12 @@ int TQueryBuilder::AddSelectExpression(TString expression, TString alias)
     return SelectEntries_.size() - 1;
 }
 
-void TQueryBuilder::AddWhereConjunct(TString expression)
+void TQueryBuilder::AddWhereConjunct(std::string expression)
 {
     WhereConjuncts_.push_back(std::move(expression));
 }
 
-void TQueryBuilder::AddGroupByExpression(TString expression)
+void TQueryBuilder::AddGroupByExpression(std::string expression)
 {
     GroupByEntries_.push_back(TEntryWithAlias{
         std::move(expression),
@@ -59,7 +58,7 @@ void TQueryBuilder::AddGroupByExpression(TString expression)
     });
 }
 
-void TQueryBuilder::AddGroupByExpression(TString expression, TString alias)
+void TQueryBuilder::AddGroupByExpression(std::string expression, std::string alias)
 {
     GroupByEntries_.push_back(TEntryWithAlias{
         std::move(expression),
@@ -72,12 +71,12 @@ void TQueryBuilder::SetWithTotals(EWithTotalsMode withTotalsMode)
     WithTotalsMode_ = withTotalsMode;
 }
 
-void TQueryBuilder::AddHavingConjunct(TString expression)
+void TQueryBuilder::AddHavingConjunct(std::string expression)
 {
     HavingConjuncts_.push_back(std::move(expression));
 }
 
-void TQueryBuilder::AddOrderByExpression(TString expression)
+void TQueryBuilder::AddOrderByExpression(std::string expression)
 {
     OrderByEntries_.push_back(TOrderByEntry{
         std::move(expression),
@@ -85,7 +84,7 @@ void TQueryBuilder::AddOrderByExpression(TString expression)
     });
 }
 
-void TQueryBuilder::AddOrderByExpression(TString expression, std::optional<EOrderByDirection> direction)
+void TQueryBuilder::AddOrderByExpression(std::string expression, std::optional<EOrderByDirection> direction)
 {
     OrderByEntries_.push_back(TOrderByEntry{
         std::move(expression),
@@ -93,12 +92,12 @@ void TQueryBuilder::AddOrderByExpression(TString expression, std::optional<EOrde
     });
 }
 
-void TQueryBuilder::AddOrderByAscendingExpression(TString expression)
+void TQueryBuilder::AddOrderByAscendingExpression(std::string expression)
 {
     AddOrderByExpression(std::move(expression), EOrderByDirection::Ascending);
 }
 
-void TQueryBuilder::AddOrderByDescendingExpression(TString expression)
+void TQueryBuilder::AddOrderByDescendingExpression(std::string expression)
 {
     AddOrderByExpression(std::move(expression), EOrderByDirection::Descending);
 }
@@ -114,9 +113,9 @@ void TQueryBuilder::SetLimit(i64 limit)
 }
 
 void TQueryBuilder::AddJoinExpression(
-    TString table,
-    TString alias,
-    TString onExpression,
+    std::string table,
+    std::string alias,
+    std::string onExpression,
     ETableJoinType type)
 {
     JoinEntries_.push_back(TJoinEntry{
@@ -127,94 +126,98 @@ void TQueryBuilder::AddJoinExpression(
     });
 }
 
-TString TQueryBuilder::Build()
+std::string TQueryBuilder::Build()
 {
-    std::vector<TString> parts;
-    parts.reserve(8);
+    TStringBuilder builder;
+    TDelimitedStringBuilderWrapper wrapper(&builder, " ");
 
     if (SelectEntries_.empty()) {
         THROW_ERROR_EXCEPTION("Query must have at least one SELECT expression");
     }
-    parts.push_back(JoinSeq(", ", SelectEntries_));
+    JoinToString(&wrapper, SelectEntries_.begin(), SelectEntries_.end(), &FormatEntryWithAlias);
 
     if (!Source_) {
         THROW_ERROR_EXCEPTION("Source must be specified in query");
     }
     if (!SourceAlias_) {
-        parts.push_back(Format("FROM [%v]", *Source_));
+        wrapper->AppendFormat("FROM [%v]", *Source_);
     } else {
-        parts.push_back(Format("FROM [%v] AS %v", *Source_, *SourceAlias_));
+        wrapper->AppendFormat("FROM [%v] AS %v", *Source_, *SourceAlias_);
     }
 
     for (const auto& join : JoinEntries_) {
         TStringBuf joinType = join.Type == ETableJoinType::Inner ? "JOIN" : "LEFT JOIN";
-        parts.push_back(Format("%v [%v] AS [%v] ON %v", joinType, join.Table, join.Alias, join.OnExpression));
+        wrapper->AppendFormat("%v [%v] AS [%v] ON %v", joinType, join.Table, join.Alias, join.OnExpression);
     }
 
     if (!WhereConjuncts_.empty()) {
-        parts.push_back("WHERE");
-        parts.push_back(JoinSeq(" AND ", Parenthesize(WhereConjuncts_)));
+        wrapper->AppendFormat("WHERE");
+        JoinToString(&wrapper, WhereConjuncts_.begin(), WhereConjuncts_.end(), &Parenthesize, " AND ");
     }
 
     if (!GroupByEntries_.empty()) {
-        parts.push_back("GROUP BY");
-        parts.push_back(JoinSeq(", ", GroupByEntries_));
+        wrapper->AppendString("GROUP BY");
+        JoinToString(&wrapper, GroupByEntries_.begin(), GroupByEntries_.end(), &FormatEntryWithAlias);
     }
 
     if (WithTotalsMode_ == EWithTotalsMode::BeforeHaving) {
-        parts.push_back("WITH TOTALS");
+        wrapper->AppendString("WITH TOTALS");
     }
 
     if (!HavingConjuncts_.empty()) {
         if (GroupByEntries_.empty()) {
             THROW_ERROR_EXCEPTION("Having without group by is not valid");
         }
-        parts.push_back("HAVING");
-        parts.push_back(JoinSeq(" AND ", Parenthesize(HavingConjuncts_)));
+        wrapper->AppendString("HAVING");
+        JoinToString(&wrapper, HavingConjuncts_.begin(), HavingConjuncts_.end(), &Parenthesize, " AND ");
     }
 
     if (WithTotalsMode_ == EWithTotalsMode::AfterHaving) {
-        parts.push_back("WITH TOTALS");
+        wrapper->AppendString("WITH TOTALS");
     }
 
     if (!OrderByEntries_.empty()) {
-        parts.push_back("ORDER BY");
-        parts.push_back(JoinSeq(", ", OrderByEntries_));
+        wrapper->AppendString("ORDER BY");
+        JoinToString(&wrapper, OrderByEntries_.begin(), OrderByEntries_.end(), &FormatOrderByEntry);
     }
 
     if (Offset_) {
-        parts.push_back(Format("OFFSET %v", *Offset_));
+        wrapper->AppendFormat("OFFSET %v", *Offset_);
     }
 
     if (Limit_) {
-        parts.push_back(Format("LIMIT %v", *Limit_));
+        wrapper->AppendFormat("LIMIT %v", *Limit_);
     }
 
-    return JoinSeq(" ", parts);
+    return builder.Flush();
 }
 
-void AppendToString(TString& dst, const TQueryBuilder::TEntryWithAlias& entry)
+void TQueryBuilder::FormatEntryWithAlias(TStringBuilderBase* builder, const TQueryBuilder::TEntryWithAlias& entry)
 {
-    TStringOutput output(dst);
     if (entry.Expression == "*") {
-        output << "*";
+        builder->AppendChar('*');
         return;
     }
-    output << '(' << entry.Expression << ')';
+    builder->AppendChar('(');
+    builder->AppendString(entry.Expression);
+    builder->AppendChar(')');
     if (entry.Alias) {
-        output << " AS " << *entry.Alias;
+        builder->AppendString(" AS ");
+        builder->AppendString(*entry.Alias);
     }
 }
 
-void AppendToString(TString& dst, const TQueryBuilder::TOrderByEntry& entry)
+void TQueryBuilder::FormatOrderByEntry(TStringBuilderBase* builder, const TQueryBuilder::TOrderByEntry& entry)
 {
-    TStringOutput output(dst);
-    output << '(' << entry.Expression << ')';
+    builder->AppendChar('(');
+    builder->AppendString(entry.Expression);
+    builder->AppendChar(')');
     if (entry.Direction) {
         TStringBuf directionString = (*entry.Direction == EOrderByDirection::Ascending)
             ? "ASC"
             : "DESC";
-        output << ' ' << directionString;
+        builder->AppendChar(' ');
+        builder->AppendString(directionString);
     }
 }
 
diff --git a/yt/yt/client/query_client/query_builder.h b/yt/yt/client/query_client/query_builder.h
index 3cf1e04af30..9719d9ede0e 100644
--- a/yt/yt/client/query_client/query_builder.h
+++ b/yt/yt/client/query_client/query_builder.h
@@ -29,72 +29,70 @@ DEFINE_ENUM(EWithTotalsMode,
 class TQueryBuilder
 {
 public:
-    void SetSource(TString source);
-    void SetSource(TString source, TString alias);
+    void SetSource(std::string source);
+    void SetSource(std::string source, std::string alias);
 
-    int AddSelectExpression(TString expression);
-    int AddSelectExpression(TString expression, TString alias);
+    int AddSelectExpression(std::string expression);
+    int AddSelectExpression(std::string expression, std::string alias);
 
-    void AddWhereConjunct(TString expression);
+    void AddWhereConjunct(std::string expression);
 
-    void AddGroupByExpression(TString expression);
-    void AddGroupByExpression(TString expression, TString alias);
+    void AddGroupByExpression(std::string expression);
+    void AddGroupByExpression(std::string expression, std::string alias);
 
     void SetWithTotals(EWithTotalsMode withTotalsMode);
 
-    void AddHavingConjunct(TString expression);
+    void AddHavingConjunct(std::string expression);
 
-    void AddOrderByExpression(TString expression);
-    void AddOrderByExpression(TString expression, std::optional<EOrderByDirection> direction);
+    void AddOrderByExpression(std::string expression);
+    void AddOrderByExpression(std::string expression, std::optional<EOrderByDirection> direction);
 
-    void AddOrderByAscendingExpression(TString expression);
-    void AddOrderByDescendingExpression(TString expression);
+    void AddOrderByAscendingExpression(std::string expression);
+    void AddOrderByDescendingExpression(std::string expression);
 
-    void AddJoinExpression(TString table, TString alias, TString onExpression, ETableJoinType type);
+    void AddJoinExpression(std::string table, std::string alias, std::string onExpression, ETableJoinType type);
 
     void SetOffset(i64 offset);
     void SetLimit(i64 limit);
 
-    TString Build();
+    std::string Build();
 
 private:
     struct TEntryWithAlias
     {
-        TString Expression;
-        std::optional<TString> Alias;
+        std::string Expression;
+        std::optional<std::string> Alias;
     };
 
     struct TOrderByEntry
     {
-        TString Expression;
+        std::string Expression;
         std::optional<EOrderByDirection> Direction;
     };
 
     struct TJoinEntry
     {
-        TString Table;
-        TString Alias;
-        TString OnExpression;
+        std::string Table;
+        std::string Alias;
+        std::string OnExpression;
         ETableJoinType Type;
     };
 
 private:
-    std::optional<TString> Source_;
-    std::optional<TString> SourceAlias_;
+    std::optional<std::string> Source_;
+    std::optional<std::string> SourceAlias_;
     std::vector<TEntryWithAlias> SelectEntries_;
-    std::vector<TString> WhereConjuncts_;
+    std::vector<std::string> WhereConjuncts_;
     std::vector<TOrderByEntry> OrderByEntries_;
     std::vector<TEntryWithAlias> GroupByEntries_;
     EWithTotalsMode WithTotalsMode_ = EWithTotalsMode::None;
-    std::vector<TString> HavingConjuncts_;
+    std::vector<std::string> HavingConjuncts_;
     std::vector<TJoinEntry> JoinEntries_;
     std::optional<i64> Offset_;
     std::optional<i64> Limit_;
 
-private:
-    // We overload this functions to allow the corresponding JoinSeq().
-    friend void AppendToString(TString& dst, const TEntryWithAlias& entry);
-    friend void AppendToString(TString& dst, const TOrderByEntry& entry);
+    static void FormatEntryWithAlias(TStringBuilderBase* builder, const TEntryWithAlias& entry);
+    static void FormatOrderByEntry(TStringBuilderBase* builder, const TOrderByEntry& entry);
 };
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/yt/yt/client/query_client/unittests/query_builder_ut.cpp b/yt/yt/client/query_client/unittests/query_builder_ut.cpp
new file mode 100644
index 00000000000..78c0306a6c9
--- /dev/null
+++ b/yt/yt/client/query_client/unittests/query_builder_ut.cpp
@@ -0,0 +1,43 @@
+#include <yt/yt/client/query_client/query_builder.h>
+
+#include <yt/yt/core/test_framework/framework.h>
+
+#include <gtest/gtest.h>
+
+namespace NYT::NQueryClient {
+namespace {
+
+////////////////////////////////////////////////////////////////////////////////
+
+TEST(TQueryBuilderTest, Build)
+{
+    TQueryBuilder builder;
+
+    builder.SetLimit(10);
+    builder.SetOffset(15);
+    builder.SetSource("fooTable");
+
+    builder.AddWhereConjunct("[id] = 42");
+    builder.AddWhereConjunct("[some_other_field] < 15");
+
+    builder.AddSelectExpression("[some_field] * [some_other_field]", "res");
+    builder.AddOrderByExpression("[res]", EOrderByDirection::Descending);
+    builder.AddOrderByExpression("[some_field]", EOrderByDirection::Ascending);
+
+    builder.AddJoinExpression("barTable", "bar", "fooTable.[id] = barTable.[id]", ETableJoinType::Left);
+
+    auto source = builder.Build();
+
+    EXPECT_NE(source.find("FROM [fooTable]"), source.npos);
+    EXPECT_NE(source.find("LIMIT 10"), source.npos);
+    EXPECT_NE(source.find("OFFSET 15"), source.npos);
+    EXPECT_NE(source.find("ORDER BY ([res]) DESC, ([some_field]) ASC"), source.npos);
+    EXPECT_NE(source.find("WHERE ([id] = 42) AND ([some_other_field] < 15)"), source.npos);
+    EXPECT_NE(source.find("([some_field] * [some_other_field]) AS res"), source.npos);
+    EXPECT_NE(source.find("LEFT JOIN [barTable] AS [bar] ON fooTable.[id] = barTable.[id]"), source.npos);
+};
+
+////////////////////////////////////////////////////////////////////////////////
+
+} // namespace
+} // namespace NYT::NQueryClient
diff --git a/yt/yt/client/query_client/unittests/ya.make b/yt/yt/client/query_client/unittests/ya.make
new file mode 100644
index 00000000000..4155bea266e
--- /dev/null
+++ b/yt/yt/client/query_client/unittests/ya.make
@@ -0,0 +1,17 @@
+GTEST(unittester-client-query-client)
+
+INCLUDE(${ARCADIA_ROOT}/yt/ya_cpp.make.inc)
+
+SRCS(
+    query_builder_ut.cpp
+)
+
+INCLUDE(${ARCADIA_ROOT}/yt/opensource.inc)
+
+PEERDIR(
+    yt/yt/client
+)
+
+SIZE(SMALL)
+
+END()
diff --git a/yt/yt/client/ya.make b/yt/yt/client/ya.make
index ee5c8da6ee2..7387c1ccd14 100644
--- a/yt/yt/client/ya.make
+++ b/yt/yt/client/ya.make
@@ -242,6 +242,7 @@ RECURSE(
 
 RECURSE_FOR_TESTS(
     api/unittests
+    query_client/unittests
     signature/unittests
     table_client/unittests
     unittests