From 086a98794ee4f97c9e9479ba247542c6715d2521 Mon Sep 17 00:00:00 2001 From: David Hall Date: Tue, 1 Nov 2016 16:07:13 -0500 Subject: [PATCH] MCOL-361 String::c_ptr() can cause a realloc and break things. Remove all uses of c_ptr to String objects from the server. --- dbcon/mysql/ha_calpont_execplan.cpp | 82 +++++++++++++++++++---------- dbcon/mysql/ha_calpont_impl.cpp | 29 +++++----- 2 files changed, 67 insertions(+), 44 deletions(-) diff --git a/dbcon/mysql/ha_calpont_execplan.cpp b/dbcon/mysql/ha_calpont_execplan.cpp index 56a9c6f6d..8a911acad 100755 --- a/dbcon/mysql/ha_calpont_execplan.cpp +++ b/dbcon/mysql/ha_calpont_execplan.cpp @@ -21,7 +21,7 @@ */ /** @file */ -#define DEBUG_WALK_COND +//#define DEBUG_WALK_COND #include #include #include @@ -222,7 +222,9 @@ void debug_walk(const Item *item, void *arg) { Item_string* isp = (Item_string*)item; String val, *str = isp->val_str(&val); - cout << "STRING_ITEM: >" << str->c_ptr() << '<' << endl; + string valStr; + valStr.assign(str->ptr(), str->length()); + cout << "STRING_ITEM: >" << valStr << '<' << endl; break; } case Item::REAL_ITEM: @@ -2293,9 +2295,11 @@ ReturnedColumn* buildReturnedColumn(Item* item, gp_walk_info& gwi, bool& nonSupp case Item::VARBIN_ITEM: { String val, *str = item->val_str(&val); + string valStr; + valStr.assign(str->ptr(), str->length()); if (item->unsigned_flag) { - //cc = new ConstantColumn(str->c_ptr(), (uint64_t)item->val_uint(), ConstantColumn::NUM); + //cc = new ConstantColumn(valStr, (uint64_t)item->val_uint(), ConstantColumn::NUM); // It seems that str at this point is crap if val_uint() is > MAX_BIGINT. By using // this constructor, ConstantColumn is built with the proper string. For whatever reason, // ExeMgr converts the fConstval member to numeric, rather than using the existing numeric @@ -2304,7 +2308,7 @@ ReturnedColumn* buildReturnedColumn(Item* item, gp_walk_info& gwi, bool& nonSupp } else { - rc = new ConstantColumn(str->c_ptr(), (int64_t)item->val_int(), ConstantColumn::NUM); + rc = new ConstantColumn(valStr, (int64_t)item->val_int(), ConstantColumn::NUM); } //return cc; break; @@ -2312,13 +2316,17 @@ ReturnedColumn* buildReturnedColumn(Item* item, gp_walk_info& gwi, bool& nonSupp case Item::STRING_ITEM: { String val, *str = item->val_str(&val); - rc = new ConstantColumn(str->c_ptr()); + string valStr; + valStr.assign(str->ptr(), str->length()); + rc = new ConstantColumn(valStr); break; } case Item::REAL_ITEM: { String val, *str = item->val_str(&val); - rc = new ConstantColumn(str->c_ptr(), item->val_real()); + string valStr; + valStr.assign(str->ptr(), str->length()); + rc = new ConstantColumn(valStr, item->val_real()); break; } case Item::DECIMAL_ITEM: @@ -2351,20 +2359,25 @@ ReturnedColumn* buildReturnedColumn(Item* item, gp_walk_info& gwi, bool& nonSupp (tmpVec.size() == 0)) { String val, *str = ifp->val_str(&val); + string valStr; + if (str) + { + valStr.assign(str->ptr(), str->length()); + } if (!str) { rc = new ConstantColumn("", ConstantColumn::NULLDATA); } else if (ifp->result_type() == STRING_RESULT) { - rc = new ConstantColumn(str->c_ptr(), ConstantColumn::LITERAL); + rc = new ConstantColumn(valStr, ConstantColumn::LITERAL); rc->resultType(colType_MysqlToIDB(item)); } else if (ifp->result_type() == DECIMAL_RESULT) rc = buildDecimalColumn(ifp, gwi); else { - rc = new ConstantColumn(str->c_ptr(), ConstantColumn::NUM); + rc = new ConstantColumn(valStr, ConstantColumn::NUM); rc->resultType(colType_MysqlToIDB(item)); } break; @@ -2431,7 +2444,9 @@ ReturnedColumn* buildReturnedColumn(Item* item, gp_walk_info& gwi, bool& nonSupp case Item::DATE_ITEM: { String val, *str = item->val_str(&val); - rc = new ConstantColumn(str->c_ptr()); + string valStr; + valStr.assign(str->ptr(), str->length()); + rc = new ConstantColumn(valStr); break; } case Item::WINDOW_FUNC_ITEM: @@ -3206,18 +3221,20 @@ ConstantColumn* buildDecimalColumn(Item *item, gp_walk_info &gwi) Item_decimal* idp = (Item_decimal*)item; IDB_Decimal infinidb_decimal; String val, *str = item->val_str(&val); + string valStr; + valStr.assign(str->ptr(), str->length()); ostringstream infinidb_decimal_val; uint32_t i = 0; - if (str->c_ptr()[0] == '+' || str->c_ptr()[0] == '-') + if (str->ptr()[0] == '+' || str->ptr()[0] == '-') { - infinidb_decimal_val << str->c_ptr()[0]; + infinidb_decimal_val << str->ptr()[0]; i = 1; } for (; i < str->length(); i++) { - if (str->c_ptr()[i] == '.') + if (str->ptr()[i] == '.') continue; - infinidb_decimal_val << str->c_ptr()[i]; + infinidb_decimal_val << str->ptr()[i]; } infinidb_decimal.value = strtoll(infinidb_decimal_val.str().c_str(), 0, 10); @@ -3230,7 +3247,7 @@ ConstantColumn* buildDecimalColumn(Item *item, gp_walk_info &gwi) else infinidb_decimal.scale = idp->decimals; infinidb_decimal.precision = idp->max_length - idp->decimals; - return new ConstantColumn(str->c_ptr(), infinidb_decimal); + return new ConstantColumn(valStr, infinidb_decimal); } SimpleColumn* buildSimpleColumn(Item_field* ifp, gp_walk_info& gwi) @@ -3490,7 +3507,11 @@ ReturnedColumn* buildAggregateColumn(Item* item, gp_walk_info& gwi) (dynamic_cast(ac))->orderCols(orderCols); parm.reset(rowCol); if (gc->str_separator()) - (dynamic_cast(ac))->separator(gc->str_separator()->c_ptr()); + { + string separator; + separator.assign(gc->str_separator()->ptr(), gc->str_separator()->length()); + (dynamic_cast(ac))->separator(separator); + } } else { @@ -3927,7 +3948,7 @@ void gp_walk(const Item *item, void *arg) string cval; if (str->ptr()) { - cval = str->c_ptr(); + cval.assign(str->ptr(), str->length()); } size_t spos = cval.find_last_not_of(" "); if (spos != string::npos) @@ -4040,7 +4061,8 @@ void gp_walk(const Item *item, void *arg) ifp->functype() != Item_func::MULT_EQUAL_FUNC) { String val, *str = ifp->val_str(&val); - + string valStr; + valStr.assign(str->ptr(), str->length()); ConstantColumn *cc = NULL; if (!str) //@ bug 2844 check whether parameter is defined { @@ -4048,7 +4070,7 @@ void gp_walk(const Item *item, void *arg) } else if (ifp->result_type() == STRING_RESULT) { - cc = new ConstantColumn(str->c_ptr(), ConstantColumn::LITERAL); + cc = new ConstantColumn(valStr, ConstantColumn::LITERAL); } else if (ifp->result_type() == DECIMAL_RESULT) { @@ -4056,7 +4078,7 @@ void gp_walk(const Item *item, void *arg) } else { - cc = new ConstantColumn(str->c_ptr(), ConstantColumn::NUM); + cc = new ConstantColumn(valStr, ConstantColumn::NUM); cc->resultType(colType_MysqlToIDB(item)); } @@ -4077,7 +4099,7 @@ void gp_walk(const Item *item, void *arg) else gwip->rcWorkStack.push(cc); if (str) - IDEBUG( cout << "Const F&E " << item->full_name() << " evaluate: " << str->c_ptr() << endl ); + IDEBUG( cout << "Const F&E " << item->full_name() << " evaluate: " << valStr << endl ); break; } @@ -4707,11 +4729,8 @@ int getSelectPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, bool i // @todo process from subquery if (table_ptr->derived) { - // cout << "DERIVED TABLE DEBUG" << endl; String str; (table_ptr->derived->first_select())->print(gwi.thd, &str, QT_INFINIDB_DERIVED); - // cout << str.c_ptr() << endl; - // cout << "DERIVED TABLE DEBUG END" << endl; SELECT_LEX *select_cursor = table_ptr->derived->first_select(); FromSubQuery fromSub(gwi, select_cursor); @@ -5228,6 +5247,8 @@ int getSelectPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, bool i string(ifp->func_name()) != "set_user_var") { String val, *str = ifp->val_str(&val); + string valStr; + valStr.assign(str->ptr(), str->length()); ConstantColumn *cc = NULL; if (!str) { @@ -5235,7 +5256,7 @@ int getSelectPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, bool i } else if (ifp->result_type() == STRING_RESULT) { - cc = new ConstantColumn(str->c_ptr(), ConstantColumn::LITERAL); + cc = new ConstantColumn(valStr, ConstantColumn::LITERAL); } else if (ifp->result_type() == DECIMAL_RESULT) { @@ -5243,7 +5264,7 @@ int getSelectPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, bool i } else { - cc = new ConstantColumn(str->c_ptr(), ConstantColumn::NUM); + cc = new ConstantColumn(valStr, ConstantColumn::NUM); cc->resultType(colType_MysqlToIDB(item)); } SRCP srcp(cc); @@ -5342,9 +5363,10 @@ int getSelectPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, bool i srcp->alias(item->name); Item_string* isp = reinterpret_cast(item); - String val; - String* str = isp->val_str(&val); - string name = "'" + string(str->c_ptr()) + "'" + " " + "`" + escapeBackTick(srcp->alias().c_str()) + "`"; + String val, *str = isp->val_str(&val); + string valStr; + valStr.assign(str->ptr(), str->length()); + string name = "'" + valStr + "'" + " " + "`" + escapeBackTick(srcp->alias().c_str()) + "`"; if (sel_cols_in_create.length() != 0) sel_cols_in_create += ", "; @@ -5366,8 +5388,10 @@ int getSelectPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, bool i Item_decimal* isp = reinterpret_cast(item); String val, *str = isp->val_str(&val); + string valStr; + valStr.assign(str->ptr(), str->length()); ostringstream oss; - oss << str->c_ptr() << " `" << escapeBackTick(srcp->alias().c_str()) << "`"; + oss << valStr.c_str() << " `" << escapeBackTick(srcp->alias().c_str()) << "`"; if (sel_cols_in_create.length() != 0) sel_cols_in_create += ", "; sel_cols_in_create += oss.str(); diff --git a/dbcon/mysql/ha_calpont_impl.cpp b/dbcon/mysql/ha_calpont_impl.cpp index d87fc135e..a4fe591fc 100755 --- a/dbcon/mysql/ha_calpont_impl.cpp +++ b/dbcon/mysql/ha_calpont_impl.cpp @@ -926,16 +926,14 @@ uint32_t doUpdateDelete(THD *thd) //@Bug 2587 use val_str to replace value->name to get rid of 255 limit String val, *str; str = value->val_str(&val); -// dmlStmt += "'" + string(str->c_ptr()) + "'"; - columnAssignmentPtr->fScalarExpression = string(str->c_ptr()) ; + columnAssignmentPtr->fScalarExpression.assign(str->ptr(), str->length()); columnAssignmentPtr->fFromCol = false; } else if ( value->type() == Item::VARBIN_ITEM ) { String val, *str; str = value->val_str(&val); -// dmlStmt += "'" + string(str->c_ptr()) + "'"; - columnAssignmentPtr->fScalarExpression = string(str->c_ptr()) ; + columnAssignmentPtr->fScalarExpression.assign(str->ptr(), str->length()); columnAssignmentPtr->fFromCol = false; } else if ( value->type() == Item::FUNC_ITEM ) @@ -1056,7 +1054,6 @@ uint32_t doUpdateDelete(THD *thd) if (!tmp->field_name) //null { -// dmlStmt += "NULL"; columnAssignmentPtr->fScalarExpression = "NULL"; columnAssignmentPtr->fFromCol = false; } @@ -1064,8 +1061,7 @@ uint32_t doUpdateDelete(THD *thd) { String val, *str; str = value->val_str(&val); -// dmlStmt += string(str->c_ptr()); - columnAssignmentPtr->fScalarExpression = string(str->c_ptr()); + columnAssignmentPtr->fScalarExpression.assign(str->ptr(), str->length()); columnAssignmentPtr->fFromCol = false; } } @@ -1082,13 +1078,11 @@ uint32_t doUpdateDelete(THD *thd) str = value->val_str(&val); if (str) { -// dmlStmt += string(str->c_ptr()); - columnAssignmentPtr->fScalarExpression = string(str->c_ptr()); + columnAssignmentPtr->fScalarExpression.assign(str->ptr(), str->length()); columnAssignmentPtr->fFromCol = false; } else { -// dmlStmt += "NULL"; columnAssignmentPtr->fScalarExpression = "NULL"; columnAssignmentPtr->fFromCol = false; } @@ -2302,7 +2296,6 @@ const char* calgetsqlcount(UDF_INIT* initid, UDF_ARGS* args, char* result, unsigned long* length, char* is_null, char* error) { - sm::cpsm_conhdl_t* hndl; THD* thd = current_thd; if (!thd->infinidb_vtable.cal_conn_info) thd->infinidb_vtable.cal_conn_info = (void*)(new cal_connection_info()); @@ -2683,8 +2676,10 @@ int ha_calpont_impl_rnd_init(TABLE* table) // @bug 2547. don't need to send the plan if it's impossible where for all unions. if (thd->infinidb_vtable.impossibleWhereOnUnion) return 0; - - csep->data(thd->infinidb_vtable.original_query.c_ptr()); + string query; + query.assign(thd->infinidb_vtable.original_query.ptr(), + thd->infinidb_vtable.original_query.length()); + csep->data(query); try { csep->priority( ci->stats.userPriority(ci->stats.fHost, ci->stats.fUser)); }catch (std::exception& e) @@ -4253,7 +4248,9 @@ COND* ha_calpont_impl_cond_push(COND *cond, TABLE* table) ((thd->lex)->sql_command == SQLCOM_DELETE) || ((thd->lex)->sql_command == SQLCOM_DELETE_MULTI)) return cond; - IDEBUG( cout << "ha_calpont_impl_cond_push: " << table->alias.c_ptr() << endl ); + string alias; + alias.assign(table->alias.ptr(), table->alias.length()); + IDEBUG( cout << "ha_calpont_impl_cond_push: " << alias << endl ); if (!thd->infinidb_vtable.cal_conn_info) thd->infinidb_vtable.cal_conn_info = (void*)(new cal_connection_info()); @@ -4320,7 +4317,9 @@ int ha_calpont_impl_external_lock(THD *thd, TABLE* table, int lock_type) // @info called for every table at the beginning and at the end of a query. // used for cleaning up the tableinfo. - IDEBUG( cout << "external_lock for " << table->alias.c_ptr() << endl ); + string alias; + alias.assign(table->alias.ptr(), table->alias.length()); + IDEBUG( cout << "external_lock for " << alias << endl ); idbassert((thd->infinidb_vtable.vtable_state >= THD::INFINIDB_INIT_CONNECT && thd->infinidb_vtable.vtable_state <= THD::INFINIDB_REDO_QUERY) || thd->infinidb_vtable.vtable_state == THD::INFINIDB_ERROR);