Node-oracledb: Only last value is being inserted multiple times in the loop

I am reading a XLSX file and inserting the records to ORACLE database. XLSX contains below values

H
H
H
H
JK

But only JK is being inserted 5 times. Below is the code used

var XLSX = require('xlsx')
var workbook = XLSX.readFile('Accounts.xlsx');
var sheet_name_list = workbook.SheetNames;
var xlData = XLSX.utils.sheet_to_json(workbook.Sheets[sheet_name_list[0]]);
var connection;
var oracledb = require('oracledb');
oracledb.autoCommit = true;
var dbConfig = require(__dirname + '/dbconfig.js');
var cnt;


 oracledb.getConnection(
    dbConfig,
     function(err, connection) {
        if (err) throw err;
		
		for (i in xlData) 
		{
            
			var act_fam = xlData[i].ACCOUNT_FAMILY;
        
            
              
               connection.execute(
                `SELECT * FROM TFAMCORGDS_TEST WHERE MNEFAMCOR='`+ act_fam + `'`,
                function(err, results) {
                    if (err) throw err;
                    cnt = results.rows.length;
                
                    if (cnt === 0)
                    {
                         connection.execute(
                `INSERT INTO TFAMCORGDS_TEST (CODFAMCOR,MNEFAMCOR,DATMOD,DATFINACT) VALUES (SCORGDS.NEXTVAL,'`+ act_fam + `',SYSDATE,NULL)`,
                function(err, results) {
                    if (err) throw err;
                    console.log('Rows Inserted: ', results.rowsAffected);
                    //do work on the rows here
                    
                    
                }
            );
                    }
             });



       
            

		
		}
    }
);

And also I am not able to use variable "cnt" value outside connection.execute function though it is global variable.

I will segregate tasks in different functions and wrap them in promises instead soing callbacks in loops. the code will look something like this

var XLSX = require("xlsx");
var workbook = XLSX.readFile("Accounts.xlsx");
var sheet_name_list = workbook.SheetNames;
var xlData = XLSX.utils.sheet_to_json(workbook.Sheets[sheet_name_list[0]]);
var connection;
var oracledb = require("oracledb");
oracledb.autoCommit = true;
var dbConfig = require(__dirname + "/dbconfig.js");
var cnt;
function getConnection() {
return new Promise((resolve, reject) => {
oracledb.getConnection(dbConfig, function(err, connection) {
if (err) {
reject(err);
} 
resolve(connection);
});
});
}
function get(connection, act_fam) {
return new Promise((resolve, reject) => {
connection.execute(`SELECT * FROM TFAMCORGDS_TEST WHERE MNEFAMCOR='` + act_fam + `'`, function(err, results) {
if (err) {
reject(err);
} 
resolve(results);
});
});
}
function insert(connection, act_fam) {
return new Promise((resolve, reject) => {
connection.execute(
`INSERT INTO TFAMCORGDS_TEST (CODFAMCOR,MNEFAMCOR,DATMOD,DATFINACT) VALUES (SCORGDS.NEXTVAL,'` +
act_fam +
`',SYSDATE,NULL)`, function(err, results) {
if (err) {
reject(err);
} 
resolve(results);
});
});
}
async function main() {
const connection = await getConnection();
for (i in xlData) 
{
var act_fam = xlData[i].ACCOUNT_FAMILY;
const results = await get(connection, act_fam);
cnt = results.rows.length;
if (cnt === 0) {
const insertResult = await insert(connection, act_fam);
console.log('Rows Inserted: ', insertResult.rowsAffected);
}
}
}

Hope this helps

You should create a separate async function for access the database. Because when we perform some database operations it takes some time. we have to wait until finish that to do process next one. So i'm not going to put whole code here. Just a example.

async function main(){
//send the data to database accessing function
for(i in xlData){
await insertData(xlData[i].ACCOUNT_FAMILY);
}
}
async function insertData(data){
//do the database stuff here
}

NOTE: I'm using await keyword for perform asynchronous function.

I like Ashish Modi's answer, but I don't think he knew that the driver already supports promises, so his code could be simplified.

First, you don't need a getConnection function. The driver's getConnection function already returns a promise if you don't pass a callback, so this function doesn't add anything.

The get function has the same problem. It's not needed because the driver's execute method already supports promises.

The code could have looked like this:

var XLSX = require("xlsx");
var workbook = XLSX.readFile("Accounts.xlsx");
var sheet_name_list = workbook.SheetNames;
var xlData = XLSX.utils.sheet_to_json(workbook.Sheets[sheet_name_list[0]]);
var connection;
var oracledb = require("oracledb");
oracledb.autoCommit = true;
var dbConfig = require(__dirname + "/dbconfig.js");
async function main() {
const connection = await oracledb.getConnection(dbConfig);
for (i in xlData) 
{
var act_fam = xlData[i].ACCOUNT_FAMILY;
const results = await connection.execute(`SELECT * FROM TFAMCORGDS_TEST WHERE MNEFAMCOR='` + act_fam + `'`);
var cnt = results.rows.length;
if (cnt === 0) {
const insertResult = await connection.execute(
`INSERT INTO TFAMCORGDS_TEST (CODFAMCOR,MNEFAMCOR,DATMOD,DATFINACT) VALUES (SCORGDS.NEXTVAL,'` +
act_fam +
`',SYSDATE,NULL)`);
console.log('Rows Inserted: ', insertResult.rowsAffected);
}
}
}

However, there are still big problems with the code: SQL injection vulnerabilities and excessive round trips.

The code is currently using string concatenation to pass values to SQL statements, this will open you up to SQL injection and performance issues. You should use bind variables instead, as documented here: https://oracle.github.io/node-oracledb/doc/api.html#bind

The code would look like this after:

var XLSX = require("xlsx");
var workbook = XLSX.readFile("Accounts.xlsx");
var sheet_name_list = workbook.SheetNames;
var xlData = XLSX.utils.sheet_to_json(workbook.Sheets[sheet_name_list[0]]);
var connection;
var oracledb = require("oracledb");
oracledb.autoCommit = true;
var dbConfig = require(__dirname + "/dbconfig.js");
async function main() {
const connection = await oracledb.getConnection(dbConfig);
for (i in xlData) 
{
var act_fam = xlData[i].ACCOUNT_FAMILY;
const results = await connection.execute('SELECT * FROM TFAMCORGDS_TEST WHERE MNEFAMCOR= :act_fam', [act_fam]);
var cnt = results.rows.length;
if (cnt === 0) {
const insertResult = await connection.execute(
'INSERT INTO TFAMCORGDS_TEST (CODFAMCOR,MNEFAMCOR,DATMOD,DATFINACT) VALUES (SCORGDS.NEXTVAL, :act_fam,SYSDATE,NULL)', [act_fam]);
console.log('Rows Inserted: ', insertResult.rowsAffected);
}
}
}

Now the code is simple and safe. You could stop there if you're working with just a few rows (and the number will not grow over time) and performance is fine. Otherwise, please continue...

The current implementation is doing what we call row-by-row or slow-by-slow processing. As a developer, you should try to avoid excessive network round trips (network is the worst kind of I/O). You have two execute calls in the loop, so that's two round trips for each iteration of the loop.

With Oracle you have lots of tools available to cut down on round trips, so you could take different approaches here. For example, you could look into executeMany: https://oracle.github.io/node-oracledb/doc/api.html#-30-database-round-trips

However, in this case, I think the simplest approach would likely be to ship the statements to the database as an anonymous PL/SQL block. That would look like this:

var XLSX = require("xlsx");
var workbook = XLSX.readFile("Accounts.xlsx");
var sheet_name_list = workbook.SheetNames;
var xlData = XLSX.utils.sheet_to_json(workbook.Sheets[sheet_name_list[0]]);
var connection;
var oracledb = require("oracledb");
oracledb.autoCommit = true;
var dbConfig = require(__dirname + "/dbconfig.js");
async function main() {
const connection = await oracledb.getConnection(dbConfig);
const act_fams = [];
for (i in xlData) 
{
act_fams.push(xlData[i].ACCOUNT_FAMILY);
}
await connection.execute(
`declare
type number_aat is table of number
index by pls_integer;
l_act_fam_arr number_aat;
l_count       number;
begin
l_act_fam_arr := :act_fam_arr;
for act_fam in 1 .. l_act_fam_arr.count
loop
select count(*) 
into l_count
from tfamcorgds_test 
where mnefamcor=act_fam;
if l_count = 0
then
insert into tfamcorgds_test (
codfamcor,mnefamcor,datmod,datfinact
) values (scorgds.nextval, act_fam,sysdate,null);
end if;
end loop;
end;`,
{
act_fam_arr: {
type: oracledb.NUMBER,
val: act_fams
}
}
);
}

I've not tested this code so there are likely syntax errors. Note that the first parameter I'm passing to execute is a large string of code, a PL/SQL block. The second parameter is the bind variable, which is an array of numbers (I assumed ACCOUNT_FAMILY was a number, but you could easily change it to a string if needed).

The code and values will be sent to the database via a single network roundtrip. The PL/SQL code implements the same logic you had before in JavaScript. If you run a test with this code compared to the previous version, you should see a noticeable performance improvement (the more rows fetched, the more noticeable the improvement will be).

If MNEFAMCOR is unique, e.g. like:

CREATE TABLE TFAMCORGDS_TEST (CODFAMCOR NUMBER, MNEFAMCOR VARCHAR2(20) PRIMARY KEY, DATMOD DATE, DATFINACT VARCHAR2(10));

then another way (on top of Dan's answers) to reduce round trips is by using the batchErrors mode of executeMany(). This will allow new records to be inserted while flagging the records that were duplicates:

var XLSX = require("xlsx");
var workbook = XLSX.readFile("Accounts.xlsx");
var sheet_name_list = workbook.SheetNames;
var xlData = XLSX.utils.sheet_to_json(workbook.Sheets[sheet_name_list[0]]);
var connection;
var oracledb = require("oracledb");
var dbConfig = require(__dirname + "/dbconfig.js");
async function main() {
const connection = await oracledb.getConnection(dbConfig);
const sql = `INSERT INTO TFAMCORGDS_TEST (CODFAMCOR,MNEFAMCOR,DATMOD,DATFINACT) 
VALUES (SCORGDS.NEXTVAL, :ACCOUNT_FAMILY, SYSDATE, NULL)`;
const options = {
batchErrors: true,
bindDefs: {
ACCOUNT_FAMILY: { type: oracledb.STRING, maxSize: 20 }
}
};
const result = await connection.executeMany(sql, xlData, options);
await connection.commit();
console.log("Result is:", result);
}
main();

If your spreadsheet is really, really huge, then you may need to call executeMany() several times with subsets of the records.

A side note: Oracle 12 introduced identity columns, so the complexity of using sequences can be reduced:

CREATE TABLE TFAMCORGDS_TEST (CODFAMCOR NUMBER GENERATED BY DEFAULT ON NULL AS IDENTITY, MNEFAMCOR VARCHAR2(20) PRIMARY KEY, DATMOD DATE, DATFINACT VARCHAR2(10));

With this, you don't need to include SCORGDS.NEXTVAL in the INSERT.