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

State still in SentClientRequest when 'done' event was fired #9

Closed
baoshan opened this issue Jan 20, 2012 · 12 comments
Closed

State still in SentClientRequest when 'done' event was fired #9

baoshan opened this issue Jan 20, 2012 · 12 comments

Comments

@baoshan
Copy link

baoshan commented Jan 20, 2012

When I try to benchmark a simple query as below:

var i = 0;
var start = new Date;
var query = function () {
  request = new Request("select top 100 cityid from dbo.cities", function(err) {
    console.log('err', err);
  });

  request.on('row', function(columns) {
    columns.forEach(function(column) {         
    });
  });

  request.on('done', function(rowCount, more) {
    if(i++ < 200){
      console.log('i:', i);
      query();
    }
    else {
      console.log(new Date - start);
    }
  });
  connection.execSql(request);
};

connection.on('connect', function(err) {
    // If no error, then good to go...
    console.log('connection established');
    query();
});

below exception was thrown:
Invalid state; requests can only be made in the LoggedIn state, not the SentClientRequest state

@KillWil
Copy link

KillWil commented Feb 2, 2012

Hello, I have a similar problem.
When connection "end" or request "done" is fired I can't call another request (same error as baoshan).
How can I run queries synchronously ?

@pekim
Copy link
Collaborator

pekim commented Feb 9, 2012

Sorry for the lack of activity on this issue. I intend to take a proper look at it this weekend.

@pekim pekim closed this as completed in 39cdbb4 Feb 12, 2012
@pekim
Copy link
Collaborator

pekim commented Feb 12, 2012

Because a request can execute multiple statements, multiple done events may be emitted for a single request.

select 4; select 'abc';

So the request's done event is not the right place to initiate another request. Another request can be intiated in the request's completion callback.

var request = new Request("select ...", function(err) {
  if (err) {
    console.log('err', err);
  }

  makeRequest(...);
});

There was also a bug in the Connection's state machine which prevented a new request from being initiated in the Request's completion callback. I've fixed that bug, and added a test to test/integration/connection-test.coffee to verify that a new Request can be initiated.

@baoshan
Copy link
Author

baoshan commented Feb 13, 2012

Thanks!

Does the current code provide output parameter support?

And, should multiple result sets be obtained through the done event? If I can provide multiple callback, as

new Request("select ...", 
  function processResult1(){}, 
  function processResult2(){}, 
  function completion(){} 
);

Great job!

@pekim
Copy link
Collaborator

pekim commented Feb 13, 2012

Does the current code provide output parameter support?

No, I am afraid that it does not. I am looking to add support for parameterised requests next, and this will include input and output parameters.

And, should multiple result sets be obtained through the done event?
If I can provide multiple callback, as

No, multiple result sets are provided for with the existing API. Each result set will result in 0 or more row events, followed by a done event.

row
row
...
done
row
row
...
request-completion-callback called

In most use cases I imagine that it would be best to stick to requests with statements that cause only a single result set.

@baoshan
Copy link
Author

baoshan commented Feb 13, 2012

Enabling multiple result sets indeed satisfies some real needs which should not be ignored totally in my humble opinion :P

Great plan on supporting parameterised request!

@pekim
Copy link
Collaborator

pekim commented Feb 13, 2012

Multiple result set are supported, as I mentioned previously. Are you suggesting that multiple results should be buffered and presented to (multiple) request completion callbacks?

What would be the advantage over the current events that provide the result sets? What would happen if the number of callbacks doesn't match the number of result sets?

Result set rows can't be buffered, as that might result in vast amounts of memory being consumed.

In general I'd prefer to keep the API relatively low level, and fairly close to the TDS protocol. A higher level API could always be built on top.

I'm sorry if I'm misunderstanding what you're asking for.

@baoshan
Copy link
Author

baoshan commented Feb 14, 2012

I'm sorry for making you feel confused.

I'm arguing for

In most use cases I imagine that it would be best to stick to requests with statements that cause only a single result set.

since in some real scenarios, multi result sets helps. The current implementation in supporting multiple result sets is so great that I have no idea how it could be improved :)

I'm also fond of the philosophy -- a TDS spec's counterpoint in JavaScript / coffee script.

I'll keep playing tedious :P

@pekim
Copy link
Collaborator

pekim commented Feb 14, 2012

Thanks. I'm pleased that we're in broad agreement.

@kimi23
Copy link

kimi23 commented Dec 3, 2012

HI Pekim,

I am doing a simple insert in nodejs using tedious and when I am trying to put this in a transactions I am getting the error - "Invalid state; requests can only be made in the LoggedIn state, not the SentClientRequest state"

My Code:

var storedProcName = '[Employess].[dbo].[sp_InsertEmployee]';
var request = new Request(storedProcName, function(err, rowCount) {
if(err){
//console.log('Error on select');
}
else {
console.log(rowCount + ' rows');
}
connection.close();
});

request.addParameter('employee_name', TYPES.VarChar, 'Green Day');

request.on('row', function(columns) {
  columns.forEach(function(column) {
     if (column.value != null) {          
      console.log('Value Returned ' + column.value);
    }
  });
});

request.on('doneProc', function(rowCount, more, returnStatus) {      
});

connection.beginTransaction(function(err)
{    
  if(err){
    console.log('Error in transaction ' + err);
  }
  else
  {
    connection.callProcedure(request);
    connection.commitTransaction(function(err){
      if(err){
          console.log('Error in Commiting transaction ' + err);
      }
      else{
          console.log('Transactiuon commited successfully');
      }
    });
  }      
});

and the stored proc is a simple insert which return the @@IDENTITY when completed.

Can you please help ?

Thanks

@kimi23
Copy link

kimi23 commented Dec 4, 2012

Hi pekim,

One more question..

How do I call the request in a foreach loop as I need to insert multiple records./

SImple example.. Insert multiple locations for a customer ?

How would I do that?

Where do I call the foreach loop ?? In the callback of request?? How??

Thanks in advance.

Regards,

@michelle-becker
Copy link

So, I am having the same issue as @kimi23 - was a solution to this ever found? Are there docs somewhere that detail exactly how the tedious transaction logic should work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants