IBM / nodejs-idb-connector

A JavaScript (Node.js) library for communicating with Db2 for IBM i, with support for queries, procedures, and much more. Uses traditional callback-style syntax
MIT License
38 stars 23 forks source link

idb-connector *SP calls returns invalid numeric data -- Node 18, IDB-Connector 1.2.18 #169

Closed mikebohlen closed 5 months ago

mikebohlen commented 1 year ago

So the issue is on a stored procedure call, here i attempt a sign off on a task:

call WOR3030SP('AD', 0, 0, 'LKS4B', 14023359, 'WTBAL', 'MECH', 'A', 'KRINJ', '0001-01-01-00.00.00.000000', '', '0001-01-01-00.00.00.000000', '', '2023-06-05-09.51.29.303000', 'Y')

and the resulting data returned:

{"RSESID":"41751901\u0000","RSSEQN":"0\u0000","RSWONO":"LKS4B","RSRFID":"14023359\u0000","RSFTYP":"WTBAL ","RSSTYP":"MECH ","RSSTAT":"A","RSNTID":"88986940\u0000","RSSIGN":"KRINJ ","RSSGNTS":"2023-06-05-09.51.29.716000","RSRMVUS":" ","RSRMVTS":"0001-01-01-00.00.00.000000","RSLCUS":"KRINJ ","RSLCTS":"2023-06-05-09.51.29.716000","RSNAME":"Josiah S Krings ","RSSTMP":"VI 1063 ","RSSHOP":"012","RSTSN":".0\u0000","RSLNDG":"0\u0000"}

notice each numeric value has \u0000 appended to the end results in NaN error, Not a Number in node.

snotmare commented 1 year ago

This is related to https://github.com/IBM/nodejs-idb-connector/issues/147. #147 appears to be partially resolved, but this is a new issue found after that.

abmusse commented 1 year ago

@mikebohlen

Can you please provide the column definitions for the result set being returned by the stored procedure?

We are going to attempt a recreate of the issue

mikebohlen commented 1 year ago

Column Type Length Precision RSESID DECIMAL 9 0 RSSEQN DECIMAL 3 0 RSWONO CHARACTER 5
RSRFID DECIMAL 9 0 RSFTYP CHARACTER 10
RSSTYP CHARACTER 10
RSSTAT CHARACTER 1
RSNTID DECIMAL 9 0 RSSIGN CHARACTER 8
RSSGNTS TIMESTAMP 26
RSRMVUS CHARACTER 8
RSRMVTS TIMESTAMP 26
RSLCUS CHARACTER 10
RSLCTS TIMESTAMP 26
RSNAME CHARACTER 35
RSSTMP CHARACTER 10
RSSHOP CHARACTER 3
RSTSN DECIMAL 9 1 RSLNDG DECIMAL 7 0

snotmare commented 1 year ago

@abmusse I work with @mikebohlen and have been watching this issue. I'm not sure if it matters or not, but the decimal values behind this table are stored as packed decimal in db2.

abmusse commented 1 year ago

@snotmare Thanks for the additional info Does the issue only occur with result sets from a stored procedure output?

snotmare commented 1 year ago

@abmusse , yes. We've only run into this issue with stored procedures. Direct SQL seems fine.

abmusse commented 1 year ago

Ok, What language does the stored procedure use?

mikebohlen commented 1 year ago

Object Type Library Attribute Text WOR3030P *PGM DUNC RPGLE SQL

abmusse commented 1 year ago

OK I tried recreating a stored procedure that returns a result set with a Decimal column:

CREATE OR REPLACE TABLE test.decimaltest(test DECIMAL); 
INSERT INTO test.decimaltest(test) VALUES (10); 
 CREATE OR REPLACE PROCEDURE test.retdec () LANGUAGE SQL 
 DYNAMIC RESULT SETS 1
 BEGIN
 DECLARE C1 CURSOR FOR SELECT * FROM test.decimaltest;
 OPEN C1;
 RETURN; 
 END
[{"TEST":"10"}]

Calling the procedure returned the result set normally. My stored procedure was writtin in SQL though.

Looks like your stored procedure was written in RPGLE. I will need to recreate a stored procedure in RPGLE and see if I can reproduce the bug.

abmusse commented 1 year ago

@snotmare @mikebohlen

I'm curious what happens if you enable statement.asNumber(true) ?

If you call that right after creating your statement that should get you back the decimal values as JavaScript Numbers rather than strings. Ultimately this may be what you actually want.

I'm still investigating recreating stored procedure in RPGLE.

mikebohlen commented 1 year ago

I have asked the developer to try that. Should know shortly. Thanks

snotmare commented 1 year ago

@abmusse Yes, that worked! Calling asNumber(true) resulted in numbers being returned back without the unicode char on the end.

Does this help you locate the source of the error? Are we expected to have asNumber() turned on all the time?

snotmare commented 1 year ago

@abmusse We've done a bit more testing and have run into this issue again, even though we have asNumber() turned on. In this case, we have a stored procedure defined this way:

CREATE PROCEDURE DUNC/GPR5016SP ( 
    INOUT GACT CHAR(2) , 
    IN IGNO CHAR(1) , 
    IN CUSR CHAR(10) , 
    IN NAME CHAR(10) , 
    IN LIBRARY CHAR(10) , 
    IN "TYPE" CHAR(10) , 
    IN CHARVALUE VARCHAR(2000) , 
    IN DECIMALVALUE DECIMAL(31, 9) , 
    IN INCREMENTVALUE DECIMAL(31, 9) ) 
    DYNAMIC RESULT SETS 1 
    LANGUAGE RPGLE 
    SPECIFIC DUNC/GPR5016SP 
    NOT DETERMINISTIC 
    MODIFIES SQL DATA 
    CALLED ON NULL INPUT 
    EXTERNAL NAME 'DUNC/GPR5016P' 
    PARAMETER STYLE GENERAL ; 

The results are defined this way:

type: char(10)
length: int(10)
decimalPositions: int(10)
charValue: varchar(2000)
decimalValue: packed(31:9)

I've logged the output from the statement.fetchAll(), and we get that same unicode char on the end of the decimal value field:

[{"TYPE":"*DEC      ","LENGTH":9,"DECIMALPOSITIONS":0,"CHARVALUE":"","DECIMALVALUE":"42923783.000000000\u0000"}]

We have a work around for now, which is to look for that unicode char on the end and simply remove it. However, it would be nice to not have to do that.

Thanks for your help!

abmusse commented 1 year ago

Thanks for the additional info!

I was hoping setting statement.asNumber would provide you a workaround until we solve the original bug.

Sounds like you have a workaround in place currently, which is good to hear.

I believe we are reading in the the string + Null terminator in the code and returning that back in the result set.

I'm going to revisit the changes in https://github.com/IBM/nodejs-idb-connector/commit/99e127a1c3eedb31cdb858fd911411a4d5d2d1ef

abmusse commented 1 year ago

@snotmare @mikebohlen

I got the example rpgle stored procedure building now and I'm seeing the Nulls at the end when I call it.

Did this just start after the changes in https://github.com/IBM/nodejs-idb-connector/commit/99e127a1c3eedb31cdb858fd911411a4d5d2d1ef i.e. releases: 1.2.17 - 1.2.18?

Or was it an existing issue?

abmusse commented 1 year ago

OK I'm documenting the process of re-creating the issue with an RPGLE external Stored Procedure mainly for my notes:

1) Copy over the RPGLE source file to the IFS ( In this case the file name is decsp.rpgle)

**free
dcl-ds info qualified DIM(1);
  salary packed(9 : 2) inz(50000.25);
end-ds;

exec sql
Set Result Sets Array :info For 1 Rows;
return;

2) Run CL command to create SQL ILE RPG Object

 CRTSQLRPGI OBJ(MYLIB/DECSP) OBJTYPE(*PGM) SRCSTMF('/home/MYUSER/decsp.rpgle') TEXT('RPGLE SP returns decimal in RS') CVTCCSID(37) 

3) Save the following to an SQL file named: create_decsp_rple.sql

CREATE OR REPLACE PROCEDURE MYLIB.DECSP
LANGUAGE RPGLE NOT DETERMINISTIC
Result Sets 1
EXTERNAL NAME 'MYLIB/DECSP'
PARAMETER STYLE GENERAL;

Use db2util to create the SQL stored procedure:

db2util "$(cat create-decsp-rple.sql)"

4) Call the Stored Procedure using idb-connector:

const { dbconn, dbstmt } = require('idb-connector');

const schema = 'MYLIB';
const procedure = 'DECSP';
const sql = `CALL ${schema}.${procedure}()`;

const connection = new dbconn();
connection.debug(true);
connection.conn('*LOCAL');
const statement = new dbstmt(connection);

function cleanup(connection, statement) {
      statement.close();
      connection.disconn();
      connection.close();
}

statement.prepare(sql, (p_error) => {
  if (p_error) {
    console.error("prepare");
    cleanup(connection, statement);
    throw p_error;
  }
  statement.execute((out, e_error) => {
    if (e_error) {
      console.error("execute");
      cleanup(connection, statement);
      throw e_error;
    }
    statement.fetchAll((result, f_ error) => {
      if (f_error) {
        console.error('fetch');
        cleanup(connection, statement);
        throw f_error;
      }
      console.log(`Result Set:${JSON.stringify(result)}\n`);
      cleanup(connection, statement);
    });
  });
});

This returns

Result Set:[{"SALARY":"50000.25\u0000"}]
abmusse commented 1 year ago

From there I looked through the code, The issue looks to manifest here:

https://github.com/IBM/nodejs-idb-connector/blob/811ce13dca4e5a6268cbfcf6648818782b3d703f/src/db2ia/dbstmt.cc#L2493-L2498

In the case of RPGLE Stored Procedure Result set we end up in this if statement:

https://github.com/IBM/nodejs-idb-connector/blob/811ce13dca4e5a6268cbfcf6648818782b3d703f/src/db2ia/dbstmt.cc#L2495

In the case of the SQL Stored Procedure we end up in the else:

https://github.com/IBM/nodejs-idb-connector/blob/811ce13dca4e5a6268cbfcf6648818782b3d703f/src/db2ia/dbstmt.cc#L2497

rlength in the case of the SQL Stored Procedure is equal to -3 which mean SQL_NTS. In the RPGLE Stored Procedure case rlength is set to the length of the buffer (including the null terminator).

For reference rlength gets set in fetchData

It seems like we should just do like the else case and pass the pointer to the buffer and not the rlength. The null will get picked up as the end of the string when creating that Napi::String

I applied the following changes locally:

image

Then re-ran calling the stored procedure:

Result Set:[{"SALARY":"50000.25"}]

No Nulls!

snotmare commented 1 year ago

Well done @abmusse !

We noticed this issue when attempting to upgrade node js to v16 (and now v18). This doesn't appear to be an issue with node 14.

abmusse commented 1 year ago

Yes I ran a git blame and it looks like this change was introduced in:

https://github.com/IBM/nodejs-idb-connector/commit/56d66e7faab1a8fd61ccb6971a3555723d8a63a5

abmusse commented 1 year ago

From the comment https://github.com/IBM/nodejs-idb-connector/issues/127#issuecomment-713317686 the change did not fix the truncation issue though.

So I'm not sure why it was still merged.

Update: Looks like this commit was an attempt to resolve another issue see: https://github.com/IBM/nodejs-idb-connector/issues/123#issuecomment-713226181 (Unfortunately looks like the issues persisted)

I'll open a PR with the changes:

image

github-actions[bot] commented 12 months ago

:wave: Hi! This issue has been marked stale due to inactivity. If no further activity occurs, it will automatically be closed.