CONCPP-70 statement::addBatch test sporadically failed

The reason was, that filling of vector with executeBatch results was
done using undefined behavior - it was done using [] operator member
access for not existing members. The values were inserted correctly, but
the size of vector happened to not always be reported correctly.
Added similar test to preparedstatement.
Made clearBatch not to reset current set of parameters - that looks
like to be that jdbc specs expect.
Fixed other such cases of wrong vector use.
Made some to/from text conversion to use C locale.
This commit is contained in:
Lawrin Novitsky
2021-02-13 22:03:15 +01:00
parent 029bd01df6
commit be22353bc2
14 changed files with 184 additions and 50 deletions

View File

@ -20,13 +20,16 @@
#include <iostream>
#include <memory>
#include <regex>
#include <string>
#include <sstream>
#include <cmath>
#include "mariadb/conncpp.hpp"
int main(int argc, char** argv)
{
// Instantiate Driver
sql::Driver* driver = sql::mariadb::get_driver_instance();
// Instantiate Driver
sql::Driver* driver = sql::mariadb::get_driver_instance();
// Configure Connection
sql::SQLString url("jdbc:mariadb://localhost/test");

View File

@ -162,7 +162,7 @@ namespace mariadb
void ClientSidePreparedStatement::addBatch()
{
std::vector<Shared::ParameterHolder> holder(prepareResult->getParamCount());
for (int32_t i= 0; i <holder.size(); i++) {
for (int32_t i= 0; i < holder.size(); i++) {
holder[i]= parameters[i];
if (!holder[i]) {
logger->error(
@ -182,8 +182,6 @@ namespace mariadb
void ClientSidePreparedStatement::clearBatch()
{
parameterList.clear();
hasLongData= false;
this->parameters.clear(); // clear() doesn't change capacity
}
/** {inheritdoc}. */
@ -454,6 +452,7 @@ namespace mariadb
{
parameters.clear();
parameters.assign(prepareResult->getParamCount(), Shared::ParameterHolder());
hasLongData= false;
}
void ClientSidePreparedStatement::close()

View File

@ -38,6 +38,7 @@ namespace mariadb
Results::Results()
: statement(nullptr)
, serverPrepResult(nullptr)
, rewritten(false)
{
this->fetchSize= 0;
this->maxFieldSize= 0;
@ -95,6 +96,7 @@ namespace mariadb
, resultSetConcurrency(resultSetConcurrency)
, autoIncrement(autoIncrement)
, autoGeneratedKeys(autoGeneratedKeys)
, rewritten(false)
{
//this->cmdInformation

View File

@ -186,7 +186,6 @@ namespace sql
void ServerSidePreparedStatement::clearBatch()
{
queryParameters.clear();
hasLongData= false;
}
ParameterMetaData* ServerSidePreparedStatement::getParameterMetaData()
@ -348,6 +347,8 @@ namespace sql
void ServerSidePreparedStatement::clearParameters()
{
currentParameterHolder.clear();
//currentParameterHolder.assign(serverPrepareResult->getParamCount(), Shared::ParameterHolder());
hasLongData= false;
}

View File

@ -104,14 +104,15 @@ namespace mariadb
batchRes.reserve(std::max(updateCounts.size(), expectedSize));
size_t pos= 0;
size_t pos= updateCounts.size();
for (auto& updCnt : updateCounts) {
batchRes[pos++]= static_cast<int32_t>(updCnt);
batchRes.push_back(static_cast<int32_t>(updCnt));
}
while (pos < expectedSize) {
batchRes[pos++]= Statement::EXECUTE_FAILED;
batchRes.push_back(Statement::EXECUTE_FAILED);
++pos;
}
return batchRes;
@ -122,10 +123,9 @@ namespace mariadb
{
batchRes.clear();
batchRes.reserve(updateCounts.size());
size_t pos= 0;
for (auto& updCnt : updateCounts) {
batchRes[pos++]= static_cast<int32_t>(updCnt);
batchRes.push_back(static_cast<int32_t>(updCnt));
}
return batchRes;
}
@ -159,13 +159,14 @@ namespace mariadb
largeBatchRes.reserve(std::max(updateCounts.size(), expectedSize));
size_t pos= 0;
size_t pos= updateCounts.size();
for (auto& updCnt : updateCounts) {
largeBatchRes[pos++] = updCnt;
largeBatchRes.push_back(updCnt);
}
while (pos < expectedSize) {
largeBatchRes[pos++]= Statement::EXECUTE_FAILED;
largeBatchRes.push_back(Statement::EXECUTE_FAILED);
++pos;
}
return largeBatchRes;

View File

@ -65,6 +65,18 @@ namespace mariadb
#endif
long double RowProtocol::stringToDouble(const char* str, uint32_t len)
{
std::string doubleAsString(str, len);
std::istringstream convStream(doubleAsString);
std::locale C("C");
long double result;
convStream.imbue(C);
convStream >> result;
return result;
}
RowProtocol::RowProtocol(uint32_t _maxFieldSize, Shared::Options options)
: maxFieldSize(_maxFieldSize)
, options(options)

View File

@ -80,6 +80,7 @@ public:
static std::regex dateRegex;
static std::regex timeRegex;
static std::regex timestampRegex;
static long double stringToDouble(const char* str, uint32_t len);
protected:
static int32_t NULL_LENGTH_ ; /*-1*/

View File

@ -924,27 +924,27 @@ namespace capi
std::vector<std::vector<Shared::ParameterHolder>>& parametersList,
bool hasLongData)
{
bool needToRelease= false;
cmdPrologue();
if (options->useBulkStmts
&& !hasLongData
&& results->getAutoGeneratedKeys()==Statement::NO_GENERATED_KEYS
&& versionGreaterOrEqual(10,2,7)
&& executeBulkBatch(results, sql, serverPrepareResult, parametersList)){
&& executeBulkBatch(results, sql, serverPrepareResult, parametersList)) {
return true;
}
if (!options->useBatchMultiSend){
if (!options->useBatchMultiSend) {
return false;
}
initializeBatchReader();
capi::MYSQL_STMT *stmt= nullptr;
if (serverPrepareResult == nullptr)
{
if (serverPrepareResult == nullptr) {
serverPrepareResult= prepare(sql, true);
needToRelease= true;
}
stmt= serverPrepareResult->getStatementId();
@ -952,14 +952,13 @@ namespace capi
std::size_t totalExecutionNumber= parametersList.size();
//std::size_t parameterCount= serverPrepareResult->getParameters().size();
for (auto& paramset : parametersList)
{
for (auto& paramset : parametersList) {
executePreparedQuery(true, serverPrepareResult, results, paramset);
}
//TODO:!!! what to do with serverPrepareResult here? delete? leaking otherwise. but I don't think we can do that here
delete serverPrepareResult;
if (needToRelease) {
delete serverPrepareResult;
}
return true;
}

View File

@ -370,6 +370,8 @@ namespace capi
}
std::ostringstream timestamp;
std::locale C("C");
timestamp.imbue(C);
timestamp << timestampsPart[0] << "-";
timestamp << (timestampsPart[1] < 10 ? "0" : "") << timestampsPart[1] << "-";
@ -933,7 +935,7 @@ namespace capi
case MYSQL_TYPE_DECIMAL:
case MYSQL_TYPE_LONGLONG:
try {
return std::stold(std::string(fieldBuf.arr + pos, length));
return stringToDouble(fieldBuf.arr + pos, length);
}
// Common parent for std::invalid_argument and std::out_of_range
catch (std::logic_error& nfe) {

View File

@ -34,6 +34,7 @@
#include <locale>
#include <sstream>
#include <limits>
#include <cmath>
//#include "driver/mysql_error.h"
//Prevent windows min() macro because of std::numeric_limits<int>::min()
@ -1100,11 +1101,11 @@ void bugs::bug22292073()
SKIP("Server does not support tested functionality(JSON type)")
}
stmt->execute("DROP TABLE IF EXISTS bug22292073");
stmt->execute("create table bug22292073 (jdoc JSON);" );
stmt->execute("create table bug22292073 (jdoc JSON);");
stmt->execute("insert into bug22292073 values('{ \"name\": \"abc\", \"age\": 1 , \"misc\":\
1.2}'), ('{ \"name\": \"abcdef\", \"age\": 31 , \"misc\": 1.237843}');" );
pstmt.reset( con->prepareStatement("select JSON_EXTRACT(jdoc, '$.age') from bug22292073;") );
res.reset( pstmt->executeQuery() );
1.2}'), ('{ \"name\": \"abcdef\", \"age\": 31 , \"misc\": 1.237843}');");
pstmt.reset(con->prepareStatement("select JSON_EXTRACT(jdoc, '$.age') from bug22292073;"));
res.reset(pstmt->executeQuery());
res->next();
@ -1149,14 +1150,14 @@ void bugs::bug22292073()
void bugs::bug23212333()
{
uint64_t charCount= 256*1024+1;
uint64_t charCount = 256 * 1024 + 1;
stmt->executeUpdate("drop table if exists bug23212333");
stmt->executeUpdate("create table bug23212333(id longtext)");
pstmt.reset(con->prepareStatement("insert into bug23212333 VALUES(?)"));
std::string buffer;
buffer.append(charCount,'A');
buffer.append(charCount, 'A');
pstmt->setString(1, buffer);
pstmt->execute();
@ -1175,12 +1176,12 @@ void bugs::bug17227390()
{
std::locale::global(std::locale("fr_CA.UTF-8"));
for (int i=0; i < 2; ++i)
for (int i = 0; i < 2; ++i)
{
if (i == 0)
{
pstmt.reset( con->prepareStatement("select 1.001 as number;") );
res.reset( pstmt->executeQuery() );
pstmt.reset(con->prepareStatement("select 1.001 as number;"));
res.reset(pstmt->executeQuery());
}
else
{
@ -1191,7 +1192,6 @@ void bugs::bug17227390()
ASSERT_EQUALS(1.001L, res->getDouble(1));
ASSERT_EQUALS(1.001L, res->getDouble("number"));
}
}
catch (...) {
@ -1529,8 +1529,8 @@ void bugs::concpp62()
res.reset(stmt->executeQuery("SELECT ts, TIME(ts) from concpp62"));
ASSERT(res->next());
ASSERT_EQUALS(res->getString(1), "2015-01-20 16:14:36.709649");
ASSERT_EQUALS(res->getString(2), "16:14:36.709649");
ASSERT_EQUALS("2015-01-20 16:14:36.709649", res->getString(1));
ASSERT_EQUALS("16:14:36.709649", res->getString(2));
stmt->execute("DROP TABLE IF EXISTS concpp62");
}

View File

@ -1595,5 +1595,66 @@ void preparedstatement::executeQuery()
}
}
void preparedstatement::addBatch()
{
stmt->executeUpdate("DROP TABLE IF EXISTS testAddBatchPs");
stmt->executeUpdate("CREATE TABLE testAddBatchPs "
"(id int not NULL)");
pstmt.reset(con->prepareStatement("INSERT INTO testAddBatchPs VALUES(?)"));
pstmt->setInt(1, 1);
pstmt->addBatch();
pstmt->setInt(1, 2);
pstmt->addBatch();
pstmt->setInt(1, 3);
pstmt->addBatch();
const sql::Ints& batchRes = pstmt->executeBatch();
res.reset(stmt->executeQuery("SELECT MIN(id), MAX(id), SUM(id), count(*) FROM testAddBatchPs"));
ASSERT(res->next());
ASSERT_EQUALS(1, res->getInt(1));
ASSERT_EQUALS(3, res->getInt(2));
ASSERT_EQUALS(6, res->getInt(3));
ASSERT_EQUALS(3, res->getInt(4));
ASSERT_EQUALS(3ULL, static_cast<uint64_t>(batchRes.size()));
ASSERT_EQUALS(1, batchRes[0]);
ASSERT_EQUALS(1, batchRes[1]);
ASSERT_EQUALS(1, batchRes[2]);
////// The same, but for executeLargeBatch
stmt->executeUpdate("DELETE FROM testAddBatchPs");
pstmt->clearBatch();
pstmt->clearParameters();
pstmt->setInt(1, 4);
pstmt->addBatch();
pstmt->setInt(1, 5);
pstmt->addBatch();
pstmt->setInt(1, 6);
pstmt->addBatch();
const sql::Longs& batchLRes = pstmt->executeLargeBatch();
res.reset(stmt->executeQuery("SELECT MIN(id), MAX(id), SUM(id), count(*) FROM testAddBatchPs"));
ASSERT(res->next());
ASSERT_EQUALS(4, res->getInt(1));
ASSERT_EQUALS(6, res->getInt(2));
ASSERT_EQUALS(15, res->getInt(3));
ASSERT_EQUALS(3, res->getInt(4));
ASSERT_EQUALS(3ULL, static_cast<uint64_t>(batchLRes.size()));
ASSERT_EQUALS(1LL, batchLRes[0]);
ASSERT_EQUALS(1LL, batchLRes[1]);
ASSERT_EQUALS(1LL, batchLRes[2]);
stmt->executeUpdate("DROP TABLE testAddBatchPs");
}
} /* namespace preparedstatement */
} /* namespace testsuite */

View File

@ -68,6 +68,7 @@ public:
TEST_CASE(getWarnings);
TEST_CASE(blob);
TEST_CASE(executeQuery);
TEST_CASE(addBatch);
}
/**
@ -142,7 +143,7 @@ public:
*/
void executeQuery();
void addBatch();
};
REGISTER_FIXTURE(preparedstatement);

View File

@ -501,7 +501,22 @@ void resultset::getTypes()
}
else
{
ASSERT_EQUALS(res->getInt(1), res->getInt(id));
try
{
ASSERT_EQUALS(res->getInt(1), res->getInt(id));
}
catch (sql::SQLException & e)
{
if (!isNumber)
{
ASSERT_EQUALS(1264, e.getErrorCode());
ASSERT_EQUALS("22003", e.getSQLState());
}
else
{
throw e;
}
}
}
try
@ -542,7 +557,7 @@ void resultset::getTypes()
}
res->first();
if (it->is_negative || !inUintRange)
if (!isNumber || it->is_negative || !inUintRange)
{
try
{
@ -602,7 +617,22 @@ void resultset::getTypes()
// intValue >= 0 means it's in int64 range
if (it->is_negative || intValue >= 0)
{
ASSERT_EQUALS(res->getInt64(id), res->getInt64(1));
try
{
ASSERT_EQUALS(res->getInt64(id), res->getInt64(1));
}
catch (sql::SQLException & e)
{
if (!isNumber)
{
ASSERT_EQUALS(1264, e.getErrorCode());
ASSERT_EQUALS("22003", e.getSQLState());
}
else
{
throw e;
}
}
}
try
{
@ -642,7 +672,7 @@ void resultset::getTypes()
}
res->first();
if (it->is_negative)
if (it->is_negative || !isNumber)
{
try
{
@ -720,7 +750,7 @@ void resultset::getTypes()
}
}
if (inIntRange && (pres->getInt(id) != res->getInt(id)))
if (isNumber && inIntRange && (pres->getInt(id) != res->getInt(id)))
{
msg.str("");
msg << "... \t\tWARNING - getInt(), PS: '" << pres->getInt(id) << "'";
@ -729,7 +759,7 @@ void resultset::getTypes()
got_warning=true;
}
if (!it->is_negative && (pres->getUInt64(id) != res->getUInt64(id)))
if (isNumber && !it->is_negative && (pres->getUInt64(id) != res->getUInt64(id)))
{
msg.str("");
msg << "... \t\tWARNING - getUInt64(), PS: '" << pres->getUInt64(id) << "'";
@ -738,12 +768,12 @@ void resultset::getTypes()
got_warning=true;
}
if (inUintRange)
if (isNumber && inUintRange)
{
ASSERT_EQUALS(pres->getUInt(id), res->getUInt(id));
}
if ((it->is_negative || intValue >= 0) && pres->getInt64(id) != res->getInt64(id))
if (isNumber && (it->is_negative || intValue >= 0) && pres->getInt64(id) != res->getInt64(id))
{
msg.str("");
msg << "... \t\tWARNING - getInt64(), PS: '" << pres->getInt64(id) << "'";
@ -753,7 +783,7 @@ void resultset::getTypes()
}
// ASSERT_EQUALS(pres->getInt64(id), res->getInt64(id));
if (!it->is_negative && (pres->getUInt64(id) != res->getUInt64(id)))
if (isNumber && !it->is_negative && (pres->getUInt64(id) != res->getUInt64(id)))
{
msg.str("");
msg << "... \t\tWARNING - getUInt64(), PS: '" << pres->getUInt64(id) << "'";

View File

@ -639,9 +639,31 @@ void statement::addBatch()
ASSERT_EQUALS(6, res->getInt(3));
ASSERT_EQUALS(3, res->getInt(4));
ASSERT_EQUALS(3ULL, static_cast<uint64_t>(batchRes.size()));
ASSERT_EQUALS(static_cast<int32_t>(sql::Statement::SUCCESS_NO_INFO), batchRes[0]);
ASSERT_EQUALS(static_cast<int32_t>(sql::Statement::SUCCESS_NO_INFO), batchRes[1]);
ASSERT_EQUALS(static_cast<int32_t>(sql::Statement::SUCCESS_NO_INFO), batchRes[2]);
ASSERT_EQUALS(1, batchRes[0]);
ASSERT_EQUALS(1, batchRes[1]);
ASSERT_EQUALS(1, batchRes[2]);
////// The same, but for executeLargeBatch
st2->executeUpdate("DELETE FROM testAddBatch");
stmt->clearBatch();
stmt->addBatch("INSERT INTO testAddBatch VALUES(4),(11)");
stmt->addBatch("INSERT INTO testAddBatch VALUES(5)");
stmt->addBatch("INSERT INTO testAddBatch VALUES(6)");
const sql::Longs& batchLRes = stmt->executeLargeBatch();
res.reset(st2->executeQuery("SELECT MIN(id), MAX(id), SUM(id), count(*) FROM testAddBatch"));
ASSERT(res->next());
ASSERT_EQUALS(4, res->getInt(1));
ASSERT_EQUALS(11, res->getInt(2));
ASSERT_EQUALS(26, res->getInt(3));
ASSERT_EQUALS(4, res->getInt(4));
ASSERT_EQUALS(3ULL, static_cast<uint64_t>(batchLRes.size()));
ASSERT_EQUALS(2LL, batchLRes[0]);
ASSERT_EQUALS(1LL, batchLRes[1]);
ASSERT_EQUALS(1LL, batchLRes[2]);
stmt->executeUpdate("DROP TABLE IF EXISTS testAddBatch");
}