Address Pervaze's review comments.

Author: poojanilangekar

src/catalog/catalog.cpp

@@ -299,7 +299,7 @@ ResultType Catalog::CreateTable(const std::string &database_name,
                                 const std::string &table_name,
                                 std::unique_ptr<catalog::Schema> schema,
                                 concurrency::TransactionContext *txn,
-                                bool is_catalog, oid_t tuples_per_tilegroup,
+                                bool is_catalog, uint32_t tuples_per_tilegroup,
                                 peloton::LayoutType layout_type) {
   if (txn == nullptr)
     throw CatalogException("Do not have transaction to create table " +

src/gc/gc_manager.cpp

@@ -25,8 +25,8 @@ namespace gc {
 // Check a tuple and reclaim all varlen field
 void GCManager::CheckAndReclaimVarlenColumns(storage::TileGroup *tile_group,
                                              oid_t tuple_id) {
-  oid_t tile_count = tile_group->tile_count_;
-  oid_t tile_col_count;
+  uint32_t tile_count = tile_group->tile_count_;
+  uint32_t tile_col_count;
   type::TypeId type_id;
   char *tuple_location;
   char *field_location;

src/include/catalog/catalog.h

@@ -100,7 +100,7 @@ class Catalog {
       const std::string &database_name, const std::string &schema_name,
       const std::string &table_name, std::unique_ptr<catalog::Schema>,
       concurrency::TransactionContext *txn, bool is_catalog = false,
-      oid_t tuples_per_tilegroup = DEFAULT_TUPLES_PER_TILEGROUP,
+      uint32_t tuples_per_tilegroup = DEFAULT_TUPLES_PER_TILEGROUP,
       peloton::LayoutType layout_type = LayoutType::ROW);
 
   // Create index for a table

src/include/storage/layout.h

@@ -75,11 +75,15 @@ class Layout : public Printable {
   /** @brief  Return the layout_oid_ of this object. */
   oid_t GetOid() const { return layout_oid_; }
 
-  /** @brief  Sets the tile id and column id w.r.t. of the tile corresponding
-   *          to the specified tile group column id.
+  /** @brief  Locates the tile and column to which the specified
+   *          specified tile group column_id.
+   *          It updates the tile_id and tile_column_id references.
+   *  @param  column_id         The Id of the column to be located.
+   *  @param  tile_id           A reference to update the tile_id.
+   *  @param  tile_column_id    A reference to update the tile_column_id.
    */
-  void LocateTileAndColumn(oid_t column_offset, oid_t &tile_offset,
-                           oid_t &tile_column_offset) const;
+  void LocateTileAndColumn(oid_t column_id, oid_t &tile_id,
+                           oid_t &tile_column_id) const;
 
   /** @brief    Returns the layout difference w.r.t. the other Layout.
    *  @double   The delta between the layouts. Used by LayoutTuner class.
@@ -93,7 +97,7 @@ class Layout : public Printable {
   oid_t GetTileColumnOffset(oid_t column_id) const;
 
   /** @brief  Returns the number of columns in the layout. */
-  oid_t GetColumnCount() const { return num_columns_; }
+  uint32_t GetColumnCount() const { return num_columns_; }
 
   /** @brief Returns the tile-columns map for each tile in the TileGroup. */
   tile_map_type GetTileMap() const;

src/include/storage/tile_group.h

@@ -118,9 +118,9 @@ class TileGroup : public Printable {
   // this function is called only when building tile groups for aggregation
   // operations.
   // FIXME: GC has recycled some of the tuples, so this count is not accurate
-  oid_t GetActiveTupleCount() const;
+  uint32_t GetActiveTupleCount() const;
 
-  oid_t GetAllocatedTupleCount() const { return num_tuple_slots; }
+  uint32_t GetAllocatedTupleCount() const { return num_tuple_slots_; }
 
   TileGroupHeader *GetHeader() const { return tile_group_header; }
 
@@ -187,10 +187,10 @@ class TileGroup : public Printable {
   AbstractTable *table;  // this design is fantastic!!!
 
   // number of tuple slots allocated
-  oid_t num_tuple_slots;
+  uint32_t num_tuple_slots_;
 
   // number of tiles
-  oid_t tile_count_;
+  uint32_t tile_count_;
 
   std::mutex tile_group_mutex;
 

src/optimizer/stats/tuple_sampler.cpp

@@ -98,7 +98,7 @@ bool TupleSampler::GetTupleInTileGroup(storage::TileGroup *tile_group,
 
     storage::Tile *tile = tile_group->GetTile(tile_itr);
     const catalog::Schema &schema = *(tile->GetSchema());
-    oid_t tile_column_count = schema.GetColumnCount();
+    uint32_t tile_column_count = schema.GetColumnCount();
 
     char *tile_tuple_location = tile->GetTupleLocation(tuple_offset);
     storage::Tuple tile_tuple(&schema, tile_tuple_location);

src/storage/data_table.cpp

@@ -1203,7 +1203,7 @@ std::vector<catalog::Schema> TransformTileGroupSchema(
   // First, get info from the original tile group's schema
   std::map<oid_t, std::map<oid_t, catalog::Column>> schemas;
 
-  oid_t column_count = layout.GetColumnCount();
+  uint32_t column_count = layout.GetColumnCount();
   for (oid_t col_id = 0; col_id < column_count; col_id++) {
     // Get TileGroup layout's tile and offset for col_id.
     tile_group_layout.LocateTileAndColumn(col_id, orig_tile_offset,

src/storage/layout.cpp

@@ -81,34 +81,32 @@ Layout::Layout(const column_map_type &column_map, oid_t layout_id)
   }
 }
 
-// Sets the tile id and column id w.r.t that tile corresponding to
-// the specified tile group column id.
-void Layout::LocateTileAndColumn(oid_t column_offset, oid_t &tile_offset,
-                                 oid_t &tile_column_offset) const {
+void Layout::LocateTileAndColumn(oid_t column_id, oid_t &tile_id,
+                                 oid_t &tile_column_id) const {
   // Ensure that the column_offset is not out of bound
-  PELOTON_ASSERT(num_columns_ > column_offset);
+  PELOTON_ASSERT(num_columns_ > column_id);
 
-  // For row store layout, tile id is always 0 and the tile
-  // column_id and tile column_id is the same.
+  // For row store layout, tile_id is always 0 and the
+  // column_id and tile column_id are the same.
   if (layout_type_ == LayoutType::ROW) {
-    tile_offset = 0;
-    tile_column_offset = column_offset;
+    tile_id = 0;
+    tile_column_id = column_id;
     return;
   }
 
-  // For column store layout, tile_id is always same as column_id
-  // and the tile column_id is always 0.
+  // For column store layout, tile_id is same as column_id
+  // and the tile_column_id is always 0.
   if (layout_type_ == LayoutType::COLUMN) {
-    tile_offset = column_offset;
-    tile_column_offset = 0;
+    tile_id = column_id;
+    tile_column_id = 0;
     return;
   }
 
   // For other layouts, fetch the layout and
   // get the entry in the column map
-  auto entry = column_layout_.at(column_offset);
-  tile_offset = entry.first;
-  tile_column_offset = entry.second;
+  auto entry = column_layout_.at(column_id);
+  tile_id = entry.first;
+  tile_column_id = entry.second;
 }
 
 double Layout::GetLayoutDifference(const storage::Layout &other) const {

src/storage/tile_group.cpp

@@ -39,7 +39,7 @@ TileGroup::TileGroup(BackendType backend_type,
       backend_type(backend_type),
       tile_group_header(tile_group_header),
       table(table),
-      num_tuple_slots(tuple_count),
+      num_tuple_slots_(tuple_count),
       tile_group_layout_(layout) {
   tile_count_ = schemas.size();
   for (oid_t tile_itr = 0; tile_itr < tile_count_; tile_itr++) {
@@ -102,7 +102,7 @@ oid_t TileGroup::GetActiveTupleCount() const {
  */
 void TileGroup::CopyTuple(const Tuple *tuple, const oid_t &tuple_slot_id) {
   LOG_TRACE("Tile Group Id :: %u status :: %u out of %u slots ", tile_group_id,
-            tuple_slot_id, num_tuple_slots);
+            tuple_slot_id, num_tuple_slots_);
 
   oid_t tile_column_count;
   oid_t column_itr = 0;
@@ -136,7 +136,7 @@ oid_t TileGroup::InsertTuple(const Tuple *tuple) {
   oid_t tuple_slot_id = tile_group_header->GetNextEmptyTupleSlot();
 
   LOG_TRACE("Tile Group Id :: %u status :: %u out of %u slots ", tile_group_id,
-            tuple_slot_id, num_tuple_slots);
+            tuple_slot_id, num_tuple_slots_);
 
   // No more slots
   if (tuple_slot_id == INVALID_OID) {
@@ -183,7 +183,7 @@ oid_t TileGroup::InsertTupleFromRecovery(cid_t commit_id, oid_t tuple_slot_id,
   }
 
   LOG_TRACE("Tile Group Id :: %u status :: %u out of %u slots ", tile_group_id,
-            tuple_slot_id, num_tuple_slots);
+            tuple_slot_id, num_tuple_slots_);
 
   oid_t tile_column_count;
   oid_t column_itr = 0;
@@ -282,7 +282,7 @@ oid_t TileGroup::InsertTupleFromCheckpoint(oid_t tuple_slot_id,
   if (status == false) return INVALID_OID;
 
   LOG_TRACE("Tile Group Id :: %u status :: %u out of %u slots ", tile_group_id,
-            tuple_slot_id, num_tuple_slots);
+            tuple_slot_id, num_tuple_slots_);
 
   oid_t tile_column_count;
   oid_t column_itr = 0;

test/codegen/table_scan_translator_test.cpp

@@ -634,9 +634,9 @@ TEST_F(TableScanTranslatorTest, ScanRowLayout) {
   // Creates a table with LayoutType::ROW and
   // invokes the TableScanTranslator
   //
-  oid_t tuples_per_tilegroup = 100;
-  oid_t tilegroup_count = 5;
-  oid_t column_count = 100;
+  uint32_t tuples_per_tilegroup = 100;
+  uint32_t tilegroup_count = 5;
+  uint32_t column_count = 100;
   bool is_inlined = true;
   CreateAndLoadTableWithLayout(LayoutType::ROW, tuples_per_tilegroup,
                                tilegroup_count, column_count, is_inlined);
@@ -648,9 +648,9 @@ TEST_F(TableScanTranslatorTest, ScanColumnLayout) {
   // Creates a table with LayoutType::COLUMN and
   // invokes the TableScanTranslator
   //
-  oid_t tuples_per_tilegroup = 100;
-  oid_t tilegroup_count = 5;
-  oid_t column_count = 100;
+  uint32_t tuples_per_tilegroup = 100;
+  uint32_t tilegroup_count = 5;
+  uint32_t column_count = 100;
   bool is_inlined = true;
   CreateAndLoadTableWithLayout(LayoutType::COLUMN, tuples_per_tilegroup,
                                tilegroup_count, column_count, is_inlined);
@@ -670,7 +670,7 @@ TEST_F(TableScanTranslatorTest, MultiLayoutScan) {
   const int tuples_per_tilegroup = 100;
   const int col_count = 6;
   const bool is_inlined = true;
-  oid_t tuple_count = 100;
+  uint32_t tuple_count = 100;
 
   /////////////////////////////////////////////////////////
   // Define the schema.