Skip to content

Commit

Permalink
[DB-24504] Fix conn.execute after conn.close error
Browse files Browse the repository at this point in the history
  • Loading branch information
Robert Buck committed Dec 19, 2018
1 parent c59c7df commit b1f165a
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 73 deletions.
130 changes: 75 additions & 55 deletions src/NuoJsConnection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ void Connection::doRollback()
class ExecuteWorker : public Nan::AsyncWorker
{
public:
ExecuteWorker(Nan::Callback* callback, Connection* self, NuoDB::PreparedStatement* statement, Options options)
: Nan::AsyncWorker(callback), self(self), statement(statement), options(options), hasResults(false)
ExecuteWorker(Nan::Callback* callback, Connection* self, NuoDB::PreparedStatement* statement, Options options, const char* error)
: Nan::AsyncWorker(callback), self(self), statement(statement), options(options), error(error), hasResults(false)
{
TRACE("ExecuteWorker::ExecuteWorker");
}
Expand All @@ -317,6 +317,10 @@ class ExecuteWorker : public Nan::AsyncWorker
virtual void Execute()
{
TRACE("ExecuteWorker::Execute");
if (error) {
SetErrorMessage(error);
return;
}
try {
hasResults = self->doExecute(statement);
} catch (std::exception& e) {
Expand All @@ -332,7 +336,6 @@ class ExecuteWorker : public Nan::AsyncWorker
if (hasResults) {
TRACE(">>>>>> HAS RESULTS");
results = ResultSet::createFrom(statement, options);
// todo result set maintain strong reference to connection to prevent connection from closing early
}
Local<Value> argv[] = {
Nan::Null(),
Expand All @@ -345,6 +348,7 @@ class ExecuteWorker : public Nan::AsyncWorker
Connection* self;
NuoDB::PreparedStatement* statement;
Options options;
const char* error;
bool hasResults;
};

Expand Down Expand Up @@ -394,11 +398,17 @@ NAN_METHOD(Connection::execute)
self->setAutoCommit(options.getAutoCommit());
self->setReadOnly(options.getReadOnly());

NuoDB::PreparedStatement* statement = self->createStatement(sql, binds);
const char* error = nullptr;
NuoDB::PreparedStatement* statement = nullptr;
try {
statement = self->createStatement(sql, binds);
} catch (std::exception& e) {
error = e.what();
}

Nan::Callback* callback = new Nan::Callback(info[infoIdx].As<Function>());

ExecuteWorker* worker = new ExecuteWorker(callback, self, statement, options);
ExecuteWorker* worker = new ExecuteWorker(callback, self, statement, options, error);
worker->SaveToPersistent("nuodb:Connection", info.This());
Nan::AsyncQueueWorker(worker);
}
Expand All @@ -412,67 +422,77 @@ NuoDB::PreparedStatement* Connection::createStatement(std::string sql, Local<Arr
throw std::runtime_error(message);
}

NuoDB::PreparedStatement* statement = connection->prepareStatement(sql.c_str());
for (size_t index = 0; index < binds->Length(); index++) {
Local<Value> value = binds->Get(index);

int sqlIdx = index + 1;
int sqlType = Type::fromEsType(typeOf(value));

// The top-level case statements below correspond to the five
// fundamental data types in ES. Within these types we need to
// check if it will result in a safe conversion (e.g. Number).
switch (sqlType) {
case NuoDB::SqlType::NUOSQL_NULL:
statement->setNull(sqlIdx, NuoDB::NUOSQL_NULL);
break;

case NuoDB::SqlType::NUOSQL_BOOLEAN:
statement->setBoolean(sqlIdx, toBool(value));
break;

case NuoDB::SqlType::NUOSQL_DOUBLE:
// If the value can be safely coerced to a sized integer
// or float without loss of precision, send a numeric
// value rather than a string. If we're dealing with
// a number that is larger than can be safely coerced,
// convert it to a string.
if (isInt16(value)) {
statement->setShort(sqlIdx, toInt16(value));
} else if (isInt32(value)) {
statement->setInt(sqlIdx, toInt32(value));
} else if (isFloat(value)) {
statement->setFloat(sqlIdx, toFloat(value));
} else {
statement->setDouble(sqlIdx, toDouble(value));
NuoDB::PreparedStatement* statement = nullptr;
try {
statement = connection->prepareStatement(sql.c_str());
for (size_t index = 0; index < binds->Length(); index++) {
Local<Value> value = binds->Get(index);

int sqlIdx = index + 1;
int sqlType = Type::fromEsType(typeOf(value));

// The top-level case statements below correspond to the five
// fundamental data types in ES. Within these types we need to
// check if it will result in a safe conversion (e.g. Number).
switch (sqlType) {
case NuoDB::SqlType::NUOSQL_NULL:
statement->setNull(sqlIdx, NuoDB::NUOSQL_NULL);
break;

case NuoDB::SqlType::NUOSQL_BOOLEAN:
statement->setBoolean(sqlIdx, toBool(value));
break;

case NuoDB::SqlType::NUOSQL_DOUBLE:
// If the value can be safely coerced to a sized integer
// or float without loss of precision, send a numeric
// value rather than a string. If we're dealing with
// a number that is larger than can be safely coerced,
// convert it to a string.
if (isInt16(value)) {
statement->setShort(sqlIdx, toInt16(value));
} else if (isInt32(value)) {
statement->setInt(sqlIdx, toInt32(value));
} else if (isFloat(value)) {
statement->setFloat(sqlIdx, toFloat(value));
} else {
statement->setDouble(sqlIdx, toDouble(value));
}
break;

case NuoDB::SqlType::NUOSQL_VARCHAR: {
statement->setString(sqlIdx, toString(value).c_str());
break;
}
break;

case NuoDB::SqlType::NUOSQL_VARCHAR: {
statement->setString(sqlIdx, toString(value).c_str());
break;
case NuoDB::NUOSQL_UNDEFINED:
if (isDate(value)) {
char buffer[80];
time_t seconds = (time_t)(toInt64(value) / 1000);
struct tm* timeinfo;
timeinfo = localtime(&seconds);
strftime(buffer, 80, "%F %T", timeinfo);
statement->setString(sqlIdx, buffer);
} else {
statement->setString(sqlIdx, toString(value).c_str());
}
break;
}

case NuoDB::NUOSQL_UNDEFINED:
if (isDate(value)) {
char buffer[80];
time_t seconds = (time_t)(toInt64(value) / 1000);
struct tm* timeinfo;
timeinfo = localtime(&seconds);
strftime(buffer, 80, "%F %T", timeinfo);
statement->setString(sqlIdx, buffer);
} else {
statement->setString(sqlIdx, toString(value).c_str());
}
break;
}
} catch (NuoDB::SQLException& e) {
throw std::runtime_error(e.getText());
}

return statement;
}

bool Connection::doExecute(NuoDB::PreparedStatement* statement)
{
if (!isConnected()) {
std::string message = ErrMsg::get(ErrMsgType::errConnectionClosed);
throw std::runtime_error(message);
}

try {
return statement->execute();
} catch (NuoDB::SQLException& e) {
Expand Down
78 changes: 60 additions & 18 deletions test/2.connection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('2. testing connections', function () {
});
});

it('2.2 raises an exception if calling method, commit(), after close', function (done) {
it('2.2 raises an exception if calling method, commit(), after closed', function (done) {
driver.connect(config, function (err, connection) {
should.not.exist(err);
connection.close(function (err) {
Expand All @@ -39,15 +39,57 @@ describe('2. testing connections', function () {
done();
});
});
}
);
});
});

it('2.3 raises an exception if calling method, rollback(), after closed', function (done) {
driver.connect(config, function (err, connection) {
should.not.exist(err);
connection.close(function (err) {
should.not.exist(err);
connection.rollback(function (err) {
should.exist(err);
should.strictEqual(err.message, "connection closed");
done();
});
});
});
});

it('2.4 raises an exception if calling method, close(), after closed', function (done) {
driver.connect(config, function (err, connection) {
should.not.exist(err);
connection.close(function (err) {
should.not.exist(err);
connection.close(function (err) {
should.exist(err);
should.strictEqual(err.message, "failed to close connection [connection closed]");
done();
});
});
});
});

it('2.3 setting auto-commit', function () {
it('2.5 raises an exception if calling method, execute(), after closed', function (done) {
driver.connect(config, function (err, connection) {
should.not.exist(err);
connection.close(function (err) {
should.not.exist(err);
connection.execute('SELECT * FROM SYSTEM.CONNECTIONS', function (err, results) {
should.exist(err);
should.not.exist(results);
should.strictEqual(err.message, "connection closed");
done();
});
});
});
});

describe('2.3.1 with non-boolean values raises an exception', function () {
it('2.10 setting auto-commit', function () {
driver.connect(config, function (err, connection) {
should.not.exist(err);

describe('2.10.1 with non-boolean values raises an exception', function () {

var defaultValue;

Expand All @@ -69,23 +111,23 @@ describe('2. testing connections', function () {
callback();
};

it('2.3.1 Negative - 0', function (done) {
it('2.10.1 Negative - 0', function (done) {
setAsGlobalOption(0, done);
});

it('2.3.2 Negative - negative number', function (done) {
it('2.10.2 Negative - negative number', function (done) {
setAsGlobalOption(-1, done);
});

it('2.3.3 Negative - positive number', function (done) {
setAsGlobalOption(-1, done);
it('2.10.3 Negative - positive number', function (done) {
setAsGlobalOption(1, done);
});

it('2.3.4 Negative - NaN', function (done) {
it('2.10.4 Negative - NaN', function (done) {
setAsGlobalOption(NaN, done);
});

it('2.3.5 Negative - undefined', function (done) {
it('2.10.5 Negative - undefined', function (done) {
setAsGlobalOption(undefined, done);
});
});
Expand All @@ -98,11 +140,11 @@ describe('2. testing connections', function () {
});
});

it('2.4 setting read-only', function () {
it('2.11 setting read-only', function () {
driver.connect(config, function (err, connection) {
should.not.exist(err);

describe('2.4.1 with non-boolean values raises an exception', function () {
describe('2.11.1 with non-boolean values raises an exception', function () {

var defaultValue;

Expand All @@ -124,23 +166,23 @@ describe('2. testing connections', function () {
callback();
};

it('2.4.1.1 Negative - 0', function (done) {
it('2.11.1.1 Negative - 0', function (done) {
setAsGlobalOption(0, done);
});

it('2.4.1.2 Negative - negative number', function (done) {
it('2.11.1.2 Negative - negative number', function (done) {
setAsGlobalOption(-1, done);
});

it('2.4.1.3 Negative - positive number', function (done) {
it('2.11.1.3 Negative - positive number', function (done) {
setAsGlobalOption(-1, done);
});

it('2.4.1.4 Negative - NaN', function (done) {
it('2.11.1.4 Negative - NaN', function (done) {
setAsGlobalOption(NaN, done);
});

it('2.4.1.5 Negative - undefined', function (done) {
it('2.11.1.5 Negative - undefined', function (done) {
setAsGlobalOption(undefined, done);
});
});
Expand Down

0 comments on commit b1f165a

Please sign in to comment.