Skip to content

Commit

Permalink
Fixed CORE-4286: "Statement already has a cursor assigned" error when…
Browse files Browse the repository at this point in the history 
… trying to execute another SQL statement using different cursor name. Moved method setCursorName() from IStatement to IResultSet - it was my fault when splitting cursor from statement.
  • Loading branch information
@AlexPeshkoff
AlexPeshkoff committed Jan 17, 2014
1 parent cad838b commit 411c371
Show file tree
Hide file tree
Showing 8 changed files with 109 additions and 43 deletions.
3 changes: 1 addition & 2 deletions src/dsql/dsql.cpp
View file
Expand Up @@ -411,7 +411,7 @@ void DsqlDmlRequest::setCursor(thread_db* tdbb, const TEXT* name)
const size_t MAX_CURSOR_LENGTH = 132 - 1;
string cursor = name;

if (cursor[0] == '\"')
if (cursor.hasData() && cursor[0] == '\"')
{
// Quoted cursor names eh? Strip'em.
// Note that "" will be replaced with ".
Expand Down Expand Up @@ -469,7 +469,6 @@ void DsqlDmlRequest::setCursor(thread_db* tdbb, const TEXT* name)
}
else
{
fb_assert(this != *symbol);
ERRD_post(Arg::Gds(isc_sqlerr) << Arg::Num(-502) <<
Arg::Gds(isc_dsql_decl_err) <<
Arg::Gds(isc_dsql_cursor_redefined) << req_cursor);
Expand Down
6 changes: 3 additions & 3 deletions src/include/firebird/Provider.h
View file
Expand Up @@ -141,9 +141,10 @@ class IResultSet : public IRefCounted
virtual FB_BOOLEAN FB_CARG isEof(IStatus* status) = 0;
virtual FB_BOOLEAN FB_CARG isBof(IStatus* status) = 0;
virtual IMessageMetadata* FB_CARG getMetadata(IStatus* status) = 0;
virtual void FB_CARG setCursorName(IStatus* status, const char* name) = 0;
virtual void FB_CARG close(IStatus* status) = 0;
};
#define FB_RESULTSET_VERSION (FB_REFCOUNTED_VERSION + 10)
#define FB_RESULTSET_VERSION (FB_REFCOUNTED_VERSION + 11)

class IStatement : public IRefCounted
{
Expand Down Expand Up @@ -180,11 +181,10 @@ class IStatement : public IRefCounted
IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata, void* outBuffer) = 0;
virtual IResultSet* FB_CARG openCursor(IStatus* status, ITransaction* transaction,
IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outMetadata) = 0;
virtual void FB_CARG setCursorName(IStatus* status, const char* name) = 0;
virtual void FB_CARG free(IStatus* status) = 0;
virtual unsigned FB_CARG getFlags(IStatus* status) = 0;
};
#define FB_STATEMENT_VERSION (FB_REFCOUNTED_VERSION + 11)
#define FB_STATEMENT_VERSION (FB_REFCOUNTED_VERSION + 10)

class IRequest : public IRefCounted
{
Expand Down
2 changes: 1 addition & 1 deletion src/jrd/EngineInterface.h
View file
Expand Up @@ -154,6 +154,7 @@ class JResultSet FB_FINAL : public Firebird::RefCntIface<Firebird::IResultSet, F
virtual FB_BOOLEAN FB_CARG isEof(Firebird::IStatus* status);
virtual FB_BOOLEAN FB_CARG isBof(Firebird::IStatus* status);
virtual Firebird::IMessageMetadata* FB_CARG getMetadata(Firebird::IStatus* status);
virtual void FB_CARG setCursorName(Firebird::IStatus* status, const char* name);
virtual void FB_CARG close(Firebird::IStatus* status);

public:
Expand Down Expand Up @@ -199,7 +200,6 @@ class JStatement FB_FINAL : public Firebird::RefCntIface<Firebird::IStatement, F
virtual JResultSet* FB_CARG openCursor(Firebird::IStatus* status,
Firebird::ITransaction* transaction, Firebird::IMessageMetadata* inMetadata, void* inBuffer,
Firebird::IMessageMetadata* outMetadata);
virtual void FB_CARG setCursorName(Firebird::IStatus* status, const char* name);
virtual unsigned FB_CARG getFlags(Firebird::IStatus* status);

public:
Expand Down
6 changes: 4 additions & 2 deletions src/jrd/jrd.cpp
View file
Expand Up @@ -5001,7 +5001,7 @@ ISC_UINT64 JStatement::getAffectedRecords(IStatus* userStatus)
}


void JStatement::setCursorName(IStatus* user_status, const char* cursor)
void JResultSet::setCursorName(IStatus* user_status, const char* cursor)
{
try
{
Expand All @@ -5010,7 +5010,9 @@ void JStatement::setCursorName(IStatus* user_status, const char* cursor)

try
{
getHandle()->setCursor(tdbb, cursor);
dsql_req* req = getStatement()->getHandle();
fb_assert(req);
req->setCursor(tdbb, cursor);
}
catch (const Exception& ex)
{
Expand Down
13 changes: 10 additions & 3 deletions src/remote/client/interface.cpp
View file
Expand Up @@ -254,6 +254,7 @@ class ResultSet FB_FINAL : public Firebird::RefCntIface<Firebird::IResultSet, FB
virtual FB_BOOLEAN FB_CARG isEof(IStatus* status);
virtual FB_BOOLEAN FB_CARG isBof(IStatus* status);
virtual IMessageMetadata* FB_CARG getMetadata(IStatus* status);
virtual void FB_CARG setCursorName(IStatus* status, const char* name);
virtual void FB_CARG close(IStatus* status);

ResultSet(Statement* s, IMessageMetadata* outFmt)
Expand Down Expand Up @@ -304,7 +305,6 @@ class Statement FB_FINAL : public Firebird::RefCntIface<Firebird::IStatement, FB
IMessageMetadata* outMetadata, void* outBuffer);
virtual ResultSet* FB_CARG openCursor(IStatus* status, ITransaction* tra,
IMessageMetadata* inMetadata, void* inBuffer, IMessageMetadata* outFormat);
virtual void FB_CARG setCursorName(IStatus* status, const char* name);
virtual void FB_CARG free(IStatus* status);
virtual unsigned FB_CARG getFlags(IStatus* status);

Expand Down Expand Up @@ -3065,7 +3065,7 @@ FB_BOOLEAN ResultSet::fetchRelative(IStatus* user_status, int offset, void* buff
}


void Statement::setCursorName(IStatus* status, const char* cursor)
void ResultSet::setCursorName(IStatus* status, const char* cursor)
{
/*****************************************
*
Expand Down Expand Up @@ -3096,9 +3096,16 @@ void Statement::setCursorName(IStatus* status, const char* cursor)

// Check and validate handles, etc.

Rsr* statement = getStatement();
if (!stmt)
{
(Arg::Gds(isc_dsql_cursor_err) << Arg::Gds(isc_bad_req_handle)).raise();
}
Rsr* statement = stmt->getStatement();
CHECK_HANDLE(statement, isc_bad_req_handle);

Rdb* rdb = statement->rsr_rdb;
CHECK_HANDLE(rdb, isc_bad_db_handle);

rem_port* port = rdb->rdb_port;
RefMutexGuard portGuard(*port->port_sync, FB_FUNCTION);

Expand Down
52 changes: 39 additions & 13 deletions src/remote/server/server.cpp
View file
Expand Up @@ -2898,6 +2898,7 @@ ISC_STATUS rem_port::end_statement(P_SQLFREE* free_stmt, PACKET* sendL)
return this->send_response(sendL, 0, 0, &status_vector, true);
}
statement->rsr_cursor = NULL;
statement->rsr_cursor_name = "";
fb_assert(statement->rsr_rtr);
size_t pos;
if (!statement->rsr_rtr->rtr_cursors.find(statement, pos))
Expand Down Expand Up @@ -3230,7 +3231,19 @@ ISC_STATUS rem_port::execute_statement(P_OP op, P_SQLDATA* sqldata, PACKET* send
iMsgBuffer.metadata,
iMsgBuffer.buffer,
oMsgBuffer.metadata);
transaction->rtr_cursors.add(statement);
if (status_vector.isSuccess())
{
transaction->rtr_cursors.add(statement);

if (statement->rsr_cursor_name.hasData())
{
statement->rsr_cursor->setCursorName(&status_vector, statement->rsr_cursor_name.c_str());
if (status_vector.isSuccess())
{
statement->rsr_cursor_name = "";
}
}
}
}
else // delay execution till first fetch (with output format)
{
Expand Down Expand Up @@ -3350,13 +3363,22 @@ ISC_STATUS rem_port::fetch(P_SQLDATA * sqldata, PACKET* sendL)
statement->rsr_par_metadata,
statement->rsr_parameters.begin(),
msgBuffer.metadata);
statement->rsr_rtr->rtr_cursors.add(statement);

if (!status_vector.isSuccess())
{
status_exception::raise(status_vector.get());
}

statement->rsr_rtr->rtr_cursors.add(statement);

if (status_vector.isSuccess() && statement->rsr_cursor_name.hasData())
{
statement->rsr_cursor->setCursorName(&status_vector, statement->rsr_cursor_name.c_str());
if (status_vector.isSuccess())
{
statement->rsr_cursor_name = "";
}
}

statement->rsr_par_metadata = NULL;
statement->rsr_parameters.clear();
}
Expand Down Expand Up @@ -4053,6 +4075,7 @@ ISC_STATUS rem_port::prepare_statement(P_SQLST * prepareL, PACKET* sendL)
if (!status_vector.isSuccess())
return this->send_response(sendL, 0, 0, &status_vector, false);
}
statement->rsr_cursor_name = "";

Rdb* rdb = statement->rsr_rdb;
if (bad_db(&status_vector, rdb))
Expand All @@ -4067,13 +4090,6 @@ ISC_STATUS rem_port::prepare_statement(P_SQLST * prepareL, PACKET* sendL)
if (!status_vector.isSuccess())
return this->send_response(sendL, 0, 0, &status_vector, false);

if (statement->rsr_cursor_name.hasData())
{
statement->rsr_iface->setCursorName(&status_vector, statement->rsr_cursor_name.c_str());
if (!status_vector.isSuccess())
return this->send_response(sendL, 0, 0, &status_vector, false);
}

LocalStatus s2;
statement->rsr_iface->getInfo(&s2, infoLength, info, prepareL->p_sqlst_buffer_length, buffer);
if (!s2.isSuccess())
Expand Down Expand Up @@ -5021,6 +5037,7 @@ static void release_transaction( Rtr* transaction)
Rsr* const statement = transaction->rtr_cursors.pop();
fb_assert(statement->rsr_cursor);
statement->rsr_cursor = NULL;
statement->rsr_cursor_name = "";
}

for (Rtr** p = &rdb->rdb_transactions; *p; p = &(*p)->rtr_next)
Expand Down Expand Up @@ -5426,11 +5443,20 @@ ISC_STATUS rem_port::set_cursor(P_SQLCUR * sqlcur, PACKET* sendL)

getHandle(statement, sqlcur->p_sqlcur_statement);

statement->rsr_cursor_name = name;

if (statement->rsr_iface)
if (statement->rsr_cursor)
{
statement->rsr_cursor->setCursorName(&status_vector, name);
}
else
{
statement->rsr_iface->setCursorName(&status_vector, name);
if (statement->rsr_cursor_name.hasData() && statement->rsr_cursor_name != name)
{
status_vector.set((Arg::Gds(isc_dsql_decl_err) <<
Arg::Gds(isc_dsql_cursor_redefined) << statement->rsr_cursor_name).value());
}
else
statement->rsr_cursor_name = name;
}

return this->send_response(sendL, 0, 0, &status_vector, false);
Expand Down
2 changes: 1 addition & 1 deletion src/yvalve/YObjects.h
View file
Expand Up @@ -304,6 +304,7 @@ class YResultSet FB_FINAL : public YHelper<YResultSet, Firebird::IResultSet, FB_
virtual FB_BOOLEAN FB_CARG isEof(Firebird::IStatus* status);
virtual FB_BOOLEAN FB_CARG isBof(Firebird::IStatus* status);
virtual Firebird::IMessageMetadata* FB_CARG getMetadata(Firebird::IStatus* status);
virtual void FB_CARG setCursorName(Firebird::IStatus* status, const char* name);
virtual void FB_CARG close(Firebird::IStatus* status);

public:
Expand Down Expand Up @@ -350,7 +351,6 @@ class YStatement FB_FINAL : public YHelper<YStatement, Firebird::IStatement, FB_
Firebird::IMessageMetadata* outMetadata, void* outBuffer);
virtual Firebird::IResultSet* FB_CARG openCursor(Firebird::IStatus* status, Firebird::ITransaction* transaction,
Firebird::IMessageMetadata* inMetadata, void* inBuffer, Firebird::IMessageMetadata* outMetadata);
virtual void FB_CARG setCursorName(Firebird::IStatus* status, const char* name);
virtual void FB_CARG free(Firebird::IStatus* status);
virtual unsigned FB_CARG getFlags(Firebird::IStatus* status);

Expand Down
68 changes: 50 additions & 18 deletions src/yvalve/why.cpp
View file
Expand Up @@ -873,6 +873,7 @@ namespace {
Arg::StatusVector(status->get()).raise();
}
statement = NULL;
cursorName = "";
}
}

Expand Down Expand Up @@ -2063,7 +2064,21 @@ ISC_STATUS API_ROUTINE isc_dsql_execute2_m(ISC_STATUS* userStatus, FB_API_HANDLE
{
statement->statement->openCursor(&status, transaction,
inMsgBuffer.metadata, inMsgBuffer.buffer, outMsgBuffer.metadata);
if (!status.isSuccess())
{
return status[1];
}

fb_assert(statement->statement->cursor);

if (statement->cursorName.hasData())
{
statement->statement->cursor->setCursorName(&status, statement->cursorName.c_str());
if (status.isSuccess())
{
statement->cursorName = "";
}
}
}
else // delay execution till first fetch (with output format)
{
Expand Down Expand Up @@ -2315,14 +2330,22 @@ ISC_STATUS API_ROUTINE isc_dsql_fetch_m(ISC_STATUS* userStatus, FB_API_HANDLE* s

statement->statement->openCursor(&status, statement->transaction,
statement->parMetadata, statement->parameters.begin(), msgBuffer.metadata);
fb_assert(statement->statement->cursor);

if (!status.isSuccess())
return status[1];

fb_assert(statement->statement->cursor);
statement->parMetadata = NULL;
statement->parameters.clear();
statement->transaction = NULL;

if (statement->cursorName.hasData())
{
statement->statement->cursor->setCursorName(&status, statement->cursorName.c_str());
if (status.isSuccess())
{
statement->cursorName = "";
}
}
}

if (!statement->statement->cursor->fetchNext(&status, reinterpret_cast<UCHAR*>(msg)))
Expand Down Expand Up @@ -2366,7 +2389,7 @@ ISC_STATUS API_ROUTINE isc_dsql_free_statement(ISC_STATUS* userStatus, FB_API_HA
else if (option & DSQL_close)
{
// Only close the cursor
statement->closeCursor(&status, true);
statement->closeCursor(&status, !statement->parMetadata);
}
}
catch (const Exception& e)
Expand Down Expand Up @@ -2417,18 +2440,14 @@ ISC_STATUS API_ROUTINE isc_dsql_prepare(ISC_STATUS* userStatus, FB_API_HANDLE* t
return status[1];
}
}
statement->cursorName = "";

if (traHandle && *traHandle)
transaction = translateHandle(transactions, traHandle);

statement->statement = statement->attachment->prepare(&status, transaction, stmtLength,
sqlStmt, dialect, IStatement::PREPARE_PREFETCH_METADATA);

if (status.isSuccess() && statement->cursorName.hasData())
{
statement->statement->setCursorName(&status, statement->cursorName.c_str());
}

if (status.isSuccess())
{
StatusVector tempStatus(NULL);
Expand Down Expand Up @@ -2461,6 +2480,16 @@ ISC_STATUS API_ROUTINE isc_dsql_prepare_m(ISC_STATUS* userStatus, FB_API_HANDLE*
RefPtr<IscStatement> statement(translateHandle(statements, stmtHandle));
RefPtr<YTransaction> transaction;

if (statement->statement)
{
statement->closeStatement(&status);
if (!status.isSuccess())
{
return status[1];
}
}
statement->cursorName = "";

if (traHandle && *traHandle)
transaction = translateHandle(transactions, traHandle);

Expand All @@ -2470,11 +2499,6 @@ ISC_STATUS API_ROUTINE isc_dsql_prepare_m(ISC_STATUS* userStatus, FB_API_HANDLE*
statement->statement = statement->attachment->prepare(&status, transaction, stmtLength,
sqlStmt, dialect, flags);

if (status.isSuccess() && statement->cursorName.hasData())
{
statement->statement->setCursorName(&status, statement->cursorName.c_str());
}

if (status.isSuccess())
{
StatusVector tempStatus(NULL);
Expand Down Expand Up @@ -2502,9 +2526,17 @@ ISC_STATUS API_ROUTINE isc_dsql_set_cursor_name(ISC_STATUS* userStatus, FB_API_H
{
RefPtr<IscStatement> statement(translateHandle(statements, stmtHandle));

statement->cursorName = cursorName;
if (statement->statement)
statement->statement->setCursorName(&status, cursorName);
if (statement->statement && statement->statement->cursor)
statement->statement->cursor->setCursorName(&status, cursorName);
else
{
if (statement->cursorName.hasData() && statement->cursorName != cursorName)
{
(Arg::Gds(isc_dsql_decl_err) <<
Arg::Gds(isc_dsql_cursor_redefined) << statement->cursorName).raise();
}
statement->cursorName = cursorName;
}
}
catch (const Exception& e)
{
Expand Down Expand Up @@ -4121,11 +4153,11 @@ void YResultSet::destroy()
destroy2();
}

void YStatement::setCursorName(IStatus* status, const char* name)
void YResultSet::setCursorName(IStatus* status, const char* name)
{
try
{
YEntry<YStatement> entry(status, this);
YEntry<YResultSet> entry(status, this);

entry.next()->setCursorName(status, name);
}
Expand Down

0 comments on commit 411c371

Please sign in to comment.