"done" fires before last result? about node-sqlserver-v8 HOT 9 CLOSED

I think this has been the default behaviour inherited from the original node-sqlserver.

How about something like the following:

loadedQuery = `SET NOCOUNT ON ${loadedQuery}`;

from node-sqlserver-v8.

@NoelAbrahams I don't think that would work when loadedQuery is actually a call to a stored procedure.

For now I'm solving this by using the more flag (which doesn't seem to be documented) :

let q = conn.query(loadedQuery, (err, res, more) => {
  results = res;
  if (more === false) {
    console.log(`Final result from query: ${JSON.stringify(results)}`)
  }
});

from node-sqlserver-v8.

Actually it should work for stored procedures:

CREATE PROC dbo.foo 
AS

	DECLARE @v1 table(id int)

	DELETE @v1

	DELETE @v1

GO

SET NOCOUNT OFF 

EXEC dbo.foo

GO

SET NOCOUNT ON 

EXEC dbo.foo	

GO

DROP PROC dbo.foo

from node-sqlserver-v8.

@NoelAbrahams Yes, but not if part of the stored proc sets NOCOUNT OFF...I don't have any control over how the stored procs were set up, so I can't guarantee that isn't the case.

from node-sqlserver-v8.

@jdpearce, I see what you mean - although it is very unusual to set no-count to off. (You could do a find and replace of NOCOUNT in your stored procedure string, but that's probably going into hack territory.)

The reason I suggested the workaround is because the more flag is normally used to process a stored procedure that returns multiple result sets, e.g.

CREATE PROC dbo.foo 
AS

	SELECT v = 'result one'

	SELECT v = 'result two'

from node-sqlserver-v8.

@NoelAbrahams Unfortunately all I have to work with is something that often looks like exec myschema.MyStoredProc @thing = <value>, @thing2 = <value2>...

It's not something I can hack easily, or even tell the library it's a stored proc, because I don't want to get into parsing that string for the params.

So, I can either stick to using the more flag, which works for now and probably won't change, or I can use the "done"/"completed" event incorrectly, i.e. wait for the event and then assume the next result is the last one. That the latter might change seems more likely to me.

from node-sqlserver-v8.

an interesting discussion. I guess it boils down to should a change be made to the event ordering, specifically should done actually only fire at the end or should we leave things as they are and add documentation around the more flag explaining how it could be used.

Let me know what you think is the best option given where things stand.

thanks for raising the issue, and for the ensuing discussion.

from node-sqlserver-v8.

You're welcome. 🤓

I'd suggest documentation around the "more" flag either way.

If the event ordering is simply inherited from the original project and not an artefact of the underlying protocol, then I'd suggest that the ordering should be changed such that "done" and "completed" both fire after results are received. Documentation should be added to indicate this.

(small aside - what's the difference between "done" and "complete"?)

However, if the event ordering is actually an artefact of the underlying protocol, then simply add documentation about the expected ordering of events.

from node-sqlserver-v8.

This issue is resolved in latest version I will hence close. Please let me know if there are anymore problems

from node-sqlserver-v8.