Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to chain multiple sql query #490

Closed
inttyl opened this issue Aug 8, 2016 · 8 comments
Closed

How to chain multiple sql query #490

inttyl opened this issue Aug 8, 2016 · 8 comments
Labels

Comments

@inttyl
Copy link

inttyl commented Aug 8, 2016

Hi,
I am pretty new to oracledb with NodeJS.
I want to do mutiple sql call that are dependant.
Before executing the query sqltoExecute, I need to configure the connection by executing a query that set "set role " and then another one to "set package" and finally execute my query sqltoExecute.

I am trying to find some examples but failed to find any.

Below, a snipper of the code I tried to execute but it fails since it only execute the first query , it fails to execute the second "then" that execute the query " sqlQueries.sqlQuerySetPackage()"

I am using on NodeJS 4.4.7 on Ubuntu 14.

Thanks for help.

    exports.testChainingSqlQueries = function(config, sqlToExecute, callback) {

        if (config) {
            oracledb.getConnection(config)
                .then(function(conn){

                    return conn.execute(
                            sqlQueries.sqlQuerySetRole()
                        )
                        .then(function(result){
                            console.log("Execution Succes : "+ sqlQueries.sqlQuery1());
                            return conn;
                        })
                        .catch(function(err){
                            console.log("Error Executing  "+ sqlQueries.sqlQuery1() );
                            return conn.close();
                        })
                })
                .then(function(conn){

                    return conn.execute(
                            sqlQueries.sqlQuerySetPackage()
                        )
                        .then(function(result){
                            console.log("Execution Succes : "+ sqlQueries.sqlQuery2());
                            return conn;
                        })
                        .catch(function(err){
                            console.log("Error executing :  "+ sqlQueries.sqlQuery2());
                            return conn.close();
                        })
                })
                .then(function(conn){
                    return conn.execute(
                            sqlToExecute
                        )
                        .then(function(result){
                            console.log("Execution Succes : "+ sqlToExecute);
                            callback(result, null);
                            return conn.close();
                        })
                        .catch(function(err){
                            console.log("Error executing : "+ sqlToExecute);
                            return conn.close();
                        })
                });

        } else {
            callback(null, {
                message: "Configuration is invalid ",
                config: config
            });
        }
    };`
@cjbj
Copy link
Member

cjbj commented Aug 9, 2016

Have a look at Promise.all. There is a tip in #350 (comment) that may interest you.

@sagiegurari has a popular wrapper that you could evaluate.

But overall, you may want to wrap up your command in a single PL/SQL block. This means node-oracledb only has to execute one statement, which may help overall scalability.

@sagiegurari
Copy link

might want to look at connection.transaction that enables to run multiple activities either in parallel (default) or in sequence (defined in options object)
https://github.com/sagiegurari/simple-oracledb#usage-transaction

@dmcghan
Copy link

dmcghan commented Aug 10, 2016

@sagiegurari After some discussion with a colleague and some testing, I would recommend removing the sequence option in the transaction method. The reason is that the connection object acts as a serialization device, meaning it can only do one thing at a time anyway. That makes the option a little misleading.

When you use the async.series method, the queuing is done in the main JS thread. When you use async.parallel, the queuing ends up being done in the C layer via a mutex. The big problem here is that libuv isn't aware that a connection can only do one thing at a time - it only knows when it has background threads available and so it sends off the work to be done.

So when you run in "parallel", you could use more than 1 background thread (perhaps all of them) and each one could be waiting on the one before it to finish its "execute". Of course other users/transactions can't use them at that time either.

Here's a test that you can run to see that there's no real performance difference with the 2 methods (actually I generally had better times with series).

Given this table:

create table perf_test (
  c varchar2(30) not null
);

Run this test:

var async = require('async');
var oracledb = require('oracledb');
var dbConfig = require('./dbconfig.js');
var asyncFunc = 'series'; //or parallel

oracledb.getConnection(dbConfig, function(err, conn) {
  var funcs = [];
  var start = Date.now();
  var idx;

  if (err) {throw err;}

  function insertData(callback) {
    conn.execute(
      'insert into perf_test (c) values (:string)', 
      [Math.random().toString(36).replace(/[^a-z]+/g, '')],
      function(err, result) {
        callback(err);
      }
    );
  }

  for (idx = 0; idx <= 100000; idx += 1) {
    funcs[idx] = insertData;
  }

  async[asyncFunc](funcs, function(err, results) {
    if (err) {throw err;};

    conn.commit(function(err) {
      if (err) {throw err;};

      conn.close(function(err) {
        if (err) {throw err;};

        console.log('done', Date.now() - start);
      });
    });
  });
});

@sagiegurari
Copy link

that's 100% true, the only thing is that sometimes, the connection.execute is not the only async IO action you are doing. sometimes, it is writing to http response/file, or reading from a file.
I have several scenario where we store specific results in file for caching in case next time the DB connection is not available. And in case the select fails, we do the opposite and read from the file.
that file.write/read is basically also async IO operation.
During boot, we have around 3 of those type of queries, so it works nicely there (1 query is done, and it starts writing results to a file while a second query is being executed).

Anyone killing the node.js threads will end up killing his process. I usually tune it to around 10/20 threads, depending on the specific component.
Saying that, I totally agree that it adds up a risk and not everyone fully knows how every piece works, so will change that from parallel to eachOfLimit with some sensible limit based on the amount of threads defined for the node.js process (default is 4 if i remember correct), so i'll probably limit to half of what is defined.

@dmcghan
Copy link

dmcghan commented Aug 10, 2016

What you're describing seems to be a better fit for async.parallel, connection.transaction seems to imply "database transaction related to this connection".

If you're going to leave the option, maybe changing it to series by default would be good.

@sagiegurari
Copy link

ya, i'm actually making that change right now.
The name transaction is true because basically all operations get their autocommit turned off, and the commit and rollback get disabled.
only after all operations are done is a commit being done (or rollback in case of any error).
which is basically a transaction (all or nothing)

@sagiegurari
Copy link

Just published a new version with few changes like moving to parallelLimit (with limit of half the node.js threads) and using series as default and writing a bit more in the docs.
should protect users from doing nasty stuff I hope.

@cjbj cjbj added the question label Oct 19, 2016
@cjbj
Copy link
Member

cjbj commented Oct 19, 2016

Closing - I believe this has run its natural course.

@cjbj cjbj closed this as completed Oct 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants