Skip to content

Commit cbd914a

Browse files
sharadrajusharadraju
committed
Fix code to handle edge cases for pool and statement handling
1 parent af85a36 commit cbd914a

File tree

9 files changed

+74
-97
lines changed

9 files changed

+74
-97
lines changed

‎lib/errors.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ const ERR_DELETE_ELEMENTS_OF_VARRAY = 133;
135135
const ERR_WRONG_VALUE_FOR_DBOBJECT_ATTR = 134;
136136
const ERR_WRONG_VALUE_FOR_DBOBJECT_ELEM = 135;
137137
const ERR_WRONG_CRED_FOR_EXTAUTH = 136;
138+
const ERR_MISSING_BIND_VALUE = 137;
138139

139140
// Oracle Net layer errors start from 500
140141
const ERR_CONNECTION_CLOSED = 500;
@@ -390,6 +391,8 @@ messages.set(ERR_WRONG_VALUE_FOR_DBOBJECT_ELEM, // NJS-135
390391
'value is of wrong type for an element of object %s');
391392
messages.set(ERR_WRONG_CRED_FOR_EXTAUTH, // NJS-136
392393
'user name and password cannot be set when using external authentication');
394+
messages.set(ERR_MISSING_BIND_VALUE, // NJS-137
395+
'a bind variable replacement value for placeholder ":%s" was not provided');
393396

394397
// Oracle Net layer errors
395398

@@ -759,6 +762,7 @@ module.exports = {
759762
ERR_WRONG_VALUE_FOR_DBOBJECT_ATTR,
760763
ERR_WRONG_VALUE_FOR_DBOBJECT_ELEM,
761764
ERR_WRONG_CRED_FOR_EXTAUTH,
765+
ERR_MISSING_BIND_VALUE,
762766
ERR_CONNECTION_CLOSED_CODE: `${ERR_PREFIX}-${ERR_CONNECTION_CLOSED}`,
763767
assert,
764768
assertArgCount,

‎lib/oracledb.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -373,10 +373,10 @@ async function _verifyOptions(options, inCreatePool) {
373373
outOptions.queueTimeout = options.queueTimeout;
374374
}
375375

376-
// queueMax must be an integer >= -1
376+
// queueMax must be an integer
377377
if (options.queueMax !== undefined) {
378-
errors.assertParamPropValue(Number.isInteger(options.queueMax) &&
379-
options.queueMax >= -1, 1, "queueMax");
378+
errors.assertParamPropValue(Number.isInteger(options.queueMax), 1,
379+
"queueMax");
380380
outOptions.queueMax = options.queueMax;
381381
}
382382

‎lib/thin/connection.js‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -683,10 +683,10 @@ class ThinConnectionImpl extends ConnectionImpl {
683683
if (normalizedName.startsWith(':')) {
684684
normalizedName = variable.name.substring(1);
685685
}
686-
if (bindInfoDict[normalizedName] === undefined) {
686+
if (!bindInfoDict.has(normalizedName)) {
687687
errors.throwErr(errors.ERR_INVALID_BIND_NAME, normalizedName);
688688
}
689-
bindInfoDict[normalizedName].forEach((bindInfo) => {
689+
bindInfoDict.get(normalizedName).forEach((bindInfo) => {
690690
stmt._setVariable(bindInfo, variable);
691691
});
692692
} else {
@@ -722,7 +722,7 @@ class ThinConnectionImpl extends ConnectionImpl {
722722
if (!isParse) {
723723
const numBinds = stmt.bindInfoList.length;
724724
const numVars = binds.length;
725-
if (numBinds !== numVars) {
725+
if (numBinds !== numVars && (numVars === 0 || !binds[0].name)) {
726726
errors.throwErr(errors.ERR_WRONG_NUMBER_OF_POSITIONAL_BINDS, numBinds, numVars);
727727
}
728728
for (const variable of binds) {
@@ -771,7 +771,7 @@ class ThinConnectionImpl extends ConnectionImpl {
771771
if (statement.numQueryVars > 0) {
772772
result.metaData = thinUtil.getMetadataMany(statement.queryVars);
773773
}
774-
result.bindNames = Object.keys(statement.bindInfoDict);
774+
result.bindNames = Array.from(statement.bindInfoDict.keys());
775775
result.statementType = statement.statementType;
776776
return result;
777777
}
@@ -806,8 +806,8 @@ class ThinConnectionImpl extends ConnectionImpl {
806806
result.resultSet = message.resultSet;
807807
} else {
808808
statement.bufferRowIndex = 0;
809-
let bindVars = thinUtil.getBindVars(statement);
810-
let outBinds = thinUtil.getExecuteOutBinds(bindVars);
809+
const bindVars = thinUtil.getBindVars(statement);
810+
const outBinds = thinUtil.getExecuteOutBinds(bindVars);
811811
if (outBinds) {
812812
result.outBinds = outBinds;
813813
}
@@ -856,16 +856,11 @@ class ThinConnectionImpl extends ConnectionImpl {
856856
if (statement.isPlSql && statement.requiresFullExecute) {
857857
message.numExecs = 1;
858858
await this._protocol._processMessage(message);
859-
if (statement.plsqlMultipleExecs) {
860-
for (let i = 0; i < numIters - 1; i++) {
861-
message.offset = i + 1;
862-
await this._protocol._processMessage(message);
863-
}
864-
} else {
865-
message.offset = 1;
866-
message.numExecs = numIters - 1;
867-
}
868-
} else {
859+
statement.requiresFullExecute = false;
860+
message.numExecs = numIters - 1;
861+
message.offset = 1;
862+
}
863+
if (message.numExecs > 0) {
869864
await this._protocol._processMessage(message);
870865
}
871866

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,7 @@ class ExecuteMessage extends MessageWithData {
213213
if (stmt.requiresDefine) {
214214
this.writeColumnMetadata(buf, this.statement.queryVars);
215215
} else if (numParams > 0) {
216-
const returningOnly = this.processBindParams(buf, params);
217-
if (!returningOnly)
218-
return params;
216+
return this.processBindParams(buf, params);
219217
}
220218
}
221219

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

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ class MessageWithData extends Message {
302302
variable.values[this.rowIndex] = value;
303303
} else {
304304
value = this.processColumnData(buf, variable, this.rowIndex);
305-
variable.values[this.rowIndex + this.offset] = value;
305+
variable.values[this.rowIndex] = value;
306306
}
307307
}
308308
this.rowIndex++;
@@ -337,10 +337,6 @@ class MessageWithData extends Message {
337337
}
338338
this.outVariables.push(bindInfo.bindVar);
339339
}
340-
341-
if (this.statement.isPlSql && this.outVariables.length > 0) {
342-
this.statement.plsqlMultipleExecs = true;
343-
}
344340
}
345341

346342
processColumnData(buf, variable) {
@@ -588,34 +584,16 @@ class MessageWithData extends Message {
588584
}
589585

590586
processBindParams(buf, params) {
591-
let returningOnly = true;
592-
let allValuesNull = true;
593-
let bindVars = [];
594-
let bindVals = [];
587+
const bindVars = [];
588+
const nonReturningParams = [];
595589
for (const bindInfo of params) {
596590
if (!bindInfo.isReturnBind) {
597-
returningOnly = false;
598-
}
599-
bindVals = bindInfo.bindVar.values;
600-
if (allValuesNull) {
601-
for (let i = 0; i < bindVals.length; i++) {
602-
if (bindVals[i] !== null) {
603-
allValuesNull = false;
604-
break;
605-
}
606-
}
591+
nonReturningParams.push(bindInfo);
607592
}
608593
bindVars.push(bindInfo.bindVar);
609594
}
610595
this.writeColumnMetadata(buf, bindVars);
611-
612-
// plsql batch executions without bind values
613-
if (this.statement.isPlsql && this.numExecs > 1 && !allValuesNull) {
614-
buf.writeUInt8(constants.TNS_MSG_TYPE_ROW_DATA);
615-
buf.writeUInt8(constants.TNS_ESCAPE_CHAR);
616-
buf.writeUInt8(1);
617-
}
618-
return returningOnly;
596+
return nonReturningParams;
619597
}
620598

621599
writeColumnMetadata(buf, bindVars) {

‎lib/thin/statement.js‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,10 @@ class Statement {
127127
copiedStatement.bindInfoDict = new Map();
128128
bindInfoDict = copiedStatement.bindInfoDict;
129129
for (let bindInfoObj of this.bindInfoList) {
130-
if (copiedStatement.bindInfoDict.has(bindInfoObj.bindName)) {
131-
bindInfoDict[bindInfoObj.bindName].push(bindInfoObj);
130+
if (bindInfoDict.has(bindInfoObj.bindName)) {
131+
bindInfoDict.get(bindInfoObj.bindName).push(bindInfoObj);
132132
} else {
133-
bindInfoDict[bindInfoObj.bindName] = [bindInfoObj];
133+
bindInfoDict.set(bindInfoObj.bindName, [bindInfoObj]);
134134
}
135135
}
136136
copiedStatement.returnToCache = false;
@@ -213,7 +213,7 @@ class Statement {
213213
let returningSql;
214214
if (this.isQuery || this.isDml || this.isPlSql) {
215215
let inputSql = sql;
216-
if (!this.isPlSql) {
216+
if (this.isDml) {
217217
/*
218218
* get starting index after DML_RETURNING_KEY from begining of sql and starting index
219219
* after DML_RETURNING_INTO_KEY from the end of sql
@@ -260,10 +260,12 @@ class Statement {
260260
return;
261261
}
262262
let info = new BindInfo(name, isReturnBind);
263+
this.bindInfoList.push(info);
263264

264-
if (!this.bindInfoDict.hasOwnProperty(info.bindName)) { // eslint-disable-line
265-
this.bindInfoDict[info.bindName] = [info];
266-
this.bindInfoList.push(info);
265+
if (this.bindInfoDict.has(info.bindName)) {
266+
this.bindInfoDict.get(info.bindName).push(info);
267+
} else {
268+
this.bindInfoDict.set(info.bindName, [info]);
267269
}
268270
});
269271
}

‎test/dbType01.js‎

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -58,57 +58,59 @@ describe('226. dbType01.js', function() {
5858
const strInVal = "Node-oracledb";
5959
const dateInVal = new Date();
6060
const numInVal = 12;
61-
const SQL = `SELECT :1, DUMP(:1) FROM dual`;
61+
const SQL = `SELECT :BIND1, DUMP(:BIND1) FROM dual`;
6262

6363
it('226.1 DB_TYPE_VARCHAR', async () => {
64+
const bindVal = { BIND1: {val: strInVal, type: oracledb.DB_TYPE_VARCHAR }};
6465
const result = await conn.execute(SQL,
65-
[{ val: strInVal, type: oracledb.DB_TYPE_VARCHAR }]);
66+
bindVal);
6667
assert.strictEqual(strInVal, result.rows[0][0]);
6768
assert.match(result.rows[0][1], /Typ=1 Len=13/);
6869
}); // 226.1
6970

7071
it('226.2 DB_TYPE_CHAR', async () => {
72+
const bindVal = { BIND1: {val: strInVal, type: oracledb.DB_TYPE_CHAR }};
7173
const result = await conn.execute(SQL,
72-
[{ val: strInVal, type: oracledb.DB_TYPE_CHAR }]);
74+
bindVal);
7375
assert.strictEqual(strInVal, result.rows[0][0]);
7476
assert.match(result.rows[0][1], /Typ=96 Len=13/);
7577
}); // 226.2
7678

7779
it('226.3 DB_TYPE_NVARCHAR', async () => {
78-
const result = await conn.execute(SQL,
79-
[{ val: strInVal, type: oracledb.DB_TYPE_NVARCHAR }]);
80+
const bindVal = { BIND1: {val: strInVal, type: oracledb.DB_TYPE_NVARCHAR }};
81+
const result = await conn.execute(SQL, bindVal);
8082
assert.strictEqual(strInVal, result.rows[0][0]);
8183
assert.match(result.rows[0][1], /Typ=1 Len=26/);
8284
}); // 226.3
8385

8486
it('226.4 DB_TYPE_NCHAR', async () => {
85-
const result = await conn.execute(SQL,
86-
[{ val: strInVal, type: oracledb.DB_TYPE_NCHAR }]);
87+
const bindVal = { BIND1: {val: strInVal, type: oracledb.DB_TYPE_NCHAR }};
88+
const result = await conn.execute(SQL, bindVal);
8789
assert.strictEqual(strInVal, result.rows[0][0]);
8890
assert.match(result.rows[0][1], /Typ=96 Len=26/);
8991
}); // 226.4
9092

9193
it('226.5 DB_TYPE_DATE', async () => {
92-
const result = await conn.execute(SQL,
93-
[{ val: dateInVal, type: oracledb.DB_TYPE_DATE }]);
94+
const bindVal = { BIND1: {val: dateInVal, type: oracledb.DB_TYPE_DATE }};
95+
const result = await conn.execute(SQL, bindVal);
9496
assert.match(result.rows[0][1], /Typ=12 Len=7/);
9597
}); // 226.5
9698

9799
it('226.6 DB_TYPE_TIMESTAMP_LTZ', async () => {
98-
const result = await conn.execute(SQL,
99-
[{ val: dateInVal, type: oracledb.DB_TYPE_TIMESTAMP_LTZ }]);
100+
const bindVal = { BIND1: {val: dateInVal, type: oracledb.DB_TYPE_TIMESTAMP_LTZ}};
101+
const result = await conn.execute(SQL, bindVal);
100102
assert.match(result.rows[0][1], /Typ=231 Len=11/);
101103
}); // 226.6
102104

103105
it('226.7 DB_TYPE_TIMESTAMP', async () => {
104-
const result = await conn.execute(SQL,
105-
[{ val: dateInVal, type: oracledb.DB_TYPE_TIMESTAMP }]);
106+
const bindVal = { BIND1: {val: dateInVal, type: oracledb.DB_TYPE_TIMESTAMP}};
107+
const result = await conn.execute(SQL, bindVal);
106108
assert.match(result.rows[0][1], /Typ=180 Len=11/);
107109
});
108110

109111
it('226.8 DB_TYPE_TIMESTAMP_TZ', async () => {
110-
const result = await conn.execute(SQL,
111-
[{ val: dateInVal, type: oracledb.DB_TYPE_TIMESTAMP_TZ }]);
112+
const bindVal = { BIND1: {val: dateInVal, type: oracledb.DB_TYPE_TIMESTAMP_TZ}};
113+
const result = await conn.execute(SQL, bindVal);
112114
assert.match(result.rows[0][1], /Typ=181 Len=13/);
113115
});
114116

@@ -120,28 +122,38 @@ describe('226. dbType01.js', function() {
120122
});
121123

122124
it('226.10 DB_TYPE_BINARY_FLOAT', async () => {
123-
const result = await conn.execute(SQL,
124-
[{ val: numInVal, type: oracledb.DB_TYPE_BINARY_FLOAT }]);
125+
const bindVal = { BIND1: {val: numInVal, type: oracledb.DB_TYPE_BINARY_FLOAT}};
126+
const result = await conn.execute(SQL, bindVal);
125127
assert.match(result.rows[0][1], /Typ=100 Len=4/);
126128
});
127129

128130
it('226.11 DB_TYPE_BINARY_DOUBLE', async () => {
129-
const result = await conn.execute(SQL,
130-
[{ val: numInVal, type: oracledb.DB_TYPE_BINARY_DOUBLE }]);
131+
const bindVal = { BIND1: {val: numInVal, type: oracledb.DB_TYPE_BINARY_DOUBLE}};
132+
const result = await conn.execute(SQL, bindVal);
131133
assert.match(result.rows[0][1], /Typ=101 Len=8/);
132134
});
133135

134136
it('226.12 Infinity, DB_TYPE_BINARY_FLOAT', async () => {
135137
const num = 1 / 0;
136-
const result = await conn.execute(SQL,
137-
[{ val: num, type: oracledb.DB_TYPE_BINARY_FLOAT }]);
138+
const bindVal = { BIND1: {val: num, type: oracledb.DB_TYPE_BINARY_FLOAT}};
139+
const result = await conn.execute(SQL, bindVal);
138140
assert.match(result.rows[0][1], /Typ=100 Len=4/);
139141
});
140142

141143
it('226.13 Infinity, DB_TYPE_BINARY_DOUBLE', async () => {
142144
const num = 1 / 0;
143-
const result = await conn.execute(SQL,
144-
[{ val: num, type: oracledb.DB_TYPE_BINARY_DOUBLE }]);
145+
const bindVal = { BIND1: {val: num, type: oracledb.DB_TYPE_BINARY_DOUBLE}};
146+
const result = await conn.execute(SQL, bindVal);
145147
assert.match(result.rows[0][1], /Typ=101 Len=8/);
146148
});
149+
150+
it('226.14 DB_TYPE_VARCHAR Repeating using bindByPos', async () => {
151+
const SQL = `SELECT :1, DUMP(:1) FROM dual`;
152+
const bindVal = {val: strInVal, type: oracledb.DB_TYPE_VARCHAR};
153+
const result = await conn.execute(SQL,
154+
[bindVal, bindVal]);
155+
assert.strictEqual(strInVal, result.rows[0][0]);
156+
assert.match(result.rows[0][1], /Typ=1 Len=13/);
157+
}); // 226.14
158+
147159
});

‎test/list.txt‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,11 @@ Overview of node-oracledb functional tests
6565
2.8 connection queueing
6666
2.8.1 basic case
6767
2.8.2 generates NJS-040 if request is queued and queueTimeout expires
68-
2.8.3 does not generate NJS-040 if request is queued for less time than queueTimeout
69-
2.8.4 generates NJS-076 if request exceeds queueMax
70-
2.8.5 generates NJS-076 if request exceeds queueMax 0
71-
2.8.6 request queue never terminate for queueTimeout set to 0
68+
2.8.3 generates NJS-076 if request exceeds queueMax
69+
2.8.4 generates NJS-076 if request exceeds queueMax 0
70+
2.8.5 request queue never terminate for queueTimeout set to 0
71+
2.8.6 queueMax range check, queueMax -1
72+
2.8.7 queueMax range check, queueMax -0.5 not an integer
7273
2.9 _enableStats & _logStats functionality
7374
2.9.1 does not work after the pool has been terminated
7475
2.10 Close method

‎test/pool.js‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -515,20 +515,7 @@ describe('2. pool.js', function() {
515515
await pool.close(0);
516516
});
517517

518-
it('2.8.7 queueMax range check, queueMax -2 out of range', async function() {
519-
const config = {...dbConfig,
520-
poolMin : 1,
521-
poolMax : 1,
522-
poolIncrement : 0,
523-
queueMax : -2
524-
};
525-
await assert.rejects(
526-
async () => await oracledb.createPool(config),
527-
/NJS-007:/
528-
);
529-
});
530-
531-
it('2.8.8 queueMax range check, queueMax -0.5 not an integer', async function() {
518+
it('2.8.7 queueMax range check, queueMax -0.5 not an integer', async function() {
532519
const config = {...dbConfig,
533520
poolMin : 1,
534521
poolMax : 1,

0 commit comments

Comments
 (0)