From d269ccd448ebf7c06ac61bc26df72ebcfa44070a Mon Sep 17 00:00:00 2001 From: wangt1xiuyi <13547954130@163.com> Date: Thu, 21 Sep 2023 02:40:16 +0000 Subject: [PATCH] fix ObOptColumnStat serialize bug --- src/share/stat/ob_dbms_stats_executor.cpp | 5 ++--- .../stat/ob_dbms_stats_export_import.cpp | 3 +-- src/share/stat/ob_dbms_stats_utils.cpp | 7 ++---- .../stat/ob_incremental_stat_estimator.cpp | 6 ++--- src/share/stat/ob_opt_column_stat.cpp | 22 +++++++++++++++++-- src/share/stat/ob_opt_column_stat.h | 2 ++ src/share/stat/ob_opt_osg_column_stat.cpp | 2 +- src/share/stat/ob_stats_estimator.cpp | 4 ++++ .../components/ob_dh_opt_stats_gather.cpp | 7 +++--- 9 files changed, 38 insertions(+), 20 deletions(-) diff --git a/src/share/stat/ob_dbms_stats_executor.cpp b/src/share/stat/ob_dbms_stats_executor.cpp index da14de6bf..beb3caed8 100644 --- a/src/share/stat/ob_dbms_stats_executor.cpp +++ b/src/share/stat/ob_dbms_stats_executor.cpp @@ -319,7 +319,7 @@ int ObDbmsStatsExecutor::set_column_stats(ObExecContext &ctx, } else if (OB_ISNULL(col_stat_handle.stat_)) { ret = OB_ERR_UNEXPECTED; LOG_WARN("get unexpected null", K(col_stat_handle.stat_), K(ret)); - } else if (OB_ISNULL(col_stat = OB_NEWx(ObOptColumnStat, alloc, (*alloc)))) { + } else if (OB_ISNULL(col_stat = ObOptColumnStat::malloc_new_column_stat(*alloc))) { ret = OB_ALLOCATE_MEMORY_FAILED; LOG_WARN("failed to create column stat", K(ret)); } else if (OB_FAIL(col_stat->deep_copy(*col_stat_handle.stat_))) { @@ -656,11 +656,10 @@ int ObDbmsStatsExecutor::init_opt_stat(ObIAllocator &allocator, } for (int64_t i = 0; OB_SUCC(ret) && i < param.column_params_.count(); ++i) { ObOptColumnStat *&col_stat = stat.column_stats_.at(i); - if (OB_ISNULL(ptr = allocator.alloc(sizeof(ObOptColumnStat)))) { + if (OB_ISNULL(col_stat = ObOptColumnStat::malloc_new_column_stat(allocator))) { ret = OB_ALLOCATE_MEMORY_FAILED; LOG_WARN("memory is not enough", K(ret), K(ptr)); } else { - col_stat = new (ptr) ObOptColumnStat(allocator); col_stat->set_table_id(param.column_params_.at(i).need_basic_stat() ? param.table_id_: -1); col_stat->set_partition_id(part_info.part_id_); col_stat->set_stat_level(extra.type_); diff --git a/src/share/stat/ob_dbms_stats_export_import.cpp b/src/share/stat/ob_dbms_stats_export_import.cpp index 8414d9ca5..2e5bc0a56 100644 --- a/src/share/stat/ob_dbms_stats_export_import.cpp +++ b/src/share/stat/ob_dbms_stats_export_import.cpp @@ -1226,11 +1226,10 @@ int ObDbmsStatsExportImport::init_opt_stat(ObExecContext &ctx, LOG_WARN("failed to get opt col stat", K(ret)); } else if (col_stat != NULL) {//find already exists opt column stat /*do nothing*/ - } else if (OB_ISNULL(ptr = param.allocator_->alloc(sizeof(ObOptColumnStat)))) { + } else if (OB_ISNULL(col_stat = ObOptColumnStat::malloc_new_column_stat(*param.allocator_))) { ret = OB_ALLOCATE_MEMORY_FAILED; LOG_WARN("memory is not enough", K(ret), K(ptr)); } else { - col_stat = new (ptr) ObOptColumnStat(*param.allocator_); col_stat->set_table_id(param.table_id_); col_stat->set_partition_id(part_id); col_stat->set_stat_level(type); diff --git a/src/share/stat/ob_dbms_stats_utils.cpp b/src/share/stat/ob_dbms_stats_utils.cpp index 5bcb8fb3b..59f976c95 100644 --- a/src/share/stat/ob_dbms_stats_utils.cpp +++ b/src/share/stat/ob_dbms_stats_utils.cpp @@ -61,7 +61,6 @@ int ObDbmsStatsUtils::init_col_stats(ObIAllocator &allocator, ObIArray &col_stats) { int ret = OB_SUCCESS; - void *ptr = NULL; if (OB_UNLIKELY(col_cnt <= 0)) { ret = OB_ERR_UNEXPECTED; LOG_WARN("get unexpected error, expected specify column cnt is great 0", K(ret), K(col_cnt)); @@ -70,11 +69,9 @@ int ObDbmsStatsUtils::init_col_stats(ObIAllocator &allocator, } else { for (int64_t i = 0; OB_SUCC(ret) && i < col_cnt; ++i) { ObOptColumnStat *&col_stat = col_stats.at(i); - if (OB_ISNULL(ptr = allocator.alloc(sizeof(ObOptColumnStat)))) { + if (OB_ISNULL(col_stat = ObOptColumnStat::malloc_new_column_stat(allocator))) { ret = OB_ALLOCATE_MEMORY_FAILED; - LOG_WARN("memory is not enough", K(ret), K(ptr)); - } else { - col_stat = new (ptr) ObOptColumnStat(allocator); + LOG_WARN("memory is not enough", K(ret), K(col_stat)); } } } diff --git a/src/share/stat/ob_incremental_stat_estimator.cpp b/src/share/stat/ob_incremental_stat_estimator.cpp index 4d40a9668..d009b1284 100644 --- a/src/share/stat/ob_incremental_stat_estimator.cpp +++ b/src/share/stat/ob_incremental_stat_estimator.cpp @@ -548,12 +548,10 @@ int ObIncrementalStatEstimator::derive_global_col_stat(ObExecContext &ctx, } else { for (int64_t i = 0; OB_SUCC(ret) && i < column_cnt; ++i) { ObOptColumnStat *col_stat = NULL; - void *ptr = NULL; - if (OB_ISNULL(ptr = alloc.alloc(sizeof(ObOptColumnStat)))) { + if (OB_ISNULL(col_stat = ObOptColumnStat::malloc_new_column_stat(alloc))) { ret = OB_ALLOCATE_MEMORY_FAILED; - LOG_WARN("memory is not enough", K(ret), K(ptr)); + LOG_WARN("memory is not enough", K(ret), K(col_stat)); } else { - col_stat = new (ptr) ObOptColumnStat(alloc); ObGlobalMinEval min_eval; ObGlobalMaxEval max_eval; ObGlobalNullEval null_eval; diff --git a/src/share/stat/ob_opt_column_stat.cpp b/src/share/stat/ob_opt_column_stat.cpp index fb243ac8c..5abc25c07 100644 --- a/src/share/stat/ob_opt_column_stat.cpp +++ b/src/share/stat/ob_opt_column_stat.cpp @@ -438,6 +438,19 @@ int ObOptColumnStat::merge_min_max(ObObj &cur, const ObObj &other, bool is_cmp_m return ret; } +ObOptColumnStat *ObOptColumnStat::malloc_new_column_stat(common::ObIAllocator &allocator) +{ + ObOptColumnStat *new_col_stat = OB_NEWx(ObOptColumnStat, (&allocator), allocator); + if (new_col_stat != NULL) { + if (OB_ISNULL(new_col_stat->get_llc_bitmap())) { + new_col_stat->~ObOptColumnStat(); + allocator.free(new_col_stat); + new_col_stat = NULL; + } + } + return new_col_stat; +} + OB_DEF_SERIALIZE(ObOptColumnStat) { int ret = OB_SUCCESS; LST_DO_CODE(OB_UNIS_ENCODE, @@ -495,8 +508,13 @@ OB_DEF_DESERIALIZE(ObOptColumnStat) { avg_length_, object_type_); if (llc_bitmap_size_ !=0 && data_len - pos >= llc_bitmap_size_) { - memcpy(llc_bitmap_, buf + pos, llc_bitmap_size_); - pos += llc_bitmap_size_; + if (OB_ISNULL(llc_bitmap_)) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("get unexpected null", K(ret), K(llc_bitmap_), K(llc_bitmap_size_), K(data_len), K(pos)); + } else { + memcpy(llc_bitmap_, buf + pos, llc_bitmap_size_); + pos += llc_bitmap_size_; + } } OB_UNIS_DECODE(total_col_len_); return ret; diff --git a/src/share/stat/ob_opt_column_stat.h b/src/share/stat/ob_opt_column_stat.h index 64850b491..cec5a5516 100644 --- a/src/share/stat/ob_opt_column_stat.h +++ b/src/share/stat/ob_opt_column_stat.h @@ -317,6 +317,8 @@ public: common::ObCollationType get_collation_type() const { return cs_type_; } void set_collation_type(common::ObCollationType cs_type) { cs_type_ = cs_type; } + static ObOptColumnStat *malloc_new_column_stat(common::ObIAllocator &allocator); + TO_STRING_KV(K_(table_id), K_(partition_id), K_(column_id), diff --git a/src/share/stat/ob_opt_osg_column_stat.cpp b/src/share/stat/ob_opt_osg_column_stat.cpp index 03b12f885..d6d7e90c7 100644 --- a/src/share/stat/ob_opt_osg_column_stat.cpp +++ b/src/share/stat/ob_opt_osg_column_stat.cpp @@ -67,7 +67,7 @@ void ObOptOSGColumnStat::reset() ObOptOSGColumnStat* ObOptOSGColumnStat::create_new_osg_col_stat(common::ObIAllocator &allocator) { ObOptOSGColumnStat *new_osg_col_stat = OB_NEWx(ObOptOSGColumnStat, (&allocator), allocator); - ObOptColumnStat *new_col_stat = OB_NEWx(ObOptColumnStat, (&allocator), allocator); + ObOptColumnStat *new_col_stat = ObOptColumnStat::malloc_new_column_stat(allocator); if (OB_NOT_NULL(new_osg_col_stat) && OB_NOT_NULL(new_col_stat)) { new_osg_col_stat->col_stat_ = new_col_stat; } else { diff --git a/src/share/stat/ob_stats_estimator.cpp b/src/share/stat/ob_stats_estimator.cpp index 7927564ec..f81f2ad93 100644 --- a/src/share/stat/ob_stats_estimator.cpp +++ b/src/share/stat/ob_stats_estimator.cpp @@ -566,6 +566,10 @@ int ObStatsEstimator::copy_hybrid_hist_stat(ObOptStat &src_opt_stat, !src_col_stat->get_histogram().is_valid()) { LOG_TRACE("no need copy histogram", K(src_col_stat->get_histogram()), K(dst_col_stat->get_histogram()), K(i), K(j)); + if (!src_col_stat->get_histogram().is_valid() && + dst_col_stat->get_histogram().is_hybrid()) { + dst_col_stat->get_histogram().reset(); + } } else { ObHistogram &src_hist = src_col_stat->get_histogram(); dst_col_stat->get_histogram().set_type(src_hist.get_type()); diff --git a/src/sql/engine/px/datahub/components/ob_dh_opt_stats_gather.cpp b/src/sql/engine/px/datahub/components/ob_dh_opt_stats_gather.cpp index 31d968ff2..f334f5d91 100644 --- a/src/sql/engine/px/datahub/components/ob_dh_opt_stats_gather.cpp +++ b/src/sql/engine/px/datahub/components/ob_dh_opt_stats_gather.cpp @@ -45,7 +45,7 @@ OB_DEF_DESERIALIZE(ObOptStatsGatherPieceMsg) ObOptTableStat *tmp_stat = OB_NEWx(ObOptTableStat, (&arena_)); if (OB_ISNULL(tmp_stat)) { ret = OB_ALLOCATE_MEMORY_FAILED; - LOG_WARN("failed to allocate memory"); + LOG_WARN("failed to allocate memory", K(ret)); } else if (OB_FAIL(tmp_stat->deserialize(buf, data_len, pos))) { LOG_WARN("deserialize datum store failed", K(ret), K(i)); } else if (OB_FAIL(table_stats_.push_back(tmp_stat))) { @@ -57,9 +57,10 @@ OB_DEF_DESERIALIZE(ObOptStatsGatherPieceMsg) for (int64_t i = 0; OB_SUCC(ret) && i < size; ++i) { int col_stat_size = 0; OB_UNIS_DECODE(col_stat_size); - ObOptColumnStat *tmp_col_stat = OB_NEWx(ObOptColumnStat, (&arena_), arena_); + ObOptColumnStat *tmp_col_stat = ObOptColumnStat::malloc_new_column_stat(arena_); if (OB_ISNULL(tmp_col_stat)) { - LOG_WARN("failed to create new col stat"); + ret = OB_ALLOCATE_MEMORY_FAILED; + LOG_WARN("failed to create new col stat", K(ret)); } else if (OB_FAIL(tmp_col_stat->deserialize(buf, data_len, pos))) { LOG_WARN("deserialize datum store failed", K(ret), K(i)); } else if (OB_FAIL(column_stats_.push_back(tmp_col_stat))) {