Skip to content
This repository has been archived by the owner on Sep 27, 2019. It is now read-only.

Catalog code cleanup #1414

Merged
merged 10 commits into from
Jun 27, 2018
Merged

Catalog code cleanup #1414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bf70821
Catalog code cleanup
tli2 Jun 18, 2018
6ba5b94
Merge branch 'master' into tianyu-catalog-cleanup
apavlo Jun 19, 2018
39353a2
Address some code review comments.
tli2 Jun 19, 2018
ca376df
Merge branch 'tianyu-catalog-cleanup' of https://github.com/tli2/pelo…
tli2 Jun 19, 2018
14327a4
Merge branch 'master' into tianyu-catalog-cleanup
tli2 Jun 19, 2018
2b04223
Merge branch 'master' into tianyu-catalog-cleanup
apavlo Jun 20, 2018
13dcf3f
Merge branch 'master' of https://github.com/cmu-db/peloton into tiany…
tli2 Jun 27, 2018
364769d
Rename "XXXObject" to "CatalogEntry"
tli2 Jun 27, 2018
85baabb
Rename AddPlpgsqlFunction
tli2 Jun 27, 2018
b8868eb
Merge branch 'master' into tianyu-catalog-cleanup
tli2 Jun 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/binder/bind_node_visitor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ void BindNodeVisitor::Visit(parser::AnalyzeStatement *node) {
void BindNodeVisitor::Visit(expression::TupleValueExpression *expr) {
if (!expr->GetIsBound()) {
std::tuple<oid_t, oid_t, oid_t> col_pos_tuple;
std::shared_ptr<catalog::TableCatalogObject> table_obj = nullptr;
std::shared_ptr<catalog::TableCatalogEntry> table_obj = nullptr;
type::TypeId value_type;
int depth = -1;

Expand Down
16 changes: 9 additions & 7 deletions src/binder/binder_context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ void BinderContext::AddRegularTable(const std::string db_name,
const std::string table_alias,
concurrency::TransactionContext *txn) {
// using catalog object to retrieve meta-data
auto table_object = catalog::Catalog::GetInstance()->GetTableObject(
db_name, schema_name, table_name, txn);
auto table_object = catalog::Catalog::GetInstance()->GetTableCatalogEntry(txn,
db_name,
schema_name,
table_name);

if (regular_table_alias_map_.find(table_alias) !=
regular_table_alias_map_.end() ||
Expand Down Expand Up @@ -79,9 +81,9 @@ void BinderContext::AddNestedTable(

bool BinderContext::GetColumnPosTuple(
const std::string &col_name,
std::shared_ptr<catalog::TableCatalogObject> table_obj,
std::shared_ptr<catalog::TableCatalogEntry> table_obj,
std::tuple<oid_t, oid_t, oid_t> &col_pos_tuple, type::TypeId &value_type) {
auto column_object = table_obj->GetColumnObject(col_name);
auto column_object = table_obj->GetColumnCatalogEntry(col_name);
if (column_object == nullptr) {
return false;
}
Expand Down Expand Up @@ -138,7 +140,7 @@ bool BinderContext::GetColumnPosTuple(

bool BinderContext::GetRegularTableObj(
std::shared_ptr<BinderContext> current_context, std::string &alias,
std::shared_ptr<catalog::TableCatalogObject> &table_obj, int &depth) {
std::shared_ptr<catalog::TableCatalogEntry> &table_obj, int &depth) {
while (current_context != nullptr) {
auto iter = current_context->regular_table_alias_map_.find(alias);
if (iter != current_context->regular_table_alias_map_.end()) {
Expand Down Expand Up @@ -174,9 +176,9 @@ void BinderContext::GenerateAllColumnExpressions(
std::vector<std::unique_ptr<expression::AbstractExpression>> &exprs) {
for (auto &entry : regular_table_alias_map_) {
auto &table_obj = entry.second;
auto col_cnt = table_obj->GetColumnObjects().size();
auto col_cnt = table_obj->GetColumnCatalogEntries().size();
for (size_t i = 0; i < col_cnt; i++) {
auto col_obj = table_obj->GetColumnObject(i);
auto col_obj = table_obj->GetColumnCatalogEntry(i);
auto tv_expr = new expression::TupleValueExpression(
std::string(col_obj->GetColumnName()), std::string(entry.first));
tv_expr->SetValueType(col_obj->GetColumnType());
Expand Down
7 changes: 5 additions & 2 deletions src/brain/query_logger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,11 @@ void QueryLogger::LogQuery(std::string query_string, uint64_t timestamp) {

// Log query + fingerprint
auto &query_history_catalog = catalog::QueryHistoryCatalog::GetInstance();
query_history_catalog.InsertQueryHistory(
query_string, fingerprint.GetFingerprint(), timestamp, nullptr, txn);
query_history_catalog.InsertQueryHistory(txn,
query_string,
fingerprint.GetFingerprint(),
timestamp,
nullptr);

// We're done
txn_manager.CommitTransaction(txn);
Expand Down
70 changes: 40 additions & 30 deletions src/catalog/abstract_catalog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,22 +44,22 @@
namespace peloton {
namespace catalog {

AbstractCatalog::AbstractCatalog(oid_t catalog_table_oid,
std::string catalog_table_name,
AbstractCatalog::AbstractCatalog(storage::Database *pg_catalog,
catalog::Schema *catalog_table_schema,
storage::Database *pg_catalog) {
oid_t catalog_table_oid,
std::string catalog_table_name) {
// set database_oid
database_oid = pg_catalog->GetOid();
database_oid_ = pg_catalog->GetOid();
// Create catalog_table_
catalog_table_ = storage::TableFactory::GetDataTable(
database_oid, catalog_table_oid, catalog_table_schema, catalog_table_name,
database_oid_, catalog_table_oid, catalog_table_schema, catalog_table_name,
DEFAULT_TUPLES_PER_TILEGROUP, true, false, true);
// Add catalog_table_ into pg_catalog database
pg_catalog->AddTable(catalog_table_, true);
}

AbstractCatalog::AbstractCatalog(const std::string &catalog_table_ddl,
concurrency::TransactionContext *txn) {
AbstractCatalog::AbstractCatalog(concurrency::TransactionContext *txn,
const std::string &catalog_table_ddl) {
// get catalog table schema
auto &peloton_parser = parser::PostgresParser::GetInstance();
auto create_plan = std::dynamic_pointer_cast<planner::CreatePlan>(
Expand All @@ -71,21 +71,27 @@ AbstractCatalog::AbstractCatalog(const std::string &catalog_table_ddl,
auto catalog_database_name = create_plan->GetDatabaseName();
PELOTON_ASSERT(catalog_schema_name == std::string(CATALOG_SCHEMA_NAME));
// create catalog table
Catalog::GetInstance()->CreateTable(
catalog_database_name, catalog_schema_name, catalog_table_name,
std::unique_ptr<catalog::Schema>(catalog_table_schema), txn, true);
Catalog::GetInstance()->CreateTable(txn,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless this function also creates the DataTable object (which it shouldn't), maybe we should rename this to CreateTableEntry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Sorry I missed this. I will rename this to TableObject or TableEntry depending on what you think makes sense for the above comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ironically, I think it does create the DataTable object (Not that it should)

catalog_database_name,
catalog_schema_name,
std::unique_ptr<catalog::Schema>(
catalog_table_schema),
catalog_table_name,
true);

// get catalog table oid
auto catalog_table_object = Catalog::GetInstance()->GetTableObject(
catalog_database_name, catalog_schema_name, catalog_table_name, txn);
auto catalog_table_object = Catalog::GetInstance()->GetTableCatalogEntry(txn,
catalog_database_name,
catalog_schema_name,
catalog_table_name);

// set catalog_table_
try {
catalog_table_ = storage::StorageManager::GetInstance()->GetTableWithOid(
catalog_table_object->GetDatabaseOid(),
catalog_table_object->GetTableOid());
// set database_oid
database_oid = catalog_table_object->GetDatabaseOid();
database_oid_ = catalog_table_object->GetDatabaseOid();
} catch (CatalogException &e) {
LOG_TRACE("Can't find table %d! Return false",
catalog_table_object->GetTableOid());
Expand All @@ -97,8 +103,8 @@ AbstractCatalog::AbstractCatalog(const std::string &catalog_table_ddl,
* @param txn TransactionContext
* @return Whether insertion is Successful
*/
bool AbstractCatalog::InsertTuple(std::unique_ptr<storage::Tuple> tuple,
concurrency::TransactionContext *txn) {
bool AbstractCatalog::InsertTuple(concurrency::TransactionContext *txn,
std::unique_ptr<storage::Tuple> tuple) {
if (txn == nullptr)
throw CatalogException("Insert tuple requires transaction");

Expand Down Expand Up @@ -137,9 +143,9 @@ bool AbstractCatalog::InsertTuple(std::unique_ptr<storage::Tuple> tuple,
* @param txn TransactionContext
* @return Whether deletion is Successful
*/
bool AbstractCatalog::DeleteWithIndexScan(
oid_t index_offset, std::vector<type::Value> values,
concurrency::TransactionContext *txn) {
bool AbstractCatalog::DeleteWithIndexScan(concurrency::TransactionContext *txn,
oid_t index_offset,
std::vector<type::Value> values) {
if (txn == nullptr)
throw CatalogException("Delete tuple requires transaction");

Expand Down Expand Up @@ -189,9 +195,10 @@ bool AbstractCatalog::DeleteWithIndexScan(
*/
std::unique_ptr<std::vector<std::unique_ptr<executor::LogicalTile>>>
AbstractCatalog::GetResultWithIndexScan(
std::vector<oid_t> column_offsets, oid_t index_offset,
std::vector<type::Value> values,
concurrency::TransactionContext *txn) const {
concurrency::TransactionContext *txn,
std::vector<oid_t> column_offsets,
oid_t index_offset,
std::vector<type::Value> values) const {
if (txn == nullptr) throw CatalogException("Scan table requires transaction");

// Index scan
Expand Down Expand Up @@ -238,9 +245,10 @@ AbstractCatalog::GetResultWithIndexScan(
* @return Unique pointer of vector of logical tiles
*/
std::unique_ptr<std::vector<std::unique_ptr<executor::LogicalTile>>>
AbstractCatalog::GetResultWithSeqScan(std::vector<oid_t> column_offsets,
expression::AbstractExpression *predicate,
concurrency::TransactionContext *txn) {
AbstractCatalog::GetResultWithSeqScan(
concurrency::TransactionContext *txn,
expression::AbstractExpression *predicate,
std::vector<oid_t> column_offsets) {
if (txn == nullptr) throw CatalogException("Scan table requires transaction");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CatalogException vs. PELOTON_ASSERT.
Should this be an assert? Is it ever legitimate to call this function without a transaction? If not, it should be an ASSERT.


// Sequential scan
Expand Down Expand Up @@ -272,8 +280,9 @@ AbstractCatalog::GetResultWithSeqScan(std::vector<oid_t> column_offsets,
* Note: Use catalog::Catalog::CreateIndex() if you can, only ColumnCatalog and
* IndexCatalog should need this
*/
void AbstractCatalog::AddIndex(const std::vector<oid_t> &key_attrs,
oid_t index_oid, const std::string &index_name,
void AbstractCatalog::AddIndex(const std::string &index_name,
oid_t index_oid,
const std::vector<oid_t> &key_attrs,
IndexConstraintType index_constraint) {
auto schema = catalog_table_->GetSchema();
auto key_schema = catalog::Schema::CopySchema(schema, key_attrs);
Expand Down Expand Up @@ -307,10 +316,11 @@ void AbstractCatalog::AddIndex(const std::vector<oid_t> &key_attrs,
* @param index_offset Offset of index for scan
* @return true if successfully executes
*/
bool AbstractCatalog::UpdateWithIndexScan(
std::vector<oid_t> update_columns, std::vector<type::Value> update_values,
std::vector<type::Value> scan_values, oid_t index_offset,
concurrency::TransactionContext *txn) {
bool AbstractCatalog::UpdateWithIndexScan(concurrency::TransactionContext *txn,
oid_t index_offset,
std::vector<type::Value> scan_values,
std::vector<oid_t> update_columns,
std::vector<type::Value> update_values) {
if (txn == nullptr) throw CatalogException("Scan table requires transaction");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto for exception vs. PELOTON_ASSERT comment above.


std::unique_ptr<executor::ExecutorContext> context(
Expand Down
Loading