sqlanywhere / node-sqlanywhere

SAP SQL Anywhere Database Client for Node
Apache License 2.0
38 stars 36 forks source link

Application crashes on execution #38

Closed DCrow closed 4 years ago

DCrow commented 4 years ago

App crashes randomly(node reports it as abort) when executing an sql statement. Sometimes the same statement executes successfully but most times it crashes.

I am using node - 10.0.0 sqlanywhere - 16.0.0.2252

Sample code

var sqlanywhere = require('sqlanywhere');
var conn = sqlanywhere.createConnection();

conn.connect("<some connection string>", function(err) {
  if (err) throw err;
  conn.exec('call test(100500);', [], function (err, result) {
    if (err) throw err;

    console.log(result);
    conn.disconnect();
  });
});

test procedure declaration

create or replace procedure test(@param int)
begin
    select newid() as a
end
DanCummins-SAP commented 4 years ago

Hi DCrow,

I got the following response from one of our developers.

The sqlanywhere and sqla-hana node.js drivers do not support stored procedures that take INOUT or OUTPUT parameters as well as returning result set. Stored procedures with IN parameters as well returning result set are fully supported. By default, parameters defined in a stored procedure will be INOUT, if no key word, IN, INOUT, or OUT is specified for the parameters.

The stored procedure used was defined as:

create or replace procedure test(@param int) begin select newid() as a end

Therefore the parameter @param is an INOUT parameter, even though this parameter has never been used in the stored procedure. To workaround this problem, the user should use the key word, IN for the parameter.

Hope this helps and Best Regards, Dan

DCrow commented 4 years ago

Hello Dan,

It really does not crash if I specify parameter as IN, but still a least it should not crash, maybe return an error that it can't process the given query?

Unfortunately there are a lot of procedures where IN/INOUT/OUT statements are not written(implicit INOUT), any many of them are written by different development teams...

Are there any other alternatives?

DanCummins-SAP commented 4 years ago

Hi DCrow,

It's a limitation - the driver can do either an OUT parameter, or a result set, but not both. Unfortunately the app will likely need modification.

You're right it shouldn't crash. We were not able to reproduce a crash in-house, we got an error, "Cannot convert uniqueidentifier to integer". Could you give the exact version of node, SQLA server and SQLA client, and driver version? Also the platform and version. Maybe then we would be able to reproduce the crashing behaviour?

Kind regards, Dan

DCrow commented 4 years ago

I am using

Node - 10.0.0
SQLA - 16.0.0.2252 (Tried by creating a local database using dbinit/dbeng, so client and server versions should be the same)
node-sqlanywhere - 1.0.23
Mac OS X - 10.14.6 (Darwin Kernel Version 18.7.0)

I have also successfully reproduced the same behaviour using sqlanywhere ruby library.

It seems to be a problem somewhere in sqlanywhere c api library since both(ruby and node) libraries use this library.

DanCummins-SAP commented 4 years ago

SQL Anywhere 16.0 is EOL as of December 31, 2018, so we no longer fix bugs and ship EBFs for this version. We were using version 17 in our tests. Upgrading to version 17.0.10 should avoid the crash.

Dan

DanCummins-SAP commented 4 years ago

BTW, you are correct the limitation is in the sqlanywhere c api library (the basis for several drivers including Node and Ruby). Dan

DCrow commented 4 years ago

It does return an error instead of crashing on SQLAnywhere 17.

Are there any plans to make this type of statement return a query result? It does seem that there is some support for IN/OUT parameters built in C Api library. Are there any plans to expand the library? Like MySQL/Postgres C Api ?

YufeiGuo commented 4 years ago

We have no short term plan to expand this support, because it will involve big changes in dblib. However, I think you can work around this problem by introducing extra result sets instead of using output parameters in the stored procedure, because the node.js driver supports multiple result sets.

DCrow commented 4 years ago

It does not always work as expected

This example returns only the result from the first query

select 1; call test(100500);

This query also returns only the first result

select 2; call test(100500); select 3;

Even a simple sql query such as this does not return all results...

select 2; select 3;

But If I use something like this, it will return result from the procedure

update products set name = 'Test' where id = 1; call test(100500); 

All in all this seems to be a no-go since I don't always know what to expect...

YufeiGuo commented 4 years ago

We are trying to replace the OUTPUT parameters with SELECT statements. So the SELECT statements should be inside of the stored procedure.

DCrow commented 4 years ago

It does not crash when I declare the procedure as

create or replace procedure test(@param int)
begin
    select 1; select newid() as a
end

But it reads only the first result...

Also this approach breaks other services using the same procedure, since it returns multiple results, but in most integrations are expecting only one result instead of multiple

YufeiGuo commented 4 years ago

The stmt.exec(...) will return the first result set. To get the next result set, you need to call rs = stmt.getMoreResults(); Here is a simple example: stmt = conn.prepare( 'select 1 as a from dummy; select 2 as b, 3 as c from dummy' ); rs = stmt.exec(); // rs == [ { a: 1 } ] rs = stmt.getMoreResults(); // rs = [ { b: 2, c: 3 } ] stmt.drop();

DCrow commented 4 years ago

Thank you!

This way it does really do everything as I need it to.