fix(binder): allow multiple DDL in one parsed statement, fix and/or

build_support/build-web-shell.sh

@@ -11,7 +11,7 @@ BUSTUB_PUBLIC_VERSION="$(git -C "${DIR}/../../bustub_public" describe --always -
 mkdir -p cmake-build-wasm
 cd cmake-build-wasm
 emcmake cmake .. -DCMAKE_BUILD_TYPE=MinSizeRel
-make -j wasm-shell
+make -j$(nproc) wasm-shell
 cp -a ../tools/wasm-shell/extra_files/* bin/
 sed -i '' "s|\${BUSTUB_PRIVATE_VERSION}|${BUSTUB_PRIVATE_VERSION}|" bin/index.html
 sed -i '' "s|\${BUSTUB_PUBLIC_VERSION}|${BUSTUB_PUBLIC_VERSION}|" bin/index.html

src/binder/bind_select.cpp

@@ -671,19 +671,26 @@ auto Binder::BindAExpr(duckdb_libpgquery::PGAExpr *root) -> std::unique_ptr<Boun
 auto Binder::BindBoolExpr(duckdb_libpgquery::PGBoolExpr *root) -> std::unique_ptr<BoundExpression> {
   BUSTUB_ASSERT(root, "nullptr");
   switch (root->boolop) {
-    case duckdb_libpgquery::PG_AND_EXPR: {
-      auto exprs = BindExpressionList(root->args);
-      if (exprs.size() != 2) {
-        throw bustub::Exception("AND should have 2 args");
-      }
-      return std::make_unique<BoundBinaryOp>("and", std::move(exprs[0]), std::move(exprs[1]));
-    }
+    case duckdb_libpgquery::PG_AND_EXPR:
     case duckdb_libpgquery::PG_OR_EXPR: {
+      std::string op_name;
+      if (root->boolop == duckdb_libpgquery::PG_AND_EXPR) {
+        op_name = "and";
+      } else if (root->boolop == duckdb_libpgquery::PG_OR_EXPR) {
+        op_name = "or";
+      } else {
+        UNREACHABLE("invalid op");
+      }
+
       auto exprs = BindExpressionList(root->args);
-      if (exprs.size() != 2) {
-        throw bustub::Exception("OR should have 2 args");
+      if (exprs.size() <= 1) {
+        throw bustub::Exception("AND should have at least 1 arg");
+      }
+      auto expr = std::make_unique<BoundBinaryOp>("and", std::move(exprs[0]), std::move(exprs[1]));
+      for (size_t i = 2; i < exprs.size(); i++) {
+        expr = std::make_unique<BoundBinaryOp>("and", std::move(expr), std::move(exprs[i]));
       }
-      return std::make_unique<BoundBinaryOp>("or", std::move(exprs[0]), std::move(exprs[1]));
+      return expr;
     }
     case duckdb_libpgquery::PG_NOT_EXPR: {
       auto exprs = BindExpressionList(root->args);
@@ -748,7 +755,7 @@ auto Binder::BindExplain(duckdb_libpgquery::PGExplainStmt *stmt) -> std::unique_
       }
     }
   }
-  return std::make_unique<ExplainStatement>(TransformStatement(stmt->query), explain_options);
+  return std::make_unique<ExplainStatement>(BindStatement(stmt->query), explain_options);
 }
 
 //===----------------------------------------------------------------------===//

src/binder/binder.cpp

@@ -42,29 +42,20 @@ namespace bustub {
 
 Binder::Binder(const Catalog &catalog) : catalog_(catalog), scope_(nullptr) {}
 
-void Binder::ParseAndBindQuery(const std::string &query) {
-  duckdb::PostgresParser parser;
-  parser.Parse(query);
-  if (!parser.success) {
+void Binder::ParseAndSave(const std::string &query) {
+  parser_.Parse(query);
+  if (!parser_.success) {
     LOG_INFO("Query failed to parse!");
-    throw Exception(fmt::format("Query failed to parse: {}", parser.error_message));
+    throw Exception(fmt::format("Query failed to parse: {}", parser_.error_message));
     return;
   }
 
-  if (parser.parse_tree == nullptr) {
+  if (parser_.parse_tree == nullptr) {
     LOG_INFO("parser received empty statement");
     return;
   }
 
-  statements_ = TransformParseTree(parser.parse_tree);
-
-  if (!statements_.empty()) {
-    auto &last_statement = statements_.back();
-    last_statement->stmt_length_ = query.size() - last_statement->stmt_location_;
-    for (auto &statement : statements_) {
-      statement->query_ = query;
-    }
-  }
+  SaveParseTree(parser_.parse_tree);
 }
 
 auto Binder::IsKeyword(const std::string &text) -> bool { return duckdb::PostgresParser::IsKeyword(text); }

src/binder/transformer.cpp

@@ -41,26 +41,17 @@
 
 namespace bustub {
 
-auto Binder::TransformParseTree(duckdb_libpgquery::PGList *tree) -> std::vector<std::unique_ptr<BoundStatement>> {
+void Binder::SaveParseTree(duckdb_libpgquery::PGList *tree) {
   std::vector<std::unique_ptr<BoundStatement>> statements;
   for (auto entry = tree->head; entry != nullptr; entry = entry->next) {
-    auto stmt = TransformStatement(static_cast<duckdb_libpgquery::PGNode *>(entry->data.ptr_value));
-    statements.push_back(std::move(stmt));
+    statement_nodes_.push_back(reinterpret_cast<duckdb_libpgquery::PGNode *>(entry->data.ptr_value));
   }
-  return statements;
 }
 
-auto Binder::TransformStatement(duckdb_libpgquery::PGNode *stmt) -> std::unique_ptr<BoundStatement> {
+auto Binder::BindStatement(duckdb_libpgquery::PGNode *stmt) -> std::unique_ptr<BoundStatement> {
   switch (stmt->type) {
-    case duckdb_libpgquery::T_PGRawStmt: {
-      auto raw_stmt = reinterpret_cast<duckdb_libpgquery::PGRawStmt *>(stmt);
-      auto result = TransformStatement(raw_stmt->stmt);
-      if (result) {
-        result->stmt_location_ = raw_stmt->stmt_location;
-        result->stmt_length_ = raw_stmt->stmt_len;
-      }
-      return result;
-    }
+    case duckdb_libpgquery::T_PGRawStmt:
+      return BindStatement(reinterpret_cast<duckdb_libpgquery::PGRawStmt *>(stmt)->stmt);
     case duckdb_libpgquery::T_PGCreateStmt:
       return BindCreate(reinterpret_cast<duckdb_libpgquery::PGCreateStmt *>(stmt));
     case duckdb_libpgquery::T_PGInsertStmt:

src/common/bustub_instance.cpp

@@ -156,9 +156,10 @@ void BustubInstance::ExecuteSql(const std::string &sql, ResultWriter &writer) {
   }
 
   bustub::Binder binder(*catalog_);
-  binder.ParseAndBindQuery(sql);
+  binder.ParseAndSave(sql);
 
-  for (const auto &statement : binder.statements_) {
+  for (auto *stmt : binder.statement_nodes_) {
+    auto statement = binder.BindStatement(stmt);
     switch (statement->type_) {
       case StatementType::CREATE_STATEMENT: {
         const auto &create_stmt = dynamic_cast<const CreateStatement &>(*statement);

src/include/binder/binder.h

@@ -86,13 +86,10 @@ class Binder {
  public:
   explicit Binder(const Catalog &catalog);
 
-  /** The parsed SQL statements from an invocation to ParseQuery. */
-  std::vector<std::unique_ptr<BoundStatement>> statements_;
-
   /** Attempts to parse a query into a series of SQL statements. The parsed statements
-   * will be stored in the `statements_` variable.
+   * will be stored in the `statements_nodes_` variable.
    */
-  void ParseAndBindQuery(const std::string &query);
+  void ParseAndSave(const std::string &query);
 
   /** Return true if the given text matches a keyword of the parser. */
   static auto IsKeyword(const std::string &text) -> bool;
@@ -104,10 +101,10 @@ class Binder {
   static auto Tokenize(const std::string &query) -> std::vector<SimplifiedToken>;
 
   /** Transform a Postgres parse tree into a std::vector of SQL Statements. */
-  auto TransformParseTree(duckdb_libpgquery::PGList *tree) -> std::vector<std::unique_ptr<BoundStatement>>;
+  void SaveParseTree(duckdb_libpgquery::PGList *tree);
 
   /** Transform a Postgres statement into a single SQL statement. */
-  auto TransformStatement(duckdb_libpgquery::PGNode *stmt) -> std::unique_ptr<BoundStatement>;
+  auto BindStatement(duckdb_libpgquery::PGNode *stmt) -> std::unique_ptr<BoundStatement>;
 
   /** Get the std::string representation of a Postgres node tag. */
   static auto NodeTagToString(duckdb_libpgquery::PGNodeTag type) -> std::string;
@@ -201,6 +198,9 @@ class Binder {
    */
   auto NewContext() -> ContextGuard { return ContextGuard(&scope_); }
 
+  /** Store all statement parse node */
+  std::vector<duckdb_libpgquery::PGNode *> statement_nodes_;
+
  private:
   /** Catalog will be used during the binding process. USERS SHOULD ENSURE IT OUTLIVES THE BINDER,
    * otherwise it's a dangling reference.
@@ -212,6 +212,8 @@ class Binder {
 
   /** Sometimes we will need to assign a name to some unnamed items. This variable gives them a universal ID. */
   size_t universal_id_{0};
+
+  duckdb::PostgresParser parser_;
 };
 
 }  // namespace bustub

src/include/binder/bound_statement.h

@@ -26,15 +26,6 @@ class BoundStatement {
   /** The statement type. */
   StatementType type_;
 
-  /** The statement location within the query string. */
-  int32_t stmt_location_;
-
-  /** The statement length within the query string. */
-  int32_t stmt_length_;
-
-  /** The query text that corresponds to this SQL statement. */
-  std::string query_;
-
  public:
   /** Render this statement as a string. */
   virtual auto ToString() const -> std::string {

src/include/execution/expressions/arithmetic_expression.h

@@ -67,7 +67,7 @@ class ArithmeticExpression : public AbstractExpression {
 
   /** @return the string representation of the expression node and its children */
   auto ToString() const -> std::string override {
-    return fmt::format("{}{}{}", *GetChildAt(0), compute_type_, *GetChildAt(1));
+    return fmt::format("({}{}{})", *GetChildAt(0), compute_type_, *GetChildAt(1));
   }
 
   BUSTUB_EXPR_CLONE_WITH_CHILDREN(ArithmeticExpression);

src/include/execution/expressions/comparison_expression.h

@@ -51,7 +51,7 @@ class ComparisonExpression : public AbstractExpression {
 
   /** @return the string representation of the expression node and its children */
   auto ToString() const -> std::string override {
-    return fmt::format("{}{}{}", *GetChildAt(0), comp_type_, *GetChildAt(1));
+    return fmt::format("({}{}{})", *GetChildAt(0), comp_type_, *GetChildAt(1));
   }
 
   BUSTUB_EXPR_CLONE_WITH_CHILDREN(ComparisonExpression);

src/include/execution/expressions/logic_expression.h

@@ -59,7 +59,7 @@ class LogicExpression : public AbstractExpression {
 
   /** @return the string representation of the expression node and its children */
   auto ToString() const -> std::string override {
-    return fmt::format("{}{}{}", *GetChildAt(0), logic_type_, *GetChildAt(1));
+    return fmt::format("({}{}{})", *GetChildAt(0), logic_type_, *GetChildAt(1));
   }
 
   BUSTUB_EXPR_CLONE_WITH_CHILDREN(LogicExpression);
@@ -111,10 +111,10 @@ struct fmt::formatter<bustub::LogicType> : formatter<string_view> {
     string_view name;
     switch (c) {
       case bustub::LogicType::And:
-        name = "&&";
+        name = "and";
         break;
       case bustub::LogicType::Or:
-        name = "||";
+        name = "or";
         break;
       default:
         name = "Unknown";

test/binder/binder_test.cpp

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "binder/binder.h"
+#include <memory>
 #include "binder/bound_statement.h"
 #include "catalog/catalog.h"
 #include "gtest/gtest.h"
@@ -48,8 +49,13 @@ auto TryBind(const std::string &query) {
       bustub::Schema(std::vector{bustub::Column{"x", TypeId::VARCHAR, 100}, bustub::Column{"y", TypeId::VARCHAR, 100}}),
       false);
 
-  binder.ParseAndBindQuery(query);
-  return std::move(binder.statements_);
+  binder.ParseAndSave(query);
+  std::vector<std::unique_ptr<BoundStatement>> statements;
+  for (auto *stmt : binder.statement_nodes_) {
+    auto statement = binder.BindStatement(stmt);
+    statements.emplace_back(std::move(statement));
+  }
+  return statements;
 }
 
 void PrintStatements(const std::vector<std::unique_ptr<BoundStatement>> &statements) {