Skip to content

Commit d045f4c

Browse files
sharadrajusharadraju
committed
Code refactoring + bug fixes for cursor leaks and SODA lock setting
1 parent 6602843 commit d045f4c

File tree

5 files changed

+146
-208
lines changed

5 files changed

+146
-208
lines changed

‎doc/src/release_notes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ Thin Mode Changes
2525
in embedded quotes and in JSON syntax.
2626
`Issue #1605 <https://github.com/oracle/node-oracledb/issues/1605>`__.
2727

28+
#) Corrected bug that caused cursors to be leaked when calling
29+
Connection.getStatementInfo().
30+
2831
#) Fixed bug that caused an exception to be thrown unnecessarily when a connection was closed.
2932
`Issue #1604 <https://github.com/oracle/node-oracledb/issues/1604>`__.
3033

‎lib/thin/connection.js

Lines changed: 117 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,98 @@ class ThinConnectionImpl extends ConnectionImpl {
159159
}
160160
}
161161

162+
//---------------------------------------------------------------------------
163+
// _execute()
164+
//
165+
// Calls the RPC that executes a SQL statement and returns the results.
166+
//---------------------------------------------------------------------------
167+
async _execute(statement, numIters, binds, options, executeManyFlag) {
168+
169+
// perform binds
170+
const numBinds = statement.bindInfoList.length;
171+
const numVars = binds.length;
172+
if (numBinds !== numVars && (numVars === 0 || !binds[0].name)) {
173+
errors.throwErr(errors.ERR_WRONG_NUMBER_OF_POSITIONAL_BINDS, numBinds, numVars);
174+
}
175+
for (let i = 0; i < binds.length; i++) {
176+
await this._bind(statement, binds[i], i + 1);
177+
}
178+
if (statement.isPlSql && (options.batchErrors || options.dmlRowCounts)) {
179+
errors.throwErr(errors.ERR_EXEC_MODE_ONLY_FOR_DML);
180+
}
181+
182+
// send database request
183+
const message = new messages.ExecuteMessage(this, statement, options);
184+
message.numExecs = numIters;
185+
message.arrayDmlRowCounts = options.dmlRowCounts;
186+
message.batchErrors = options.batchErrors;
187+
if (statement.isPlSql && statement.requiresFullExecute) {
188+
message.numExecs = 1;
189+
await this._protocol._processMessage(message);
190+
statement.requiresFullExecute = false;
191+
message.numExecs = numIters - 1;
192+
message.offset = 1;
193+
}
194+
if (message.numExecs > 0) {
195+
await this._protocol._processMessage(message);
196+
statement.requiresFullExecute = false;
197+
}
198+
199+
// if a define is required, send an additional request to the database
200+
if (statement.requiresDefine && statement.sql) {
201+
statement.requiresFullExecute = true;
202+
await this._protocol._processMessage(message);
203+
statement.requiresFullExecute = false;
204+
statement.requiresDefine = false;
205+
}
206+
207+
// process results
208+
const result = {};
209+
if (statement.numQueryVars > 0) {
210+
result.resultSet = message.resultSet;
211+
} else {
212+
statement.bufferRowIndex = 0;
213+
const outBinds = thinUtil.getOutBinds(statement, numIters,
214+
executeManyFlag);
215+
if (outBinds) {
216+
result.outBinds = outBinds;
217+
}
218+
if (executeManyFlag) {
219+
if (!statement.isPlSql) {
220+
result.rowsAffected = statement.rowCount;
221+
delete statement.rowCount;
222+
}
223+
if (options.dmlRowCounts) {
224+
result.dmlRowCounts = options.dmlRowCounts;
225+
}
226+
if (options.batchErrors) {
227+
result.batchErrors = options.batchErrors;
228+
}
229+
} else {
230+
if (statement.isPlSql && options.implicitResultSet) {
231+
result.implicitResults = options.implicitResultSet;
232+
}
233+
if (statement.lastRowid) {
234+
result.lastRowid = statement.lastRowid;
235+
delete statement.lastRowid;
236+
}
237+
if (statement.isPlSql) {
238+
if (statement.rowCount) {
239+
result.rowsAffected = statement.rowCount;
240+
}
241+
} else {
242+
result.rowsAffected = statement.rowCount || 0;
243+
}
244+
if (statement.rowCount) {
245+
delete statement.rowCount;
246+
}
247+
}
248+
this._returnStatement(statement);
249+
}
250+
251+
return result;
252+
}
253+
162254
//---------------------------------------------------------------------------
163255
// _parseElementType()
164256
//
@@ -778,26 +870,6 @@ class ThinConnectionImpl extends ConnectionImpl {
778870
return resultSet;
779871
}
780872

781-
//---------------------------------------------------------------------------
782-
// Prepares the sql given by user and binds the value to the statement object
783-
//---------------------------------------------------------------------------
784-
async _prepareAndBind(sql, binds, options, isParse = false) {
785-
const stmt = this._prepare(sql, options);
786-
let position = 0;
787-
if (!isParse) {
788-
const numBinds = stmt.bindInfoList.length;
789-
const numVars = binds.length;
790-
if (numBinds !== numVars && (numVars === 0 || !binds[0].name)) {
791-
errors.throwErr(errors.ERR_WRONG_NUMBER_OF_POSITIONAL_BINDS, numBinds, numVars);
792-
}
793-
for (const variable of binds) {
794-
await this._bind(stmt, variable, position + 1);
795-
position += 1;
796-
}
797-
}
798-
return stmt;
799-
}
800-
801873
//---------------------------------------------------------------------------
802874
// Clears the statement cache for the connection
803875
//---------------------------------------------------------------------------
@@ -818,139 +890,46 @@ class ThinConnectionImpl extends ConnectionImpl {
818890
}
819891

820892
//---------------------------------------------------------------------------
821-
// Parses the sql given by User
822-
// calls the OAL8 RPC that parses the SQL statement and returns the metadata
823-
// information for a statment.
893+
// getStatementInfo()
894+
//
895+
// Parses the SQL statement and returns information about the statement.
824896
//---------------------------------------------------------------------------
825897
async getStatementInfo(sql) {
826898
const options = {};
827899
const result = {};
828-
const statement = await this._prepareAndBind(sql, null, options, true);
900+
const statement = this._prepare(sql, options);
829901
options.connection = this;
830-
// parse the statement (but not for DDL which doesn't support it)
831-
if (!statement.isDdl) {
832-
const message = new messages.ExecuteMessage(this, statement, options);
833-
message.parseOnly = true;
834-
await this._protocol._processMessage(message);
835-
}
836-
if (statement.numQueryVars > 0) {
837-
result.metaData = thinUtil.getMetadataMany(statement.queryVars);
838-
}
839-
result.bindNames = Array.from(statement.bindInfoDict.keys());
840-
result.statementType = statement.statementType;
841-
return result;
842-
}
843-
844-
//---------------------------------------------------------------------------
845-
// Prepares the sql given by the user,
846-
// calls the OAL8 RPC that executes a SQL statement and returns the results.
847-
//---------------------------------------------------------------------------
848-
async execute(sql, numIters, binds, options, executeManyFlag) {
849-
const result = {};
850-
if (executeManyFlag) {
851-
return await this.executeMany(sql, numIters, binds, options);
852-
}
853-
const statement = await this._prepareAndBind(sql, binds, options);
854-
855-
// send the initial request to the database
856-
const message = new messages.ExecuteMessage(this, statement, options);
857-
message.numExecs = 1;
858-
await this._protocol._processMessage(message);
859-
statement.requiresFullExecute = false;
860-
861-
// if a define is required, send an additional request to the database
862-
if (statement.requiresDefine && statement.sql) {
863-
statement.requiresFullExecute = true;
864-
await this._protocol._processMessage(message);
865-
statement.requiresFullExecute = false;
866-
statement.requiresDefine = false;
867-
}
868-
869-
// process message results
870-
if (statement.numQueryVars > 0) {
871-
result.resultSet = message.resultSet;
872-
} else {
873-
statement.bufferRowIndex = 0;
874-
const bindVars = thinUtil.getBindVars(statement);
875-
const outBinds = thinUtil.getExecuteOutBinds(bindVars);
876-
if (outBinds) {
877-
result.outBinds = outBinds;
878-
}
879-
if (statement.isPlSql) {
880-
if (options.implicitResultSet) {
881-
result.implicitResults = options.implicitResultSet;
882-
}
883-
}
884-
if (statement.lastRowid) {
885-
result.lastRowid = statement.lastRowid;
886-
delete statement.lastRowid;
887-
}
888-
if (statement.isPlSql) {
889-
if (statement.rowCount) {
890-
result.rowsAffected = statement.rowCount;
891-
}
892-
} else {
893-
result.rowsAffected = statement.rowCount || 0;
902+
try {
903+
if (!statement.isDdl) {
904+
const message = new messages.ExecuteMessage(this, statement, options);
905+
message.parseOnly = true;
906+
await this._protocol._processMessage(message);
894907
}
895-
if (statement.rowCount) {
896-
delete statement.rowCount;
908+
if (statement.numQueryVars > 0) {
909+
result.metaData = thinUtil.getMetadataMany(statement.queryVars);
897910
}
911+
result.bindNames = Array.from(statement.bindInfoDict.keys());
912+
result.statementType = statement.statementType;
913+
return result;
914+
} finally {
898915
this._returnStatement(statement);
899916
}
900-
901-
return result;
902917
}
903918

904919
//---------------------------------------------------------------------------
905-
// executeMany()
920+
// execute()
906921
//
907-
// Prepares the sql given by the user, calls the OAL8 RPC that executes a SQL
908-
// statement multiple times and returns the results.
922+
// Calls the RPC that executes a SQL statement and returns the results.
909923
//---------------------------------------------------------------------------
910-
async executeMany(sql, numIters, binds, options) {
911-
const statement = await this._prepareAndBind(sql, binds, options);
912-
if (statement.isPlSql && (options.batchErrors || options.dmlRowCounts)) {
913-
errors.throwErr(errors.ERR_EXEC_MODE_ONLY_FOR_DML);
914-
}
915-
916-
// send database request
917-
const message = new messages.ExecuteMessage(this, statement, options);
918-
message.numExecs = numIters;
919-
message.arrayDmlRowCounts = options.dmlRowCounts;
920-
message.batchErrors = options.batchErrors;
921-
if (statement.isPlSql && statement.requiresFullExecute) {
922-
message.numExecs = 1;
923-
await this._protocol._processMessage(message);
924-
statement.requiresFullExecute = false;
925-
message.numExecs = numIters - 1;
926-
message.offset = 1;
927-
}
928-
if (message.numExecs > 0) {
929-
await this._protocol._processMessage(message);
930-
}
931-
932-
// process results
933-
const returnObj = {};
934-
statement.bufferRowIndex = 0;
935-
const bindVars = thinUtil.getBindVars(statement);
936-
const outBinds = thinUtil.getExecuteManyOutBinds(bindVars, numIters);
937-
if (outBinds) {
938-
returnObj.outBinds = outBinds;
939-
}
940-
const rowsAffected = !statement.isPlSql ? statement.rowCount : undefined;
941-
if (rowsAffected) {
942-
returnObj.rowsAffected = rowsAffected;
943-
delete statement.rowCount;
944-
}
945-
if (options.dmlRowCounts) {
946-
returnObj.dmlRowCounts = options.dmlRowCounts;
947-
}
948-
if (options.batchErrors) {
949-
returnObj.batchErrors = options.batchErrors;
924+
async execute(sql, numIters, binds, options, executeManyFlag) {
925+
const statement = this._prepare(sql, options);
926+
try {
927+
return await this._execute(statement, numIters, binds, options,
928+
executeManyFlag);
929+
} catch (err) {
930+
this._returnStatement(statement);
931+
throw err;
950932
}
951-
this._returnStatement(statement);
952-
953-
return returnObj;
954933
}
955934

956935
//---------------------------------------------------------------------------

‎lib/thin/protocol/messages/withData.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ class MessageWithData extends Message {
116116
this.errorOccurred = false;
117117
this.statement.moreRowsToFetch = false;
118118
} else if (this.errorInfo.num !== 0 && this.errorInfo.cursorId !== 0) {
119-
this.connection._addCursorToClose(this.statement.cursorId);
120119
this.connection.statementCache.delete(this.statement.sql);
120+
this.statement.returnToCache = false;
121121
}
122122
if (this.errorInfo.batchErrors) {
123123
this.errorOccurred = false;

0 commit comments

Comments
 (0)