From b29d0c9daa43a3cc4c62e5963f08179f3b5ef33b Mon Sep 17 00:00:00 2001 From: drrtuy Date: Mon, 24 Feb 2020 17:22:52 +0300 Subject: [PATCH] MCOL-641 Changed the hint to search for GTest headers. This commit introduces DataConvert UTs. DataConvert::decimalToString now can negative values. Next version for Row::toString(), applyMapping UT checks. Row:equals() is now wide-DECIMAL aware. --- cmake/FindGTest.cmake | 3 +- dbcon/execplan/calpontsystemcatalog.h | 6 ++ utils/dataconvert/CMakeLists.txt | 6 ++ utils/dataconvert/dataconvert.cpp | 30 ++++----- utils/rowgroup/rowgroup-tests.cpp | 27 ++++++-- utils/rowgroup/rowgroup.cpp | 16 ++--- utils/rowgroup/rowgroup.h | 93 +++++++++++++++++++++++++-- 7 files changed, 142 insertions(+), 39 deletions(-) diff --git a/cmake/FindGTest.cmake b/cmake/FindGTest.cmake index 7b1fb3957..5860ca415 100644 --- a/cmake/FindGTest.cmake +++ b/cmake/FindGTest.cmake @@ -17,10 +17,9 @@ find_library(PTHREAD_LIBRARY HINTS ${GTEST_ROOT_DIR}/lib ) - find_path(GTEST_INCLUDE_DIR NAMES gtest.h - HINTS ${GTEST_ROOT_DIR}/include + HINTS ${GTEST_ROOT_DIR}/include/gtest ) include(FindPackageHandleStandardArgs) diff --git a/dbcon/execplan/calpontsystemcatalog.h b/dbcon/execplan/calpontsystemcatalog.h index 0f7b66aa2..4450ce429 100644 --- a/dbcon/execplan/calpontsystemcatalog.h +++ b/dbcon/execplan/calpontsystemcatalog.h @@ -1004,6 +1004,12 @@ inline bool isNumeric(const execplan::CalpontSystemCatalog::ColDataType type) } } +inline bool isDecimal(const execplan::CalpontSystemCatalog::ColDataType type) +{ + return (type == execplan::CalpontSystemCatalog::DECIMAL || + type == execplan::CalpontSystemCatalog::UDECIMAL); +} + /** convenience function to determine if column type is an * unsigned type */ diff --git a/utils/dataconvert/CMakeLists.txt b/utils/dataconvert/CMakeLists.txt index 3e37b3c0e..168efdf80 100644 --- a/utils/dataconvert/CMakeLists.txt +++ b/utils/dataconvert/CMakeLists.txt @@ -11,3 +11,9 @@ add_library(dataconvert SHARED ${dataconvert_LIB_SRCS}) add_dependencies(dataconvert loggingcpp) install(TARGETS dataconvert DESTINATION ${ENGINE_LIBDIR} COMPONENT columnstore-engine) + +if (WITH_DATACONVERT_UT) + add_executable(dataconvert_tests dataconvert-tests.cpp) + target_link_libraries(dataconvert_tests ${ENGINE_LDFLAGS} ${GTEST_LIBRARIES} ${ENGINE_EXEC_LIBS} ${MARIADB_CLIENT_LIBS}) + install(TARGETS dataconvert_tests DESTINATION ${ENGINE_BINDIR} COMPONENT columnstore-platform) +endif() diff --git a/utils/dataconvert/dataconvert.cpp b/utils/dataconvert/dataconvert.cpp index 619ac6c77..6838cebbe 100644 --- a/utils/dataconvert/dataconvert.cpp +++ b/utils/dataconvert/dataconvert.cpp @@ -1220,7 +1220,7 @@ size_t DataConvert::writeIntPart(T* dec, char* p, if (buflen <= p-original_p) { - throw QueryDataExcept("toString() char buffer overflow.", formatErr); + throw QueryDataExcept("writeIntPart() char buffer overflow.", formatErr); } return p-original_p; } @@ -1352,31 +1352,27 @@ void DataConvert::decimalToString(T* valuePtr, unsigned int buflen, cscDataType colDataType) // We don't need the last one { - if (*valuePtr < static_cast(0)) + T value = *valuePtr; + char* ptr = &buf[0]; + size_t l1 = buflen; + if (value < static_cast(0)) { - *buf++ = '-'; - *valuePtr *= -1; + *ptr++ = '-'; + value *= -1; + idbassert(l1 >= 2); + l1--; } - toString(valuePtr, buf, buflen); + //we want to move the last scale chars right by one spot to insert the dp + //we want to move the trailing null as well, so it's really scale+1 chars + + toString(&value, ptr, buflen); // Biggest ColumnStore supports is DECIMAL(38,x), or 38 total digits+dp+sign for column if (scale == 0) return; - //we want to move the last scale chars right by one spot to insert the dp - //we want to move the trailing null as well, so it's really scale+1 chars - size_t l1 = strlen(buf); - char* ptr = &buf[0]; - - if (*valuePtr < 0) - { - ptr++; - idbassert(l1 >= 2); - l1--; - } - //need to make sure we have enough leading zeros for this to work... //at this point scale is always > 0 size_t l2 = 1; diff --git a/utils/rowgroup/rowgroup-tests.cpp b/utils/rowgroup/rowgroup-tests.cpp index e63f0b2db..7b9708526 100644 --- a/utils/rowgroup/rowgroup-tests.cpp +++ b/utils/rowgroup/rowgroup-tests.cpp @@ -67,14 +67,15 @@ class RowDecimalTest : public ::testing::Test { rg.setData(&rgD); rg.initRow(&r); + rg.initRow(&rOutMappingCheck); rowSize = r.getSize(); rg.getRow(0, &r); int128_t nullValue = 0; int128_t bigValue = 0; uint64_t* uint128_pod = reinterpret_cast(&nullValue); - uint128_pod[0] = joblist::BINARYEMPTYROW; - uint128_pod[1] = joblist::BINARYNULL; + uint128_pod[0] = joblist::BINARYNULL; + uint128_pod[1] = joblist::BINARYEMPTYROW; bigValue = -static_cast(0xFFFFFFFF)*0xFFFFFFFFFFFFFFFF; //char buf[utils::precisionByWidth(16)+3]; //memset(&buf[0], 0, sizeof(buf)); @@ -82,7 +83,7 @@ class RowDecimalTest : public ::testing::Test { //dataconvert::DataConvert::decimalToString(&bigValue, scale1, buf, // utils::precisionByWidth(sizeof(bigValue))+3,types[0]); //std::cout << buf << std::endl; - sValueVector.push_back(nullValue-2); + sValueVector.push_back(nullValue); sValueVector.push_back(-42); sValueVector.push_back(bigValue); sValueVector.push_back(0); @@ -136,6 +137,7 @@ class RowDecimalTest : public ::testing::Test { // void TearDown() override {} rowgroup::Row r; + rowgroup::Row rOutMappingCheck; rowgroup::RowGroup rg; rowgroup::RGData rgD; uint32_t rowSize; @@ -200,7 +202,7 @@ TEST_F(RowDecimalTest, toStringCheck) { exemplarVector.push_back(std::string("0: -42 42 -121 -121 -121 -121 ")); exemplarVector.push_back(std::string("0: -79228162495817593515539431425 -79228162495817593515539431425 0 0 0 0 ")); exemplarVector.push_back(std::string("0: 0 0 129 129 129 -127 ")); - exemplarVector.push_back(std::string("0: -18446744073709551618 -18446744073709551618 9223372036854775807 2147483647 32767 127 ")); + exemplarVector.push_back(std::string("0: -3 -3 9223372036854775807 2147483647 32767 127 ")); rg.getRow(0, &r); r.initToNull(); @@ -212,7 +214,22 @@ TEST_F(RowDecimalTest, toStringCheck) { TEST_F(RowDecimalTest, toCSVCheck) { } + TEST_F(RowDecimalTest, applyMappingCheck) { + int mapping[] = {0, 1, -1, -1, -1, -1}; + rg.getRow(1, &r); + rg.getRow(2, &rOutMappingCheck); + int128_t* s128Value = rOutMappingCheck.getBinaryField(0); + uint128_t* u128Value = rOutMappingCheck.getBinaryField(1); + EXPECT_NE(sValueVector[1], *s128Value); + EXPECT_NE(uValueVector[1], *u128Value); + applyMapping(mapping, r, &rOutMappingCheck); + s128Value = rOutMappingCheck.getBinaryField(0); + EXPECT_EQ(sValueVector[1], *s128Value); + u128Value = rOutMappingCheck.getBinaryField(1); + EXPECT_EQ(uValueVector[1], *u128Value); } -TEST_F(RowDecimalTest, equalsCheck) { + +// WIP +TEST_F(RowDecimalTest, RowEqualsCheck) { } diff --git a/utils/rowgroup/rowgroup.cpp b/utils/rowgroup/rowgroup.cpp index bd84117a2..448c8ad5c 100644 --- a/utils/rowgroup/rowgroup.cpp +++ b/utils/rowgroup/rowgroup.cpp @@ -638,12 +638,12 @@ string Row::toString() const // WIP case CalpontSystemCatalog::DECIMAL: case CalpontSystemCatalog::UDECIMAL: - if (colWidths[i] > MAXLEGACYWIDTH) + if (colWidths[i] == sizeof(int128_t)) { char *buf = (char*)alloca(precision[i] + 3); // empty the buffer - dataconvert::DataConvert::toString(getBinaryField(i), - buf, precision[i]+3); //WIP scale[i] + dataconvert::DataConvert::decimalToString(getBinaryField(i), + scale[i], buf, precision[i]+3, types[i]); os << buf << " "; break; } @@ -1048,8 +1048,6 @@ bool Row::isNullValue(uint32_t colIndex) const case 8: return (*((uint64_t*) &data[offsets[colIndex]]) == joblist::CHAR8NULL); - case 16: - std::cout << __FILE__<< ":" <<__LINE__ << " Fix for 16 Bytes ?" << std::endl; default: return (*((uint64_t*) &data[offsets[colIndex]]) == *((uint64_t*) joblist::CPNULLSTRMARK.c_str())); } @@ -1646,10 +1644,10 @@ void applyMapping(const int* mapping, const Row& in, Row* out) // WIP this doesn't look right b/c we can pushdown colType // Migrate to offset based methods here // code precision 2 width convertor - else if (UNLIKELY(in.getColTypes()[i] == execplan::CalpontSystemCatalog::DECIMAL && - in.getColumnWidth(i) == 16)) - out->setBinaryField_offset(in.getBinaryField(i), 16, - out->getOffset(mapping[i])); + else if (UNLIKELY(execplan::isDecimal(in.getColTypes()[i]) + && in.getColumnWidth(i) == 16)) + out->setBinaryField_offset(in.getBinaryField(i), 16, + out->getOffset(mapping[i])); else if (in.isUnsigned(i)) out->setUintField(in.getUintField(i), mapping[i]); else diff --git a/utils/rowgroup/rowgroup.h b/utils/rowgroup/rowgroup.h index 6a2bc7961..831fa6d0b 100644 --- a/utils/rowgroup/rowgroup.h +++ b/utils/rowgroup/rowgroup.h @@ -375,6 +375,8 @@ public: inline uint64_t getUintField(uint32_t colIndex) const; template inline int64_t getIntField(uint32_t colIndex) const; inline int64_t getIntField(uint32_t colIndex) const; + template + inline bool equals(T* value, uint32_t colIndex) const; template inline bool equals(uint64_t val, uint32_t colIndex) const; inline bool equals(long double val, uint32_t colIndex) const; bool equals(const std::string& val, uint32_t colIndex) const; @@ -664,6 +666,12 @@ inline bool Row::inStringTable(uint32_t col) const return strings && getColumnWidth(col) >= sTableThreshold && !forceInline[col]; } +template +inline bool Row::equals(T* value, uint32_t colIndex) const +{ + return reinterpret_cast(&data[offsets[colIndex]]) == value; +} + template inline bool Row::equals(uint64_t val, uint32_t colIndex) const { @@ -681,8 +689,6 @@ inline bool Row::equals(uint64_t val, uint32_t colIndex) const case 8: return *((uint64_t*) &data[offsets[colIndex]]) == val; - case 16: - std::cout << __FILE__<< ":" <<__LINE__ << " Fix for 16 Bytes ?" << std::endl; default: idbassert(0); throw std::logic_error("Row::equals(): bad length."); @@ -710,8 +716,6 @@ inline uint64_t Row::getUintField(uint32_t colIndex) const case 8: return *((uint64_t*) &data[offsets[colIndex]]); - case 16: - std::cout << __FILE__<< ":" <<__LINE__ << " Fix for 16 Bytes ?" << std::endl; default: idbassert(0); throw std::logic_error("Row::getUintField(): bad length."); @@ -730,8 +734,6 @@ inline uint64_t Row::getUintField(uint32_t colIndex) const case 4: return *((uint32_t*) &data[offsets[colIndex]]); - case 16: - std::cout << __FILE__<< ":" <<__LINE__ << " Fix for 16 Bytes ?" << std::endl; case 8: return *((uint64_t*) &data[offsets[colIndex]]); @@ -1281,6 +1283,85 @@ inline uint64_t Row::hash(uint32_t lastCol) const return ret; } +inline bool Row::equals(const Row& r2, const std::vector& keyCols) const +{ + for (uint32_t i = 0; i < keyCols.size(); i++) + { + const uint32_t& col = keyCols[i]; + + cscDataType columnType = getColType(i); + + if (!isLongString(col)) + { + if (UNLIKELY(columnType == execplan::CalpontSystemCatalog::LONGDOUBLE)) + { + if (getLongDoubleField(i) != r2.getLongDoubleField(i)) + return false; + } + else if (UNLIKELY(execplan::isDecimal(columnType))) + { + if (getBinaryField(i) != r2.getBinaryField(i)) + return false; + } + else if (getUintField(col) != r2.getUintField(col)) + return false; + } + else + { + if (getStringLength(col) != r2.getStringLength(col)) + return false; + + if (memcmp(getStringPointer(col), r2.getStringPointer(col), getStringLength(col))) + return false; + } + } + + return true; +} + +inline bool Row::equals(const Row& r2, uint32_t lastCol) const +{ + // This check fires with empty r2 only. + if (lastCol >= columnCount) + return true; + + if (!useStringTable && !r2.useStringTable) + return !(memcmp(&data[offsets[0]], &r2.data[offsets[0]], offsets[lastCol + 1] - offsets[0])); + + for (uint32_t i = 0; i <= lastCol; i++) + { + cscDataType columnType = getColType(i); + if (!isLongString(i)) + { + if (UNLIKELY(getColType(i) == execplan::CalpontSystemCatalog::LONGDOUBLE)) + { + if (getLongDoubleField(i) != r2.getLongDoubleField(i)) + return false; + } + else if (UNLIKELY(execplan::isDecimal(columnType))) + { + if (getBinaryField(i) != r2.getBinaryField(i)) + return false; + } + + else if (getUintField(i) != r2.getUintField(i)) + return false; + } + else + { + uint32_t len = getStringLength(i); + + if (len != r2.getStringLength(i)) + return false; + + if (memcmp(getStringPointer(i), r2.getStringPointer(i), len)) + return false; + } + } + + return true; +} + inline bool Row::equals(const Row& r2) const { return equals(r2, columnCount - 1);