GetAffectedIndex: Make input params a const reference

Passing a nullptr sql_statement to this function doesn't make much
sense. So, let's make it a reference.

And while we can do it, let's constify it.

Rename: isDisjoint -> IsDisjoint and make it inline

Add namespace closures

Author: pranavk

src/include/catalog/catalog_cache.h

@@ -20,8 +20,8 @@
 namespace peloton {
 
 namespace planner {
-    class PlanUtil;
-}
+class PlanUtil;
+}  // namespace planner
 
 namespace catalog {
 

src/include/parser/sql_statement.h

@@ -19,10 +19,10 @@
 
 #include <vector>
 
+#include "common/internal_types.h"
 #include "common/macros.h"
 #include "common/printable.h"
 #include "common/sql_node_visitor.h"
-#include "common/internal_types.h"
 
 namespace peloton {
 
@@ -43,7 +43,7 @@ class SQLStatement : public Printable {
 
   virtual ~SQLStatement() {}
 
-  virtual StatementType GetType() { return stmt_type; }
+  virtual StatementType GetType() const { return stmt_type; }
 
   // Get a string representation for debugging
   virtual const std::string GetInfo(int num_indent) const;

src/include/planner/plan_util.h

@@ -19,20 +19,20 @@
 #include "planner/abstract_scan_plan.h"
 #include "planner/delete_plan.h"
 #include "planner/insert_plan.h"
-#include "planner/update_plan.h"
 #include "planner/populate_index_plan.h"
+#include "planner/update_plan.h"
 #include "storage/data_table.h"
 #include "util/string_util.h"
 
 namespace peloton {
 
 namespace catalog {
 class CatalogCache;
-}
+}  // namespace catalog
 
 namespace parser {
 class SQLStatement;
-}
+}  // namespace parser
 
 namespace planner {
 
@@ -60,7 +60,8 @@ class PlanUtil {
   * @return set of affected index object ids
   */
   static const std::set<oid_t> GetAffectedIndexes(
-      catalog::CatalogCache &catalog_cache, parser::SQLStatement *sql_stmt);
+      catalog::CatalogCache &catalog_cache,
+      const parser::SQLStatement &sql_stmt);
 
  private:
   ///

src/include/util/set_util.h

@@ -6,7 +6,7 @@
 //
 // Identification: src/include/util/set_util.h
 //
-// Copyright (c) 2015-18, Carnegie Mellon University Database Group
+// Copyright (c) 2015-2018, Carnegie Mellon University Database Group
 //
 //===----------------------------------------------------------------------===//
 
@@ -19,24 +19,28 @@ namespace peloton {
 class SetUtil {
  public:
   template <typename T>
-  static bool isDisjoint(const std::set<T> &setA, const std::set<T> &setB) {
-    bool disjoint = true;
-    if (setA.empty() || setB.empty()) return disjoint;
-
-    typename std::set<T>::const_iterator setA_it = setA.begin();
-    typename std::set<T>::const_iterator setB_it = setB.begin();
-    while (setA_it != setA.end() && setB_it != setB.end() && disjoint) {
-      if (*setA_it == *setB_it) {
-        disjoint = false;
-      } else if (*setA_it < *setB_it) {
-        setA_it++;
-      } else {
-        setB_it++;
-      }
-    }
+  static bool IsDisjoint(const std::set<T> &setA, const std::set<T> &setB);
+};
+
+template <typename T>
+inline bool SetUtil::IsDisjoint(const std::set<T> &setA,
+                                const std::set<T> &setB) {
+  bool disjoint = true;
+  if (setA.empty() || setB.empty()) return disjoint;
 
-    return disjoint;
+  auto setA_it = setA.begin();
+  auto setB_it = setB.begin();
+  while (setA_it != setA.end() && setB_it != setB.end() && disjoint) {
+    if (*setA_it == *setB_it) {
+      disjoint = false;
+    } else if (*setA_it < *setB_it) {
+      setA_it++;
+    } else {
+      setB_it++;
+    }
   }
-};
+
+  return disjoint;
+}
 
 }  // namespace peloton

src/planner/plan_util.cpp

@@ -6,7 +6,7 @@
 //
 // Identification: src/planner/plan_util.cpp
 //
-// Copyright (c) 2015-18, Carnegie Mellon University Database Group
+// Copyright (c) 2015-2018, Carnegie Mellon University Database Group
 //
 //===----------------------------------------------------------------------===//
 
@@ -29,22 +29,25 @@ namespace peloton {
 namespace planner {
 
 const std::set<oid_t> PlanUtil::GetAffectedIndexes(
-    catalog::CatalogCache &catalog_cache, parser::SQLStatement *sql_stmt) {
+    catalog::CatalogCache &catalog_cache,
+    const parser::SQLStatement &sql_stmt) {
   std::set<oid_t> index_oids;
   std::string db_name, table_name;
-  switch (sql_stmt->GetType()) {
+  switch (sql_stmt.GetType()) {
     // For INSERT, DELETE, all indexes are affected
     case StatementType::INSERT: {
-      auto insert_stmt = static_cast<parser::InsertStatement *>(sql_stmt);
-      db_name = insert_stmt->GetDatabaseName();
-      table_name = insert_stmt->GetTableName();
+      auto &insert_stmt =
+          static_cast<const parser::InsertStatement &>(sql_stmt);
+      db_name = insert_stmt.GetDatabaseName();
+      table_name = insert_stmt.GetTableName();
     }
       PELOTON_FALLTHROUGH;
     case StatementType::DELETE: {
       if (table_name.empty() || db_name.empty()) {
-        auto delete_stmt = static_cast<parser::DeleteStatement *>(sql_stmt);
-        db_name = delete_stmt->GetDatabaseName();
-        table_name = delete_stmt->GetTableName();
+        auto &delete_stmt =
+            static_cast<const parser::DeleteStatement &>(sql_stmt);
+        db_name = delete_stmt.GetDatabaseName();
+        table_name = delete_stmt.GetTableName();
       }
       auto indexes_map = catalog_cache.GetDatabaseObject(db_name)
                              ->GetTableObject(table_name)
@@ -54,13 +57,14 @@ const std::set<oid_t> PlanUtil::GetAffectedIndexes(
       }
     } break;
     case StatementType::UPDATE: {
-      auto update_stmt = static_cast<parser::UpdateStatement *>(sql_stmt);
-      db_name = update_stmt->table->GetDatabaseName();
-      table_name = update_stmt->table->GetTableName();
+      auto &update_stmt =
+          static_cast<const parser::UpdateStatement &>(sql_stmt);
+      db_name = update_stmt.table->GetDatabaseName();
+      table_name = update_stmt.table->GetTableName();
       auto db_object = catalog_cache.GetDatabaseObject(db_name);
       auto table_object = db_object->GetTableObject(table_name);
 
-      auto &update_clauses = update_stmt->updates;
+      auto &update_clauses = update_stmt.updates;
       std::set<oid_t> update_oids;
       for (const auto &update_clause : update_clauses) {
         LOG_TRACE("Affected column name for table(%s) in UPDATE query: %s",
@@ -76,7 +80,7 @@ const std::set<oid_t> PlanUtil::GetAffectedIndexes(
         const std::vector<oid_t> &key_attrs =
             index.second->GetKeyAttrs();  // why it's a vector, and not set?
         const std::set<oid_t> key_attrs_set(key_attrs.begin(), key_attrs.end());
-        if (!SetUtil::isDisjoint(key_attrs_set, update_oids)) {
+        if (!SetUtil::IsDisjoint(key_attrs_set, update_oids)) {
           LOG_TRACE("Index (%s) is affected",
                     index.second->GetIndexName().c_str());
           index_oids.insert(index.first);
@@ -87,7 +91,7 @@ const std::set<oid_t> PlanUtil::GetAffectedIndexes(
       break;
     default:
       LOG_TRACE("Does not support finding affected indexes for query type: %d",
-                static_cast<int>(sql_stmt->GetType()));
+                static_cast<int>(sql_stmt.GetType()));
   }
   return (index_oids);
 }

test/util/set_util_test.cpp

@@ -6,7 +6,7 @@
 //
 // Identification: test/set_util_test.cpp
 //
-// Copyright (c) 2015-18, Carnegie Mellon University Database Group
+// Copyright (c) 2015-2018, Carnegie Mellon University Database Group
 //
 //===----------------------------------------------------------------------===//