From ff534dba7fe53f2a497bc8dfaa2dcf7cf226b057 Mon Sep 17 00:00:00 2001 From: Roman Nozdrin Date: Tue, 24 Jan 2023 12:54:50 +0000 Subject: [PATCH] MCOL-5384 This commit replaces shared pointer to CSC with CSC ctor that is cleaned up leaving a scope CSC default ctor was private b/c it must not allow to use CSC outside thread cache. However there are some places in the plugin code that need a standalone syscat that is cleaned up leaving the scope. The decision is to make the restriction mentioned organizational rather than syntactical. --- dbcon/execplan/calpontsystemcatalog.cpp | 6 ++++- dbcon/execplan/calpontsystemcatalog.h | 6 ++++- dbcon/mysql/ha_mcs_client_udfs.cpp | 33 ++++++++++++++----------- dbcon/mysql/ha_mcs_partition.cpp | 32 +++++++++++------------- dbcon/mysql/is_columnstore_columns.cpp | 16 +++++------- dbcon/mysql/is_columnstore_tables.cpp | 13 ++++------ 6 files changed, 54 insertions(+), 52 deletions(-) diff --git a/dbcon/execplan/calpontsystemcatalog.cpp b/dbcon/execplan/calpontsystemcatalog.cpp index 332a076f0..17e09d857 100644 --- a/dbcon/execplan/calpontsystemcatalog.cpp +++ b/dbcon/execplan/calpontsystemcatalog.cpp @@ -1992,7 +1992,11 @@ CalpontSystemCatalog::CalpontSystemCatalog() : fExeMgr(new ClientRotator(0, "Exe CalpontSystemCatalog::~CalpontSystemCatalog() { - delete fExeMgr; + if (fExeMgr) + { + delete fExeMgr; + fExeMgr = nullptr; + } } #if 0 diff --git a/dbcon/execplan/calpontsystemcatalog.h b/dbcon/execplan/calpontsystemcatalog.h index 2f2238c8c..edd9aecd3 100644 --- a/dbcon/execplan/calpontsystemcatalog.h +++ b/dbcon/execplan/calpontsystemcatalog.h @@ -215,6 +215,7 @@ class CalpontSystemCatalog : public datatypes::SystemCatalog uint64_t nextvalue; // next autoincrement value uint32_t charsetNumber; const CHARSET_INFO* cs; + private: long timeZone; @@ -842,9 +843,12 @@ class CalpontSystemCatalog : public datatypes::SystemCatalog /** Destructor */ ~CalpontSystemCatalog(); + // This ctor must be used to produce non-shared CSC that exists for a short period, + // e.g. in client UDFs like calshowpartitions(). + explicit CalpontSystemCatalog(); + private: /** Constuctors */ - explicit CalpontSystemCatalog(); explicit CalpontSystemCatalog(const CalpontSystemCatalog& rhs); CalpontSystemCatalog& operator=(const CalpontSystemCatalog& rhs); diff --git a/dbcon/mysql/ha_mcs_client_udfs.cpp b/dbcon/mysql/ha_mcs_client_udfs.cpp index 16e5290c6..28edcaf2e 100644 --- a/dbcon/mysql/ha_mcs_client_udfs.cpp +++ b/dbcon/mysql/ha_mcs_client_udfs.cpp @@ -38,7 +38,7 @@ using namespace oam; #include "errorids.h" using namespace logging; -//#include "resourcemanager.h" +// #include "resourcemanager.h" #include "columnstoreversion.h" #include "ha_mcs_sysvars.h" @@ -140,7 +140,8 @@ extern "C" std::string pstr(parameter); boost::algorithm::to_lower(pstr); - if (get_fe_conn_info_ptr() == NULL) { + if (get_fe_conn_info_ptr() == NULL) + { set_fe_conn_info_ptr((void*)new cal_connection_info()); thd_set_ha_data(thd, mcs_hton, get_fe_conn_info_ptr()); } @@ -243,7 +244,8 @@ extern "C" const char* mcsgetstats(UDF_INIT* initid, UDF_ARGS* args, char* result, unsigned long* length, char* is_null, char* error) { - if (get_fe_conn_info_ptr() == NULL) { + if (get_fe_conn_info_ptr() == NULL) + { set_fe_conn_info_ptr((void*)new cal_connection_info()); thd_set_ha_data(current_thd, mcs_hton, get_fe_conn_info_ptr()); } @@ -322,7 +324,8 @@ extern "C" #endif long long mcssettrace(UDF_INIT* initid, UDF_ARGS* args, char* is_null, char* error) { - if (get_fe_conn_info_ptr() == NULL) { + if (get_fe_conn_info_ptr() == NULL) + { set_fe_conn_info_ptr((void*)new cal_connection_info()); thd_set_ha_data(current_thd, mcs_hton, get_fe_conn_info_ptr()); } @@ -387,7 +390,7 @@ extern "C" try { - if (dbrm.getSystemReady() > 0 && dbrm.getSystemQueryReady() > 0) + if (dbrm.getSystemReady() > 0 && dbrm.getSystemQueryReady() > 0) { return 1; } @@ -556,7 +559,8 @@ extern "C" { THD* thd = current_thd; - if (get_fe_conn_info_ptr() == NULL) { + if (get_fe_conn_info_ptr() == NULL) + { set_fe_conn_info_ptr((void*)new cal_connection_info()); thd_set_ha_data(thd, mcs_hton, get_fe_conn_info_ptr()); } @@ -665,7 +669,8 @@ extern "C" const char* mcscleartablelock(UDF_INIT* initid, UDF_ARGS* args, char* result, unsigned long* length, char* is_null, char* error) { - if (get_fe_conn_info_ptr() == NULL) { + if (get_fe_conn_info_ptr() == NULL) + { set_fe_conn_info_ptr((void*)new cal_connection_info()); thd_set_ha_data(current_thd, mcs_hton, get_fe_conn_info_ptr()); } @@ -789,14 +794,12 @@ extern "C" boost::algorithm::to_lower(tableName.table); } - boost::shared_ptr csc = - execplan::CalpontSystemCatalog::makeCalpontSystemCatalog( - execplan::CalpontSystemCatalog::idb_tid2sid(thd->thread_id)); - csc->identity(execplan::CalpontSystemCatalog::FE); + execplan::CalpontSystemCatalog csc; + csc.identity(execplan::CalpontSystemCatalog::FE); try { - nextVal = csc->nextAutoIncrValue(tableName); + nextVal = csc.nextAutoIncrValue(tableName); } catch (std::exception&) { @@ -931,7 +934,8 @@ extern "C" } } - if (get_fe_conn_info_ptr() == NULL) { + if (get_fe_conn_info_ptr() == NULL) + { set_fe_conn_info_ptr((void*)new cal_connection_info()); thd_set_ha_data(current_thd, mcs_hton, get_fe_conn_info_ptr()); } @@ -1095,7 +1099,8 @@ extern "C" const char* mcsgetsqlcount(UDF_INIT* initid, UDF_ARGS* args, char* result, unsigned long* length, char* is_null, char* error) { - if (get_fe_conn_info_ptr() == NULL) { + if (get_fe_conn_info_ptr() == NULL) + { set_fe_conn_info_ptr((void*)new cal_connection_info()); thd_set_ha_data(current_thd, mcs_hton, get_fe_conn_info_ptr()); } diff --git a/dbcon/mysql/ha_mcs_partition.cpp b/dbcon/mysql/ha_mcs_partition.cpp index c65c79f46..963064ed6 100644 --- a/dbcon/mysql/ha_mcs_partition.cpp +++ b/dbcon/mysql/ha_mcs_partition.cpp @@ -27,7 +27,7 @@ #include #include #include -//#include +// #include #include using namespace std; @@ -378,13 +378,12 @@ void partitionByValue_common(UDF_ARGS* args, // inp try { - boost::shared_ptr csc = - CalpontSystemCatalog::makeCalpontSystemCatalog(tid2sid(current_thd->thread_id)); - csc->identity(execplan::CalpontSystemCatalog::FE); + CalpontSystemCatalog csc; + csc.identity(execplan::CalpontSystemCatalog::FE); CalpontSystemCatalog::TableColName tcn = make_tcn(schema, table, column, lower_case_table_names); - csc->identity(CalpontSystemCatalog::FE); - OID_t oid = csc->lookupOID(tcn); - CalpontSystemCatalog::ColType ct = csc->colType(oid); + csc.identity(CalpontSystemCatalog::FE); + OID_t oid = csc.lookupOID(tcn); + CalpontSystemCatalog::ColType ct = csc.colType(oid); const char* timeZone = current_thd->variables.time_zone->get_name()->ptr(); long timeZoneOffset; dataconvert::timeZoneToOffset(timeZone, strlen(timeZone), &timeZoneOffset); @@ -626,12 +625,11 @@ extern "C" } boost::algorithm::to_lower(column); - boost::shared_ptr csc = - CalpontSystemCatalog::makeCalpontSystemCatalog(tid2sid(current_thd->thread_id)); - csc->identity(CalpontSystemCatalog::FE); + CalpontSystemCatalog csc; + csc.identity(CalpontSystemCatalog::FE); CalpontSystemCatalog::TableColName tcn = make_tcn(schema, table, column, lower_case_table_names); - OID_t oid = csc->lookupOID(tcn); - ct = csc->colType(oid); + OID_t oid = csc.lookupOID(tcn); + ct = csc.colType(oid); if (oid == -1) { @@ -701,7 +699,6 @@ extern "C" else output << "Enabled"; } - // use our own buffer to make sure it fits. initid->ptr = new char[output.str().length() + 1]; memcpy(initid->ptr, output.str().c_str(), output.str().length()); @@ -1294,12 +1291,11 @@ extern "C" } boost::algorithm::to_lower(column); - boost::shared_ptr csc = - CalpontSystemCatalog::makeCalpontSystemCatalog(tid2sid(current_thd->thread_id)); - csc->identity(CalpontSystemCatalog::FE); + CalpontSystemCatalog csc; + csc.identity(CalpontSystemCatalog::FE); CalpontSystemCatalog::TableColName tcn = make_tcn(schema, table, column, lower_case_table_names); - OID_t oid = csc->lookupOID(tcn); - ct = csc->colType(oid); + OID_t oid = csc.lookupOID(tcn); + ct = csc.colType(oid); if (oid == -1) { diff --git a/dbcon/mysql/is_columnstore_columns.cpp b/dbcon/mysql/is_columnstore_columns.cpp index 4076bbd1d..c45abdf94 100644 --- a/dbcon/mysql/is_columnstore_columns.cpp +++ b/dbcon/mysql/is_columnstore_columns.cpp @@ -60,15 +60,12 @@ static int is_columnstore_columns_fill(THD* thd, TABLE_LIST* tables, COND* cond) TABLE* table = tables->table; InformationSchemaCond isCond; - boost::shared_ptr systemCatalogPtr = - execplan::CalpontSystemCatalog::makeCalpontSystemCatalog( - execplan::CalpontSystemCatalog::idb_tid2sid(thd->thread_id)); - + execplan::CalpontSystemCatalog csc; const std::vector< std::pair > - catalog_tables = systemCatalogPtr->getTables(); + catalog_tables = csc.getTables(); - systemCatalogPtr->identity(execplan::CalpontSystemCatalog::FE); + csc.identity(execplan::CalpontSystemCatalog::FE); if (cond) { @@ -89,7 +86,7 @@ static int is_columnstore_columns_fill(THD* thd, TABLE_LIST* tables, COND* cond) // So simply ignore the dropped table. try { - column_rid_list = systemCatalogPtr->columnRIDs((*it).second, true, lower_case_table_names); + column_rid_list = csc.columnRIDs((*it).second, true, lower_case_table_names); } catch (IDBExcept& ex) { @@ -105,9 +102,8 @@ static int is_columnstore_columns_fill(THD* thd, TABLE_LIST* tables, COND* cond) for (size_t col_num = 0; col_num < column_rid_list.size(); col_num++) { - execplan::CalpontSystemCatalog::TableColName tcn = - systemCatalogPtr->colName(column_rid_list[col_num].objnum); - execplan::CalpontSystemCatalog::ColType ct = systemCatalogPtr->colType(column_rid_list[col_num].objnum); + execplan::CalpontSystemCatalog::TableColName tcn = csc.colName(column_rid_list[col_num].objnum); + execplan::CalpontSystemCatalog::ColType ct = csc.colType(column_rid_list[col_num].objnum); table->field[0]->store(tcn.schema.c_str(), tcn.schema.length(), cs); table->field[1]->store(tcn.table.c_str(), tcn.table.length(), cs); diff --git a/dbcon/mysql/is_columnstore_tables.cpp b/dbcon/mysql/is_columnstore_tables.cpp index 63b8517cb..0928ac625 100644 --- a/dbcon/mysql/is_columnstore_tables.cpp +++ b/dbcon/mysql/is_columnstore_tables.cpp @@ -47,11 +47,8 @@ static int is_columnstore_tables_fill(THD* thd, TABLE_LIST* tables, COND* cond) TABLE* table = tables->table; InformationSchemaCond isCond; - boost::shared_ptr systemCatalogPtr = - execplan::CalpontSystemCatalog::makeCalpontSystemCatalog( - execplan::CalpontSystemCatalog::idb_tid2sid(thd->thread_id)); - - systemCatalogPtr->identity(execplan::CalpontSystemCatalog::FE); + execplan::CalpontSystemCatalog csc; + csc.identity(execplan::CalpontSystemCatalog::FE); if (cond) { @@ -60,7 +57,7 @@ static int is_columnstore_tables_fill(THD* thd, TABLE_LIST* tables, COND* cond) const std::vector< std::pair > - catalog_tables = systemCatalogPtr->getTables(); + catalog_tables = csc.getTables(); for (std::vector >::const_iterator it = @@ -72,7 +69,7 @@ static int is_columnstore_tables_fill(THD* thd, TABLE_LIST* tables, COND* cond) try { - execplan::CalpontSystemCatalog::TableInfo tb_info = systemCatalogPtr->tableInfo((*it).second); + execplan::CalpontSystemCatalog::TableInfo tb_info = csc.tableInfo((*it).second); std::string create_date = dataconvert::DataConvert::dateToString((*it).second.create_date); table->field[0]->store((*it).second.schema.c_str(), (*it).second.schema.length(), cs); table->field[1]->store((*it).second.table.c_str(), (*it).second.table.length(), cs); @@ -83,7 +80,7 @@ static int is_columnstore_tables_fill(THD* thd, TABLE_LIST* tables, COND* cond) if (tb_info.tablewithautoincr) { table->field[5]->set_notnull(); - table->field[5]->store(systemCatalogPtr->nextAutoIncrValue((*it).second)); + table->field[5]->store(csc.nextAutoIncrValue((*it).second)); } else {