Addressed Prashanth's initial comments

Author: poojanilangekar

src/catalog/layout_catalog.cpp

@@ -2,11 +2,11 @@
 //
 //                         Peloton
 //
-// layout_catalog.h
+// layout_catalog.cpp
 //
-// Identification: src/include/catalog/layout_catalog.cpp
+// Identification: src/catalog/layout_catalog.cpp
 //
-// Copyright (c) 2015-18, Carnegie Mellon University Database Group
+// Copyright (c) 2015-2018, Carnegie Mellon University Database Group
 //
 //===----------------------------------------------------------------------===//
 

src/codegen/updater.cpp

@@ -46,7 +46,7 @@ char *Updater::GetDataPtr(uint32_t tile_group_id, uint32_t tuple_offset) {
 
   // Get the tile offset assuming that it is still a row store
   // Hence the Tile offset is 0.
-  auto layout = tile_group->GetLayout();
+  UNUSED_ATTRIBUTE const auto &layout = tile_group->GetLayout();
   PELOTON_ASSERT(layout.IsRowStore());
   tile_ = tile_group->GetTileReference(0);
   return tile_->GetTupleLocation(tuple_offset);

src/include/catalog/layout_catalog.h

@@ -10,21 +10,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-//===----------------------------------------------------------------------===//
-// pg_layout
-//
-// Schema: (column offset: column_name)
-// 0: table_oid (pkey)
-// 1: layout_oid (pkey)
-// 2: num_columns (number of columns in the layout)
-// 3: column_map (map column_oid to <tile_offset, tile_column_oid>)
-//
-// Indexes: (index offset: indexed columns)
-// 0: table_oid & layout_oid (unique & primary key)
-// 1: table_oid (non-unique)
-//
-//===----------------------------------------------------------------------===//
-
 #pragma once
 
 #include "catalog/abstract_catalog.h"

src/include/catalog/table_catalog.h

@@ -39,7 +39,7 @@ namespace peloton {
 
 namespace storage {
 class Layout;
-}
+} // namespace storage
 
 namespace catalog {
 

src/include/codegen/runtime_functions.h

@@ -17,10 +17,6 @@
 
 namespace peloton {
 
-namespace catalog {
-class Schema;
-}
-
 namespace storage {
 class DataTable;
 class TileGroup;

src/include/codegen/tile_group.h

@@ -137,7 +137,7 @@ class TileGroup {
 
     inline const TileGroup &GetTileGroup() const { return tile_group_; }
 
-    inline const ColumnLayout &GetLayout(uint32_t col_idx) const {
+    const ColumnLayout &GetLayout(uint32_t col_idx) const {
       return layout_[col_idx];
     }
 

src/include/common/internal_types.h

@@ -1406,11 +1406,6 @@ typedef std::unordered_set<expression::AbstractExpression *,
  */
 typedef std::map<oid_t, std::pair<oid_t, oid_t>> column_map_type;
 
-/* tile_map_type used to store the mapping between a tile and its columns
- * <tile index> to vector{<original column index, tile column offset>}
- */
-typedef std::map<oid_t, std::vector<std::pair<oid_t, oid_t>>> tile_map_type;
-
 //===--------------------------------------------------------------------===//
 // Wire protocol typedefs
 //===--------------------------------------------------------------------===//

src/include/storage/data_table.h

@@ -249,12 +249,12 @@ class DataTable : public AbstractTable {
 
   void ClearLayoutSamples();
 
-  inline void SetDefaultLayout(std::shared_ptr<const Layout> new_layout) {
+  void SetDefaultLayout(std::shared_ptr<const Layout> new_layout) {
     PELOTON_ASSERT(new_layout->GetColumnCount() == schema->GetColumnCount());
     default_layout_ = new_layout;
   }
 
-  inline void ResetDefaultLayout(LayoutType type = LayoutType::ROW) {
+  void ResetDefaultLayout(LayoutType type = LayoutType::ROW) {
     PELOTON_ASSERT((type == LayoutType::ROW) || (type == LayoutType::COLUMN));
     default_layout_ = std::shared_ptr<const Layout>(
         new const Layout(schema->GetColumnCount(), type));
@@ -438,8 +438,8 @@ class DataTable : public AbstractTable {
   bool dirty_ = false;
 
   // Last used layout_oid. Used while creating new layouts
-  // Initialized to COLUMN_STORE_OID since its the highest predfeined value.
-  std::atomic<oid_t> current_layout_oid_ = ATOMIC_VAR_INIT(COLUMN_STORE_OID);
+  // Initialized to COLUMN_STORE_OID since its the highest predefined value.
+  std::atomic<oid_t> current_layout_oid_;
 
   //===--------------------------------------------------------------------===//
   // TUNING MEMBERS

src/include/storage/layout.h

@@ -6,7 +6,7 @@
 //
 // Identification: src/include/storage/layout.h
 //
-// Copyright (c) 2015-18, Carnegie Mellon University Database Group
+// Copyright (c) 2015-2018, Carnegie Mellon University Database Group
 //
 //===----------------------------------------------------------------------===//
 
@@ -27,10 +27,15 @@ namespace peloton {
 
 namespace catalog {
 class Schema;
-}
+} // namespace catalog
 
 namespace storage {
 
+/** @brief used to store the mapping between a tile and its columns
+ * <tile index> to vector{<original column index, tile column offset>}
+ */
+typedef std::map<oid_t, std::vector<std::pair<oid_t, oid_t>>> tile_map_type;
+
 /**
  * @brief   Class to store the physical layout of a TileGroup.
  */
@@ -62,12 +67,10 @@ class Layout : public Printable {
   Layout(const column_map_type &column_map, oid_t layout_oid);
 
   /** @brief  Check whether this layout is a row store. */
-  inline bool IsRowStore() const { return (layout_type_ == LayoutType::ROW); }
+  bool IsRowStore() const { return (layout_type_ == LayoutType::ROW); }
 
   /** @brief  Check whether this layout is a column store. */
-  inline bool IsColumnStore() const {
-    return (layout_type_ == LayoutType::COLUMN);
-  }
+  bool IsColumnStore() const { return (layout_type_ == LayoutType::COLUMN); }
 
   /** @brief  Return the layout_oid_ of this object. */
   oid_t GetOid() const { return layout_oid_; }

src/storage/data_table.cpp

@@ -67,6 +67,7 @@ DataTable::DataTable(catalog::Schema *schema, const std::string &table_name,
       database_oid(database_oid),
       table_name(table_name),
       tuples_per_tilegroup_(tuples_per_tilegroup),
+      current_layout_oid_(ATOMIC_VAR_INIT(COLUMN_STORE_OID)),
       adapt_table_(adapt_table),
       trigger_list_(new trigger::TriggerList()) {
   if (is_catalog == true) {