This repository was archived by the owner on Sep 27, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 621
Modified pg_column_stats initialization #1352
Open
poojanilangekar
wants to merge
8
commits into
cmu-db:master
Choose a base branch
from
poojanilangekar:column_stats_fix
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+246
−156
Open
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
5926cd4
Modified catalog. Need to fix tests
poojanilangekar 5f431de
Fixed all tests
poojanilangekar c5a0bc9
Fix include statement
poojanilangekar 6ffb0b3
Fixed AnalyzeAllTables
poojanilangekar 71aeb02
Changed AnalyzeAllTables to AnalyzeAllTablesWithDatabaseOid
poojanilangekar 92d95fa
Style fix
poojanilangekar 7dfbc59
Insert test tables into pg_table
poojanilangekar 9429b86
Merge branch 'master' into column_stats_fix
tli2 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Style fix
commit 92d95fa2bb8ed6a5d9fd5bf86421f03cc13f0d08
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,7 +45,6 @@ namespace catalog { | |
|
||
class ColumnStatsCatalog : public AbstractCatalog { | ||
public: | ||
|
||
ColumnStatsCatalog(storage::Database *pg_catalog, type::AbstractPool *pool, | ||
concurrency::TransactionContext *txn); | ||
|
||
|
@@ -54,8 +53,8 @@ class ColumnStatsCatalog : public AbstractCatalog { | |
//===--------------------------------------------------------------------===// | ||
// write Related API | ||
//===--------------------------------------------------------------------===// | ||
bool InsertColumnStats(oid_t table_id, oid_t column_id, | ||
int num_rows, double cardinality, double frac_null, | ||
bool InsertColumnStats(oid_t table_id, oid_t column_id, int num_rows, | ||
double cardinality, double frac_null, | ||
std::string most_common_vals, | ||
std::string most_common_freqs, | ||
std::string histogram_bounds, std::string column_name, | ||
|
@@ -68,16 +67,14 @@ class ColumnStatsCatalog : public AbstractCatalog { | |
// Read-only Related API | ||
//===--------------------------------------------------------------------===// | ||
std::unique_ptr<std::vector<type::Value>> GetColumnStats( | ||
oid_t table_id, oid_t column_id, | ||
concurrency::TransactionContext *txn); | ||
oid_t table_id, oid_t column_id, concurrency::TransactionContext *txn); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add comment document function, args and return information. |
||
|
||
size_t GetTableStats( | ||
oid_t table_id, concurrency::TransactionContext *txn, | ||
std::map<oid_t, std::unique_ptr<std::vector<type::Value>>> & | ||
column_stats_map); | ||
oid_t table_id, concurrency::TransactionContext *txn, | ||
std::map<oid_t, std::unique_ptr<std::vector<type::Value>>> | ||
&column_stats_map); | ||
// TODO: add more if needed | ||
|
||
|
||
/** @brief private function for initialize schema of pg_index | ||
* @return unqiue pointer to schema | ||
*/ | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,8 +68,7 @@ class StatsStorage { | |
/* Functions for triggerring stats collection */ | ||
|
||
ResultType AnalyzeStatsForAllTablesWithDatabaseOid( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add function header describing this function, args, etc. |
||
oid_t database_oid, | ||
concurrency::TransactionContext *txn = nullptr); | ||
oid_t database_oid, concurrency::TransactionContext *txn = nullptr); | ||
|
||
ResultType AnalyzeStatsForTable( | ||
storage::DataTable *table, | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,21 +98,20 @@ void StatsStorage::InsertOrUpdateColumnStats( | |
cardinality, frac_null, most_common_vals.c_str(), | ||
most_common_freqs.c_str(), histogram_bounds.c_str()); | ||
auto pg_column_stats = catalog::Catalog::GetInstance() | ||
->GetSystemCatalogs(database_id) | ||
->GetColumnStatsCatalog(); | ||
->GetSystemCatalogs(database_id) | ||
->GetColumnStatsCatalog(); | ||
auto &txn_manager = concurrency::TransactionManagerFactory::GetInstance(); | ||
|
||
bool single_statement_txn = false; | ||
if (txn == nullptr) { | ||
single_statement_txn = true; | ||
txn = txn_manager.BeginTransaction(); | ||
} | ||
pg_column_stats->DeleteColumnStats(table_id, column_id, | ||
txn); | ||
pg_column_stats->InsertColumnStats( | ||
table_id, column_id, num_rows, cardinality, frac_null, | ||
most_common_vals, most_common_freqs, histogram_bounds, column_name, | ||
has_index, pool_.get(), txn); | ||
pg_column_stats->DeleteColumnStats(table_id, column_id, txn); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DeleteColumnStats if they exist, I assume. May be helpful to add comment stating that, if correct. |
||
pg_column_stats->InsertColumnStats(table_id, column_id, num_rows, cardinality, | ||
frac_null, most_common_vals, | ||
most_common_freqs, histogram_bounds, | ||
column_name, has_index, pool_.get(), txn); | ||
|
||
if (single_statement_txn) { | ||
txn_manager.CommitTransaction(txn); | ||
|
@@ -127,13 +126,13 @@ std::shared_ptr<ColumnStats> StatsStorage::GetColumnStatsByID(oid_t database_id, | |
oid_t table_id, | ||
oid_t column_id) { | ||
auto pg_column_stats = catalog::Catalog::GetInstance() | ||
->GetSystemCatalogs(database_id) | ||
->GetColumnStatsCatalog(); | ||
->GetSystemCatalogs(database_id) | ||
->GetColumnStatsCatalog(); | ||
auto &txn_manager = concurrency::TransactionManagerFactory::GetInstance(); | ||
auto txn = txn_manager.BeginTransaction(); | ||
// std::unique_ptr<std::vector<type::Value>> column_stats_vector | ||
auto column_stats_vector = pg_column_stats->GetColumnStats( | ||
table_id, column_id, txn); | ||
auto column_stats_vector = | ||
pg_column_stats->GetColumnStats(table_id, column_id, txn); | ||
txn_manager.CommitTransaction(txn); | ||
|
||
return ConvertVectorToColumnStats(database_id, table_id, column_id, | ||
|
@@ -211,8 +210,8 @@ std::shared_ptr<ColumnStats> StatsStorage::ConvertVectorToColumnStats( | |
std::shared_ptr<TableStats> StatsStorage::GetTableStats( | ||
oid_t database_id, oid_t table_id, concurrency::TransactionContext *txn) { | ||
auto pg_column_stats = catalog::Catalog::GetInstance() | ||
->GetSystemCatalogs(database_id) | ||
->GetColumnStatsCatalog(); | ||
->GetSystemCatalogs(database_id) | ||
->GetColumnStatsCatalog(); | ||
std::map<oid_t, std::unique_ptr<std::vector<type::Value>>> column_stats_map; | ||
pg_column_stats->GetTableStats(table_id, txn, column_stats_map); | ||
|
||
|
@@ -236,8 +235,8 @@ std::shared_ptr<TableStats> StatsStorage::GetTableStats( | |
oid_t database_id, oid_t table_id, std::vector<oid_t> column_ids, | ||
concurrency::TransactionContext *txn) { | ||
auto pg_column_stats = catalog::Catalog::GetInstance() | ||
->GetSystemCatalogs(database_id) | ||
->GetColumnStatsCatalog(); | ||
->GetSystemCatalogs(database_id) | ||
->GetColumnStatsCatalog(); | ||
std::map<oid_t, std::unique_ptr<std::vector<type::Value>>> column_stats_map; | ||
pg_column_stats->GetTableStats(table_id, txn, column_stats_map); | ||
|
||
|
@@ -258,18 +257,17 @@ std::shared_ptr<TableStats> StatsStorage::GetTableStats( | |
* datatables to collect their stats and store them in the column_stats_catalog. | ||
*/ | ||
ResultType StatsStorage::AnalyzeStatsForAllTablesWithDatabaseOid( | ||
oid_t database_oid, | ||
UNUSED_ATTRIBUTE concurrency::TransactionContext *txn) { | ||
oid_t database_oid, UNUSED_ATTRIBUTE concurrency::TransactionContext *txn) { | ||
if (txn == nullptr) { | ||
LOG_TRACE("Do not have transaction to analyze all tables' stats."); | ||
return ResultType::FAILURE; | ||
} | ||
|
||
auto storage_manager = storage::StorageManager::GetInstance(); | ||
auto database = storage_manager->GetDatabaseWithOid(database); | ||
auto database = storage_manager->GetDatabaseWithOid(database_oid); | ||
PELOTON_ASSERT(database != nullptr); | ||
auto pg_database = catalog::Catalog::GetInstance() | ||
->GetDatabaseObject(database_oid, txn); | ||
auto pg_database = | ||
catalog::Catalog::GetInstance()->GetDatabaseObject(database_oid, txn); | ||
auto table_objects = pg_database->GetTableObjects(); | ||
for (auto &table_object_entry : table_objects) { | ||
auto table_oid = table_object_entry.first; | ||
|
@@ -280,7 +278,7 @@ ResultType StatsStorage::AnalyzeStatsForAllTablesWithDatabaseOid( | |
LOG_TRACE("Analyzing table: %s", table_object->GetTableName().c_str()); | ||
auto table = database->GetTableWithOid(table_oid); | ||
std::unique_ptr<TableStatsCollector> table_stats_collector( | ||
new TableStatsCollector(table)); | ||
new TableStatsCollector(table)); | ||
table_stats_collector->CollectColumnStats(); | ||
InsertOrUpdateTableStats(table, table_stats_collector.get(), txn); | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comment documenting function purpose and arguments.