Failing LayoutCatalog test because of deserialization

Author: poojanilangekar

src/catalog/catalog.cpp

@@ -181,6 +181,9 @@ void Catalog::BootstrapSystemCatalogs(storage::Database *database,
   system_catalogs->GetTableCatalog()->InsertTable(
       COLUMN_CATALOG_OID, COLUMN_CATALOG_NAME, CATALOG_SCHEMA_NAME,
       database_oid, pool_.get(), txn);
+  system_catalogs->GetTableCatalog()->InsertTable(
+          LAYOUT_CATALOG_OID, LAYOUT_CATALOG_NAME, CATALOG_SCHEMA_NAME,
+          database_oid, pool_.get(), txn);
 }
 
 void Catalog::Bootstrap() {
@@ -557,18 +560,6 @@ ResultType Catalog::CreateIndex(
   return ResultType::SUCCESS;
 }
 
-/*
- *
-  // Create a new layout
-  ResultType CreateLayout(oid_t database_oid, oid_t table_oid,
-                          const column_map_type &column_map,
-                          concurrency::TransactionContext *txn);
-  // Create a new layout and set it as the default for the table
-  ResultType CreateDefaultLayout(oid_t database_oid, oid_t table_oid,
-                                 const column_map_type &column_map,
-                                 concurrency::TransactionContext *txn);
- */
-
 /*
  * @brief   create a new layout for a table
  * @param   database_oid  database to which the table belongs to
@@ -580,8 +571,8 @@ ResultType Catalog::CreateIndex(
  */
 std::shared_ptr<const storage::Layout>
 Catalog::CreateLayout(oid_t database_oid, oid_t table_oid,
-                                 const column_map_type &column_map,
-                                 concurrency::TransactionContext *txn) {
+                      const column_map_type &column_map,
+                      concurrency::TransactionContext *txn) {
 
   auto storage_manager = storage::StorageManager::GetInstance();
   auto database = storage_manager->GetDatabaseWithOid(database_oid);
@@ -617,8 +608,8 @@ Catalog::CreateLayout(oid_t database_oid, oid_t table_oid,
  */
 std::shared_ptr<const storage::Layout>
 Catalog::CreateDefaultLayout(oid_t database_oid, oid_t table_oid,
-                      const column_map_type &column_map,
-                      concurrency::TransactionContext *txn) {
+                             const column_map_type &column_map,
+                             concurrency::TransactionContext *txn) {
   auto new_layout = CreateLayout(database_oid, table_oid, column_map, txn);
   // If the layout creation was successful, set it as the default
   if (new_layout != nullptr) {

src/catalog/layout_catalog.cpp

@@ -14,7 +14,6 @@
 
 #include "catalog/catalog.h"
 #include "catalog/system_catalogs.h"
-#include "catalog/table_catalog.h"
 #include "concurrency/transaction_context.h"
 #include "storage/data_table.h"
 #include "storage/layout.h"
@@ -97,10 +96,10 @@ bool LayoutCatalog::InsertLayout(oid_t table_oid,
   auto val3 = type::ValueFactory::GetVarcharValue(layout->SerializeColumnMap(),
                                                   nullptr);
 
-  tuple->SetValue(ColumnId::TABLE_OID, val0, pool);
-  tuple->SetValue(ColumnId::LAYOUT_OID, val1, pool);
-  tuple->SetValue(ColumnId::NUM_COLUMNS, val2, pool);
-  tuple->SetValue(ColumnId::COLUMN_MAP, val3, pool);
+  tuple->SetValue(LayoutCatalog::ColumnId::TABLE_OID, val0, pool);
+  tuple->SetValue(LayoutCatalog::ColumnId::LAYOUT_OID, val1, pool);
+  tuple->SetValue(LayoutCatalog::ColumnId::NUM_COLUMNS, val2, pool);
+  tuple->SetValue(LayoutCatalog::ColumnId::COLUMN_MAP, val3, pool);
 
   // Insert the tuple
   return InsertTuple(std::move(tuple), txn);
@@ -167,13 +166,16 @@ LayoutCatalog::GetLayouts(oid_t table_oid,
 
   for (auto &tile : (*result_tiles)) {
     for (auto tuple_id : *tile) {
-      oid_t layout_oid = tile->GetValue(tuple_id, ColumnId::LAYOUT_OID)
+      oid_t layout_oid =
+              tile->GetValue(tuple_id, LayoutCatalog::ColumnId::LAYOUT_OID)
               .GetAs<oid_t>();
-      oid_t num_coulmns = tile->GetValue(tuple_id, ColumnId::NUM_COLUMNS)
+      oid_t num_coulmns =
+              tile->GetValue(tuple_id, LayoutCatalog::ColumnId::NUM_COLUMNS)
               .GetAs<oid_t>();
 
-      std::string column_map_str = tile->GetValue(
-              tuple_id, ColumnId::COLUMN_MAP).GetAs<std::string>();
+      std::string column_map_str =
+              tile->GetValue(tuple_id, LayoutCatalog::ColumnId::COLUMN_MAP)
+                      .GetAs<std::string>();
       auto column_map = storage::Layout::DeserializeColumnMap(num_coulmns,
                                                               column_map_str);
       auto layout_object =
@@ -185,5 +187,17 @@ LayoutCatalog::GetLayouts(oid_t table_oid,
   return table_object->GetLayouts();
 }
 
+std::shared_ptr<const storage::Layout>
+LayoutCatalog::GetLayoutWithOid(oid_t table_oid, oid_t layout_oid,
+                 concurrency::TransactionContext *txn) {
+  auto table_layouts = GetLayouts(table_oid, txn);
+  for (const auto &layout_entry : table_layouts) {
+    if (layout_entry.second->GetOid() == layout_oid) {
+      return layout_entry.second;
+    }
+  }
+  return nullptr;
+}
+
 }  // namespace catalog
 }  // namespace peloton

src/include/catalog/layout_catalog.h

@@ -38,8 +38,6 @@ class Layout;
 namespace catalog {
 
 class LayoutCatalog : public AbstractCatalog {
-  friend class TableCatalogObject;
-  friend class Catalog;
 
 public:
   // Global Singleton, only the first call requires passing parameters.
@@ -65,14 +63,18 @@ class LayoutCatalog : public AbstractCatalog {
 
   bool DeleteLayouts(oid_t table_oid, concurrency::TransactionContext *txn);
 
-private:
-
   //===--------------------------------------------------------------------===//
   // Read Related API
   //===--------------------------------------------------------------------===//
   const std::unordered_map<oid_t, std::shared_ptr<const storage::Layout>>
   GetLayouts(oid_t table_oid, concurrency::TransactionContext *txn);
 
+  std::shared_ptr<const storage::Layout>
+  GetLayoutWithOid(oid_t table_oid, oid_t layout_oid,
+                   concurrency::TransactionContext *txn);
+
+private:
+
   std::unique_ptr<catalog::Schema> InitializeSchema();
 
   enum ColumnId {

src/include/storage/layout.h

@@ -60,7 +60,7 @@ class Layout : public Printable {
 
   std::vector<catalog::Schema> GetLayoutSchemas(catalog::Schema* const schema) const;
 
-  std::map<oid_t, oid_t> GetColumnLayoutStats() const;
+  std::map<oid_t, oid_t> GetLayoutStats() const;
 
   std::string SerializeColumnMap() const;
 

src/storage/layout.cpp

@@ -186,7 +186,7 @@ std::vector<catalog::Schema> Layout::GetLayoutSchemas(
   return schemas;
 }
 
-std::map<oid_t, oid_t> Layout::GetColumnLayoutStats() const {
+std::map<oid_t, oid_t> Layout::GetLayoutStats() const {
   std::map<oid_t, oid_t> column_map_stats;
 
   // Cluster per-tile column count

test/catalog/catalog_test.cpp

@@ -15,6 +15,7 @@
 #include "catalog/database_catalog.h"
 #include "catalog/database_metrics_catalog.h"
 #include "catalog/index_catalog.h"
+#include "catalog/layout_catalog.h"
 #include "catalog/query_metrics_catalog.h"
 #include "catalog/system_catalogs.h"
 #include "catalog/table_catalog.h"
@@ -303,5 +304,105 @@ TEST_F(CatalogTests, DroppingCatalog) {
   EXPECT_NE(nullptr, catalog);
 }
 
+TEST_F(CatalogTests, LayoutCatalogTest) {
+  // This test creates a table, changes its layout.
+  // Create another additional layout.
+  // Ensure that default is not changed.
+  // Drops layout and verifies that the default_layout is reset.
+  // It also queries pg_layout to ensure that the entry is removed.
+
+  auto db_name = "temp_db";
+  auto table_name = "temp_table";
+  auto catalog = catalog::Catalog::GetInstance();
+  // Create database.
+  auto &txn_manager = concurrency::TransactionManagerFactory::GetInstance();
+  auto txn = txn_manager.BeginTransaction();
+  catalog->CreateDatabase(db_name, txn);
+
+  // Create table.
+  auto val0 = catalog::Column(
+          type::TypeId::INTEGER, type::Type::GetTypeSize(type::TypeId::INTEGER),
+          "val0", true);
+  auto val1 = catalog::Column(
+          type::TypeId::INTEGER, type::Type::GetTypeSize(type::TypeId::INTEGER),
+          "val1", true);
+  auto val2 = catalog::Column(
+          type::TypeId::INTEGER, type::Type::GetTypeSize(type::TypeId::INTEGER),
+          "val2", true);
+  auto val3 = catalog::Column(
+          type::TypeId::INTEGER, type::Type::GetTypeSize(type::TypeId::INTEGER),
+          "val3", true);
+  std::unique_ptr<catalog::Schema> table_schema(
+          new catalog::Schema({val0, val1, val2, val3}));
+  catalog->CreateTable(db_name, DEFAULT_SCHEMA_NAME,
+                       table_name, std::move(table_schema), txn);
+  txn_manager.CommitTransaction(txn);
+
+  txn = txn_manager.BeginTransaction();
+  auto table = catalog->GetTableWithName(db_name, DEFAULT_SCHEMA_NAME,
+                                         table_name, txn);
+  auto table_oid = table->GetOid();
+  auto database_oid = table->GetDatabaseOid();
+  txn_manager.CommitTransaction(txn);
+
+  // Change default layout.
+
+  std::map<oid_t, std::pair<oid_t, oid_t>> default_map;
+  default_map[0] = std::make_pair(0, 0);
+  default_map[1] = std::make_pair(0, 1);
+  default_map[2] = std::make_pair(1, 0);
+  default_map[3] = std::make_pair(1, 1);
+
+  txn = txn_manager.BeginTransaction();
+  auto default_layout = catalog->CreateDefaultLayout(database_oid, table_oid,
+                                                     default_map, txn);
+  txn_manager.CommitTransaction(txn);
+  EXPECT_NE(nullptr, default_layout);
+
+  // Create additional layout.
+  std::map<oid_t, std::pair<oid_t, oid_t>> non_default_map;
+  non_default_map[0] = std::make_pair(0, 0);
+  non_default_map[1] = std::make_pair(0, 1);
+  non_default_map[2] = std::make_pair(1, 0);
+  non_default_map[3] = std::make_pair(1, 1);
+
+  txn = txn_manager.BeginTransaction();
+  auto other_layout = catalog->CreateLayout(database_oid, table_oid,
+                                                 non_default_map, txn);
+  txn_manager.CommitTransaction(txn);
+
+  // Check that the default layout is still the same.
+  EXPECT_NE(*other_layout.get(), table->GetDefaultLayout());
+
+  // Drop the default layout.
+  auto default_layout_oid = default_layout->GetOid();
+  txn = txn_manager.BeginTransaction();
+  EXPECT_EQ(ResultType::SUCCESS, catalog->DropLayout(
+          database_oid, table_oid, default_layout_oid, txn));
+  txn_manager.CommitTransaction(txn);
+
+  // Check that default layout is reset and set to row_store.
+  EXPECT_NE(*default_layout.get(), table->GetDefaultLayout());
+  EXPECT_TRUE(table->GetDefaultLayout().IsRowStore());
+
+  // Query pg_layout to ensure that the entry is dropped
+  txn = txn_manager.BeginTransaction();
+  auto pg_layout = catalog->
+          GetSystemCatalogs(database_oid)->GetLayoutCatalog();
+  EXPECT_EQ(nullptr,
+            pg_layout->GetLayoutWithOid(table_oid, default_layout_oid, txn));
+
+  // The additional layout must be present in pg_layout
+  auto other_layout_oid = other_layout->GetOid();
+  EXPECT_NE(nullptr,
+            pg_layout->GetLayoutWithOid(table_oid, other_layout_oid, txn));
+  txn_manager.CommitTransaction(txn);
+
+  // Drop database
+  txn = txn_manager.BeginTransaction();
+  catalog->DropDatabaseWithName(db_name, txn);
+  txn_manager.CommitTransaction(txn);
+}
+
 }  // namespace test
 }  // namespace peloton

test/include/executor/testing_executor_util.h

@@ -75,7 +75,11 @@ class TestingExecutorUtil {
       int tuples_per_tilegroup_count = TESTS_TUPLES_PER_TILEGROUP,
       bool indexes = true, oid_t table_oid = INVALID_OID);
 
-  /** @brief Creates a basic table and adds its entry to the catalog */
+  /**
+   * @brief Creates a basic table and adds its entry to the catalog
+   *
+   * @return A pointer to the DataTable created.
+   */
   static storage::DataTable *CreateTableUpdateCatalog(
           int tuples_per_tilegroup_count, std::string &db_name);
 

test/tuning/layout_tuner_test.cpp

@@ -125,6 +125,14 @@ TEST_F(LayoutTunerTests, BasicTest) {
   EXPECT_EQ(new_default_layout.GetTileIdFromColumnId(2),0);
   EXPECT_EQ(new_default_layout.GetTileIdFromColumnId(3),1);
 
+  // Check the per tile stats of the new layout
+  // The layout must contain 2 tiles with the following stats
+  // 0 -> 3
+  // 1 -> 1
+  auto layout_stats = new_default_layout.GetLayoutStats();
+  EXPECT_EQ(layout_stats[0], 3);
+  EXPECT_EQ(layout_stats[1], 1);
+
   TestingExecutorUtil::DeleteDatabase(db_name);
 }