From d40c5607cb62c5484296ba5a479b7f639dc6de54 Mon Sep 17 00:00:00 2001 From: Vinoth Veeraraghavan Date: Fri, 11 Dec 2020 21:30:55 +0800 Subject: [PATCH] MOT code review and memcheck fixes --- .../mot/core/src/infra/config/config_tree.h | 9 --- .../system/checkpoint/checkpoint_ctrlfile.cpp | 59 +++++++++++++------ .../system/checkpoint/checkpoint_ctrlfile.h | 14 ++--- .../system/checkpoint/checkpoint_manager.cpp | 8 +-- .../system/checkpoint/checkpoint_utils.cpp | 24 ++++++-- .../src/system/checkpoint/checkpoint_utils.h | 11 +++- .../system/recovery/checkpoint_recovery.cpp | 3 +- .../src/system/recovery/recovery_manager.cpp | 6 +- .../src/system/recovery/recovery_manager.h | 4 +- .../system/statistics/process_statistics.cpp | 2 +- .../src/system/transaction/txn_access.cpp | 1 - .../core/src/system/transaction/txn_access.h | 1 - .../src/system/transaction/txn_ddl_access.cpp | 4 +- .../storage/mot/fdw_adapter/src/mot_fdw.cpp | 8 +-- .../mot/fdw_adapter/src/mot_internal.cpp | 24 +++----- .../mot/jit_exec/src/jit_llvm_blocks.cpp | 13 ++-- .../storage/mot/jit_exec/src/jit_tvm.cpp | 4 +- .../mot/jit_exec/src/jit_tvm_blocks.cpp | 13 ++-- .../storage/mot/jit_exec/src/jit_tvm_funcs.h | 8 ++- 19 files changed, 124 insertions(+), 92 deletions(-) diff --git a/src/gausskernel/storage/mot/core/src/infra/config/config_tree.h b/src/gausskernel/storage/mot/core/src/infra/config/config_tree.h index 8ff3f5b25..67fa59e4a 100644 --- a/src/gausskernel/storage/mot/core/src/infra/config/config_tree.h +++ b/src/gausskernel/storage/mot/core/src/infra/config/config_tree.h @@ -153,15 +153,6 @@ public: return m_rootSection; } - /** - * @brief Retrieves the virtual root section for modification. - * @return The root section. - */ - inline ConfigSection* ModifyRootSection() - { - return &m_rootSection; - } - /** * @brief Prints the configuration tree to the log. * @param logLevel The log level used for printing. diff --git a/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_ctrlfile.cpp b/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_ctrlfile.cpp index 0879a92a1..ae45d900d 100644 --- a/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_ctrlfile.cpp +++ b/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_ctrlfile.cpp @@ -38,48 +38,72 @@ static spin_lock g_ctrlfileLock; CheckpointControlFile* CheckpointControlFile::GetCtrlFile() { - if (initialized) + if (initialized) { return ctrlfileInst; + } g_ctrlfileLock.lock(); ctrlfileInst = new (std::nothrow) CheckpointControlFile(); if (ctrlfileInst == nullptr) { MOT_REPORT_ERROR(MOT_ERROR_OOM, "Checkpoint", "Failed to allocate memory for checkpoint control file object"); + } else { + if (!ctrlfileInst->Init()) { + MOT_REPORT_ERROR( + MOT_ERROR_INVALID_CFG, "Checkpoint", "Failed to initialize checkpoint control file object"); + delete ctrlfileInst; + ctrlfileInst = nullptr; + } } g_ctrlfileLock.unlock(); return ctrlfileInst; } -void CheckpointControlFile::Init() +bool CheckpointControlFile::Init() { - int fd = -1; - if (initialized) - return; + if (initialized) { + return true; + } do { + if (GetGlobalConfiguration().m_checkpointDir.length() >= CheckpointUtils::maxPath) { + MOT_REPORT_ERROR(MOT_ERROR_INVALID_CFG, + "Checkpoint", + "Invalid checkpoint_dir configuration, length exceeds max path length"); + break; + } + if (!CheckpointUtils::GetWorkingDir(m_fullPath)) { MOT_LOG_ERROR("Could not obtain working directory"); break; } - m_fullPath.append("/"); + + if (!CheckpointUtils::IsDirExists(m_fullPath)) { + MOT_REPORT_ERROR( + MOT_ERROR_INVALID_CFG, "Checkpoint", "Invalid checkpoint_dir configuration, directory doesn't exist"); + break; + } + + // "/" is already appended in CheckpointUtils::GetWorkingDir. m_fullPath.append(CTRL_FILE_NAME); MOT_LOG_TRACE("CheckpointControlFile: Fullpath - '%s'", m_fullPath.c_str()); + // try to open an old file - if (!CheckpointUtils::FileExists(m_fullPath)) { + if (!CheckpointUtils::IsFileExists(m_fullPath)) { MOT_LOG_INFO("CheckpointControlFile: init - a previous checkpoint was not found"); - m_controlFile.Init(); + m_ctrlFileData.Init(); } else { + int fd = -1; if (!CheckpointUtils::OpenFileRead(m_fullPath, fd)) { MOT_LOG_ERROR("CheckpointControlFile: init - could not open control file"); break; } - if (CheckpointUtils::ReadFile(fd, (char*)&m_controlFile, sizeof(CtrlFileData)) != sizeof(CtrlFileData)) { + if (CheckpointUtils::ReadFile(fd, (char*)&m_ctrlFileData, sizeof(CtrlFileData)) != sizeof(CtrlFileData)) { MOT_LOG_ERROR("CheckpointControlFile: init - failed to read data from file"); CheckpointUtils::CloseFile(fd); break; } else { - MOT_LOG_INFO( - "CheckpointControlFile: init - loaded file: checkpointId %lu", m_controlFile.entry[0].checkpointId); + MOT_LOG_INFO("CheckpointControlFile: init - loaded file: checkpointId %lu", + m_ctrlFileData.entry[0].checkpointId); } if (CheckpointUtils::CloseFile(fd)) { MOT_LOG_ERROR("CheckpointControlFile: init - failed to close file"); @@ -87,10 +111,11 @@ void CheckpointControlFile::Init() } } m_valid = true; + initialized = true; + Print(); } while (0); - initialized = true; - Print(); + return initialized; } bool CheckpointControlFile::Update(uint64_t id, uint64_t lsn, uint64_t lastReplayLsn) @@ -104,11 +129,11 @@ bool CheckpointControlFile::Update(uint64_t id, uint64_t lsn, uint64_t lastRepla break; } - m_controlFile.entry[0].checkpointId = id; - m_controlFile.entry[0].lsn = lsn; - m_controlFile.entry[0].lastReplayLsn = lastReplayLsn; + m_ctrlFileData.entry[0].checkpointId = id; + m_ctrlFileData.entry[0].lsn = lsn; + m_ctrlFileData.entry[0].lastReplayLsn = lastReplayLsn; - if (CheckpointUtils::WriteFile(fd, (char*)&m_controlFile, sizeof(CtrlFileData)) != sizeof(CtrlFileData)) { + if (CheckpointUtils::WriteFile(fd, (char*)&m_ctrlFileData, sizeof(CtrlFileData)) != sizeof(CtrlFileData)) { MOT_LOG_ERROR("CheckpointControlFile::update - failed to write control file"); CheckpointUtils::CloseFile(fd); break; diff --git a/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_ctrlfile.h b/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_ctrlfile.h index 9c52a6608..05a38f97b 100644 --- a/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_ctrlfile.h +++ b/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_ctrlfile.h @@ -35,14 +35,12 @@ namespace MOT { class CheckpointControlFile { public: CheckpointControlFile() : m_valid(false) - { - Init(); - } + {} ~CheckpointControlFile() {} - void Init(); + bool Init(); struct CtrlFileElem { CtrlFileElem(uint64_t id = invalidId, uint64_t lsn = invalidId, uint64_t replay = invalidId) @@ -65,17 +63,17 @@ public: uint64_t GetId() const { - return m_controlFile.entry[0].checkpointId; + return m_ctrlFileData.entry[0].checkpointId; } uint64_t GetLsn() const { - return m_controlFile.entry[0].lsn; + return m_ctrlFileData.entry[0].lsn; } uint64_t GetLastReplayLsn() const { - return m_controlFile.entry[0].lastReplayLsn; + return m_ctrlFileData.entry[0].lastReplayLsn; } /** @@ -118,7 +116,7 @@ private: std::string m_fullPath; - struct CtrlFileData m_controlFile; + struct CtrlFileData m_ctrlFileData; bool m_valid; }; diff --git a/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_manager.cpp b/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_manager.cpp index 592d4e71d..8d8b9e06b 100644 --- a/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_manager.cpp +++ b/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_manager.cpp @@ -620,16 +620,12 @@ void CheckpointManager::RemoveOldCheckpoints(uint64_t curCheckcpointId) void CheckpointManager::RemoveCheckpointDir(uint64_t checkpointId) { errno_t erc; + char buf[CheckpointUtils::maxPath]; std::string oldCheckpointDir; if (!CheckpointUtils::SetWorkingDir(oldCheckpointDir, checkpointId)) { MOT_LOG_ERROR("removeCheckpointDir: failed to set working directory"); return; } - char* buf = (char*)malloc(CheckpointUtils::maxPath); - if (buf == nullptr) { - MOT_LOG_ERROR("removeCheckpointDir: failed to allocate buffer"); - return; - } DIR* dir = opendir(oldCheckpointDir.c_str()); if (dir != nullptr) { @@ -663,8 +659,6 @@ void CheckpointManager::RemoveCheckpointDir(uint64_t checkpointId) errno, gs_strerror(errno)); } - - free(buf); } bool CheckpointManager::CreateCheckpointMap() diff --git a/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_utils.cpp b/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_utils.cpp index 24a089e8e..2eb810168 100644 --- a/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_utils.cpp +++ b/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_utils.cpp @@ -32,12 +32,23 @@ DECLARE_LOGGER(CheckpointUtils, Checkpoint); namespace CheckpointUtils { -bool FileExists(std::string fileName) +bool IsFileExists(std::string fileName) { - struct stat buf; - if (stat(fileName.c_str(), &buf) == -1 && errno == ENOENT) { + struct stat statBuf = {0}; + if (stat(fileName.c_str(), &statBuf) == -1 && errno == ENOENT) { return false; - } else if (!S_ISREG(buf.st_mode)) { + } else if (!S_ISREG(statBuf.st_mode)) { + return false; + } + return true; +} + +bool IsDirExists(std::string dirName) +{ + struct stat statBuf = {0}; + if (stat(dirName.c_str(), &statBuf) == -1 && errno == ENOENT) { + return false; + } else if (!S_ISDIR(statBuf.st_mode)) { return false; } return true; @@ -134,8 +145,9 @@ bool GetWorkingDir(std::string& dir) { dir.clear(); char cwd[maxPath] = {0}; - if (GetGlobalConfiguration().m_checkpointDir.length()) { - errno_t erc = strncpy_s(cwd, maxPath, GetGlobalConfiguration().m_checkpointDir.c_str(), maxPath - 1); + size_t checkpointDirLength = GetGlobalConfiguration().m_checkpointDir.length(); + if (checkpointDirLength > 0) { + errno_t erc = strncpy_s(cwd, maxPath, GetGlobalConfiguration().m_checkpointDir.c_str(), checkpointDirLength); securec_check(erc, "\0", "\0"); } else if (!getcwd(cwd, sizeof(cwd))) { MOT_REPORT_SYSTEM_ERROR(getcwd, "N/A", "Failed to get current working directory"); diff --git a/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_utils.h b/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_utils.h index 69319d204..1d6220d5a 100644 --- a/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_utils.h +++ b/src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_utils.h @@ -39,11 +39,18 @@ namespace MOT { namespace CheckpointUtils { /** - * @brief A wrapper function that checks is a file exists + * @brief A wrapper function that checks if a file exists * @param fileName The file name to check * @return Boolean value denoting success or failure. */ -bool FileExists(std::string fileName); +bool IsFileExists(std::string fileName); + +/** + * @brief A wrapper function that checks if a dir exists + * @param fileName The directory name to check + * @return Boolean value denoting success or failure. + */ +bool IsDirExists(std::string dirName); /** * @brief A wrapper function that opens a file for writing. diff --git a/src/gausskernel/storage/mot/core/src/system/recovery/checkpoint_recovery.cpp b/src/gausskernel/storage/mot/core/src/system/recovery/checkpoint_recovery.cpp index 22bc9885a..097c4903f 100644 --- a/src/gausskernel/storage/mot/core/src/system/recovery/checkpoint_recovery.cpp +++ b/src/gausskernel/storage/mot/core/src/system/recovery/checkpoint_recovery.cpp @@ -717,7 +717,6 @@ bool CheckpointRecovery::DeserializeInProcessTxns(int fd, uint64_t numEntries) bool CheckpointRecovery::IsCheckpointValid(uint64_t id) { - int fd = -1; std::string fileName; std::string workingDir; bool ret = false; @@ -728,7 +727,7 @@ bool CheckpointRecovery::IsCheckpointValid(uint64_t id) } CheckpointUtils::MakeEndFilename(fileName, workingDir, id); - if (!CheckpointUtils::FileExists(fileName)) { + if (!CheckpointUtils::IsFileExists(fileName)) { MOT_LOG_ERROR("IsCheckpointValid: checkpoint id %lu is invalid", id); break; } diff --git a/src/gausskernel/storage/mot/core/src/system/recovery/recovery_manager.cpp b/src/gausskernel/storage/mot/core/src/system/recovery/recovery_manager.cpp index 77499a89d..df0237e6c 100644 --- a/src/gausskernel/storage/mot/core/src/system/recovery/recovery_manager.cpp +++ b/src/gausskernel/storage/mot/core/src/system/recovery/recovery_manager.cpp @@ -312,7 +312,7 @@ RC RecoveryManager::RedoSegment( bool RecoveryManager::LogStats::FindIdx(uint64_t tableId, uint64_t& id) { id = m_numEntries; - std::map::iterator it; + std::map::iterator it; std::lock_guard lock(m_slock); it = m_idToIdx.find(tableId); if (it == m_idToIdx.end()) { @@ -321,7 +321,7 @@ bool RecoveryManager::LogStats::FindIdx(uint64_t tableId, uint64_t& id) return false; } m_tableStats.push_back(newEntry); - m_idToIdx.insert(std::pair(tableId, m_numEntries)); + m_idToIdx.insert(std::pair(tableId, m_numEntries)); m_numEntries++; } else { id = it->second; @@ -332,7 +332,7 @@ bool RecoveryManager::LogStats::FindIdx(uint64_t tableId, uint64_t& id) void RecoveryManager::LogStats::Print() { MOT_LOG_ERROR(">> log recovery stats >>"); - for (int i = 0; i < m_numEntries; i++) { + for (uint64_t i = 0; i < m_numEntries; i++) { MOT_LOG_ERROR("TableId %lu, Inserts: %lu, Updates: %lu, Deletes: %lu", m_tableStats[i]->m_id, m_tableStats[i]->m_inserts.load(), diff --git a/src/gausskernel/storage/mot/core/src/system/recovery/recovery_manager.h b/src/gausskernel/storage/mot/core/src/system/recovery/recovery_manager.h index d49b6a554..d2aa433cb 100644 --- a/src/gausskernel/storage/mot/core/src/system/recovery/recovery_manager.h +++ b/src/gausskernel/storage/mot/core/src/system/recovery/recovery_manager.h @@ -245,7 +245,7 @@ public: */ bool FindIdx(uint64_t tableId, uint64_t& id); - std::map m_idToIdx; + std::map m_idToIdx; std::vector m_tableStats; @@ -253,7 +253,7 @@ public: spin_lock m_slock; - int m_numEntries; + uint64_t m_numEntries; }; LogStats* m_logStats; diff --git a/src/gausskernel/storage/mot/core/src/system/statistics/process_statistics.cpp b/src/gausskernel/storage/mot/core/src/system/statistics/process_statistics.cpp index 789afb4ac..0252b189f 100644 --- a/src/gausskernel/storage/mot/core/src/system/statistics/process_statistics.cpp +++ b/src/gausskernel/storage/mot/core/src/system/statistics/process_statistics.cpp @@ -73,7 +73,7 @@ ProcessStatisticsProvider::ProcessStatisticsProvider() ++m_processorCount; } } - fclose(file); + (void)fclose(file); } MOT_LOG_DEBUG("Detected %u processors", m_processorCount); } diff --git a/src/gausskernel/storage/mot/core/src/system/transaction/txn_access.cpp b/src/gausskernel/storage/mot/core/src/system/transaction/txn_access.cpp index d63ac8fdd..c1986d2a8 100644 --- a/src/gausskernel/storage/mot/core/src/system/transaction/txn_access.cpp +++ b/src/gausskernel/storage/mot/core/src/system/transaction/txn_access.cpp @@ -151,7 +151,6 @@ bool TxnAccess::Init(TxnManager* manager) } m_initPhase = CreateRowSet; - m_initPhase = CreateInsertSet; m_insertManager = new (std::nothrow) TxnInsertAction(); if (m_insertManager == nullptr) { MOT_REPORT_ERROR(MOT_ERROR_OOM, diff --git a/src/gausskernel/storage/mot/core/src/system/transaction/txn_access.h b/src/gausskernel/storage/mot/core/src/system/transaction/txn_access.h index 4964e332c..58f0f62f1 100644 --- a/src/gausskernel/storage/mot/core/src/system/transaction/txn_access.h +++ b/src/gausskernel/storage/mot/core/src/system/transaction/txn_access.h @@ -338,7 +338,6 @@ private: InitDummyInd, CreateRowZero, CreateRowSet, - CreateInsertSet, Done } m_initPhase; diff --git a/src/gausskernel/storage/mot/core/src/system/transaction/txn_ddl_access.cpp b/src/gausskernel/storage/mot/core/src/system/transaction/txn_ddl_access.cpp index b870b3e2c..26a814580 100644 --- a/src/gausskernel/storage/mot/core/src/system/transaction/txn_ddl_access.cpp +++ b/src/gausskernel/storage/mot/core/src/system/transaction/txn_ddl_access.cpp @@ -74,8 +74,10 @@ uint32_t TxnDDLAccess::Size() void TxnDDLAccess::Reset() { - for (int i = 0; i < m_size; i++) + for (int i = 0; i < m_size; i++) { delete m_accessList[i]; + m_accessList[i] = nullptr; + } m_size = 0; } diff --git a/src/gausskernel/storage/mot/fdw_adapter/src/mot_fdw.cpp b/src/gausskernel/storage/mot/fdw_adapter/src/mot_fdw.cpp index 205250d5e..492e6d2ef 100644 --- a/src/gausskernel/storage/mot/fdw_adapter/src/mot_fdw.cpp +++ b/src/gausskernel/storage/mot/fdw_adapter/src/mot_fdw.cpp @@ -519,7 +519,7 @@ static bool IsOrderingApplicable(PathKey* pathKey, RelOptInfo* rel, MOT::Index* do { const int16_t* cols = ix->GetColumnKeyFields(); int16_t numKeyCols = ix->GetNumFields(); - ListCell* lcEm; + ListCell* lcEm = nullptr; foreach (lcEm, pathKey->pk_eclass->ec_members) { EquivalenceMember* em = (EquivalenceMember*)lfirst(lcEm); @@ -975,7 +975,7 @@ static TupleTableSlot* MOTIterateForeignScan(ForeignScanState* node) return nullptr; } - MOT::Row* currRow; + MOT::Row* currRow = nullptr; MOTFdwStateSt* festate = (MOTFdwStateSt*)node->fdw_state; TupleTableSlot* slot = node->ss.ss_ScanTupleSlot; bool found = false; @@ -1430,7 +1430,7 @@ static TupleTableSlot* MOTExecForeignUpdate( { MOTFdwStateSt* fdwState = (MOTFdwStateSt*)resultRelInfo->ri_FdwState; MOT::RC rc = MOT::RC_OK; - MOT::Row* currRow; + MOT::Row* currRow = nullptr; AttrNumber num = fdwState->m_ctidNum - 1; MOTRecConvertSt cv; @@ -1488,7 +1488,7 @@ static TupleTableSlot* MOTExecForeignDelete( { MOTFdwStateSt* fdwState = (MOTFdwStateSt*)resultRelInfo->ri_FdwState; MOT::RC rc = MOT::RC_OK; - MOT::Row* currRow; + MOT::Row* currRow = nullptr; AttrNumber num = fdwState->m_ctidNum - 1; MOTRecConvertSt cv; diff --git a/src/gausskernel/storage/mot/fdw_adapter/src/mot_internal.cpp b/src/gausskernel/storage/mot/fdw_adapter/src/mot_internal.cpp index 08950b5ee..7cc39af50 100644 --- a/src/gausskernel/storage/mot/fdw_adapter/src/mot_internal.cpp +++ b/src/gausskernel/storage/mot/fdw_adapter/src/mot_internal.cpp @@ -71,18 +71,6 @@ oid == FLOAT8OID || oid == INT1OID || oid == DATEOID || oid == TIMEOID || oid == TIMESTAMPOID || \ oid == TIMESTAMPTZOID) -#define FILL_KEY_NULL(toid, buf, len) \ - { \ - errno_t erc = memset_s(buf, len, 0x00, len); \ - securec_check(erc, "\0", "\0"); \ - } - -#define FILL_KEY_MAX(toid, buf, len) \ - { \ - errno_t erc = memset_s(buf, len, 0xff, len); \ - securec_check(erc, "\0", "\0"); \ - } - MOT::MOTEngine* MOTAdaptor::m_engine = nullptr; static XLOGLogger xlogger; @@ -838,7 +826,8 @@ void MOTAdaptor::OpenCursor(Relation rel, MOTFdwStateSt* festate) festate->m_stateKey[bIx].InitKey(keyLength); buf = festate->m_stateKey[bIx].GetKeyBuf(); - FILL_KEY_MAX(INT8OID, buf, keyLength); + errno_t erc = memset_s(buf, keyLength, 0xff, keyLength); + securec_check(erc, "\0", "\0"); festate->m_cursor[bIx] = ix->Search(&festate->m_stateKey[bIx], false, false, festate->m_currTxn->GetThdId(), found); break; @@ -853,7 +842,8 @@ void MOTAdaptor::OpenCursor(Relation rel, MOTFdwStateSt* festate) festate->m_stateKey[1].InitKey(keyLength); buf = festate->m_stateKey[1].GetKeyBuf(); - FILL_KEY_MAX(INT8OID, buf, keyLength); + errno_t erc = memset_s(buf, keyLength, 0xff, keyLength); + securec_check(erc, "\0", "\0"); festate->m_cursor[1] = ix->Search(&festate->m_stateKey[1], false, false, festate->m_currTxn->GetThdId(), found); } else { @@ -1257,8 +1247,7 @@ MOT::RC MOTAdaptor::CreateTable(CreateForeignTableStmt* table, ::TransactionId t break; } - ListCell* cell; - + ListCell* cell = nullptr; foreach (cell, table->base.tableElts) { int16 typeLen = 0; bool isBlob = false; @@ -1692,7 +1681,8 @@ void MOTAdaptor::CreateKeyBuffer(Relation rel, MOTFdwStateSt* festate, int start Datum val = ExecEvalExpr((ExprState*)(expr), festate->m_econtext, &is_null, nullptr); if (is_null) { MOT_ASSERT((offset + fieldLengths[i]) <= keyLength); - FILL_KEY_NULL(desc->attrs[orgCols[i] - 1]->atttypid, buf + offset, fieldLengths[i]); + errno_t erc = memset_s(buf + offset, fieldLengths[i], 0x00, fieldLengths[i]); + securec_check(erc, "\0", "\0"); } else { MOT::Column* col = festate->m_table->GetField(orgCols[i]); uint8_t fill = 0x00; diff --git a/src/gausskernel/storage/mot/jit_exec/src/jit_llvm_blocks.cpp b/src/gausskernel/storage/mot/jit_exec/src/jit_llvm_blocks.cpp index 16caef7cd..dd172dc4c 100644 --- a/src/gausskernel/storage/mot/jit_exec/src/jit_llvm_blocks.cpp +++ b/src/gausskernel/storage/mot/jit_exec/src/jit_llvm_blocks.cpp @@ -2022,7 +2022,8 @@ static llvm::Value* buildAggregateCount( static bool buildAggregateMaxMin(JitLlvmCodeGenContext* ctx, JitAggregate* aggregate, llvm::Value* var_expr) { - bool result = true; + bool result = false; + llvm::Value* aggregateExpr = nullptr; // we first check if the min/max value is null and if so just store the column value JIT_IF_BEGIN(test_max_min_value_null) @@ -2035,21 +2036,25 @@ static bool buildAggregateMaxMin(JitLlvmCodeGenContext* ctx, JitAggregate* aggre llvm::Value* current_aggregate = AddGetAggValue(ctx); switch (aggregate->_aggreaget_op) { case JIT_AGGREGATE_MAX: - result = buildAggregateMax(ctx, aggregate, current_aggregate, var_expr); + aggregateExpr = buildAggregateMax(ctx, aggregate, current_aggregate, var_expr); break; case JIT_AGGREGATE_MIN: - result = buildAggregateMin(ctx, aggregate, current_aggregate, var_expr); + aggregateExpr = buildAggregateMin(ctx, aggregate, current_aggregate, var_expr); break; default: MOT_REPORT_ERROR( MOT_ERROR_INTERNAL, "JIT Compile", "Invalid aggregate operator %d", (int)aggregate->_aggreaget_op); - result = false; break; } JIT_IF_END() + if (aggregateExpr != nullptr) { + AddSetAggValue(ctx, aggregateExpr); + result = true; + } + return result; } diff --git a/src/gausskernel/storage/mot/jit_exec/src/jit_tvm.cpp b/src/gausskernel/storage/mot/jit_exec/src/jit_tvm.cpp index 2e2dfc229..966d9655e 100644 --- a/src/gausskernel/storage/mot/jit_exec/src/jit_tvm.cpp +++ b/src/gausskernel/storage/mot/jit_exec/src/jit_tvm.cpp @@ -1054,7 +1054,9 @@ void Builder::CreateRet(Instruction* instruction) Instruction* Builder::CreateConst(uint64_t value) { - return new (std::nothrow) ConstInstruction(value); + Instruction* result = new (std::nothrow) ConstInstruction(value); + _current_function->addInstruction(result); + return result; } BasicBlock* Builder::GetInsertBlock() diff --git a/src/gausskernel/storage/mot/jit_exec/src/jit_tvm_blocks.cpp b/src/gausskernel/storage/mot/jit_exec/src/jit_tvm_blocks.cpp index 255e428c1..fe88b8380 100644 --- a/src/gausskernel/storage/mot/jit_exec/src/jit_tvm_blocks.cpp +++ b/src/gausskernel/storage/mot/jit_exec/src/jit_tvm_blocks.cpp @@ -1981,7 +1981,8 @@ static Expression* buildAggregateCount( static bool buildAggregateMaxMin(JitTvmCodeGenContext* ctx, JitAggregate* aggregate, Expression* var_expr) { - bool result = true; + bool result = false; + Expression* aggregateExpr = nullptr; // we first check if the min/max value is null and if so just store the column value JIT_IF_BEGIN(test_max_min_value_null) @@ -1994,21 +1995,25 @@ static bool buildAggregateMaxMin(JitTvmCodeGenContext* ctx, JitAggregate* aggreg Expression* current_aggregate = AddGetAggValue(ctx); switch (aggregate->_aggreaget_op) { case JIT_AGGREGATE_MAX: - result = buildAggregateMax(ctx, aggregate, current_aggregate, var_expr); + aggregateExpr = buildAggregateMax(ctx, aggregate, current_aggregate, var_expr); break; case JIT_AGGREGATE_MIN: - result = buildAggregateMin(ctx, aggregate, current_aggregate, var_expr); + aggregateExpr = buildAggregateMin(ctx, aggregate, current_aggregate, var_expr); break; default: MOT_REPORT_ERROR( MOT_ERROR_INTERNAL, "JIT Compile", "Invalid aggregate operator %d", (int)aggregate->_aggreaget_op); - result = false; break; } JIT_IF_END() + if (aggregateExpr != nullptr) { + AddSetAggValue(ctx, aggregateExpr); + result = true; + } + return result; } diff --git a/src/gausskernel/storage/mot/jit_exec/src/jit_tvm_funcs.h b/src/gausskernel/storage/mot/jit_exec/src/jit_tvm_funcs.h index c1fb55f49..3d3e3f2ed 100644 --- a/src/gausskernel/storage/mot/jit_exec/src/jit_tvm_funcs.h +++ b/src/gausskernel/storage/mot/jit_exec/src/jit_tvm_funcs.h @@ -2362,7 +2362,9 @@ public: ~SaveAvgArrayInstruction() final { - _avg_array = nullptr; + if (_avg_array != nullptr) { + delete _avg_array; + } } uint64_t Exec(tvm::ExecContext* exec_context) final @@ -2460,7 +2462,9 @@ public: ~SetAggValueInstruction() final { - _value = nullptr; + if (_value != nullptr) { + delete _value; + } } Datum Exec(tvm::ExecContext* exec_context) final