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

select-insert when binding strings #114

Closed krisbaehr closed 4 years ago

krisbaehr commented 4 years ago

Describe the bug When performing a select-insert, and adding multiple rows, the string data types are written as blanks. Also, since timestamps are bound as strings, an insert with a timestamp results in an error since '' is not a valid timestamp. Inserting just one row with this method works as expected.

To Reproduce Steps to reproduce the behavior:

  1. Create a simple table with a string and a decimal value
  2. Run the script below

Expected behavior When inserting multiple rows, the string column in the table should contain the value that was bound, instead of blank.

Screenshots Inserting one row (works): image

image

Inserting two rows (does not work): image

image

server4.txt

Node.js version: v10.16.3 idb-connector version: 1.2.7 IBM i version: 3 7

Debug Output Prepare().
SQLPrepare(0): select * from final table ( insert into crpcnad (uid, adid) values (?, ?), (?, ?) ) BindParamAsync().
SQLBindParameter(0) TYPE[ 1] SIZE[ 10] DIGI[0] IO[1] IND[ -3] INDEX[0]
SQLBindParameter(0) TYPE[ 3] SIZE[ 8] DIGI[7] IO[1] IND[ 8] INDEX[1]
SQLBindParameter(0) TYPE[ 1] SIZE[ 10] DIGI[0] IO[1] IND[ -3] INDEX[2]
SQLBindParameter(0) TYPE[ 3] SIZE[ 8] DIGI[7] IO[1] IND[ 8] INDEX[3]
Execute().
SQLExecuteAsync(0):
SQLNUMRESULTSCOLS(0) Column Count = 3
DescCol(0) index[0] Type[DECIMAL] Scale[0] Precise[9] DescCol(0) index[1] Type[CHAR] Scale[0] Precise[10] DescCol(0) index[2] Type[DECIMAL] Scale[0] Precise[9] SQLFreeStmt: stmth 3 [SQL_DROP]
SQLFreeStmt(0)
SQLDisconnect: conn obj [18034c550] handler [2]
SQLFreeConnect: conn obj [18034c550] handler [2]
SQLFreeConnect[0]

dmabupt commented 4 years ago

Hello @krisbaehr , Would you try to change this line const STRING = 1; to const STRING = 0;?

I am not sure about the internal process of the CLI driver. But it seems that in this case, we have to calculate the length of the input string by idb-connector itself in advance. https://github.com/IBM/nodejs-idb-connector/blob/master/src/db2ia/dbstmt.cc#L2556-L2563

dmabupt commented 4 years ago

BTW, the first parameter of API execute() should be an array of the output parameters -- statement.execute((out, error) => {...

And the API bindParam() will be deprecated since v1.2.7. It is recommended to use the new and simplified API bindParameters() like this -->

// let params = [
//  ['foo', idb.SQL_PARAM_INPUT, STRING],
//  [1, idb.SQL_PARAM_INPUT, DECIMAL],
//  ['bar', idb.SQL_PARAM_INPUT, STRING],
//  [2, idb.SQL_PARAM_INPUT, DECIMAL]
// ];

let params = [ 'foo', 1, 'bar', 2];

statement.prepare(sql, (error) => {
    statement.bindParameters(params, (error) => {
        statement.execute((out, error) => {  
...
krisbaehr commented 4 years ago

@dmabupt Thank you for the tips! I changed the script to set the STRING constant to 0 and everything worked. I tried inserting one row and multiple rows and it worked for both.

I also tried the new bindParameters() syntax as suggested. The two rows were inserted but suffered from the same issue where the string column was inserted as blank. Inserting one row worked.

dmabupt commented 4 years ago

@dmabupt Thank you for the tips! I changed the script to set the STRING constant to 0 and everything worked. I tried inserting one row and multiple rows and it worked for both.

I also tried the new bindParameters() syntax as suggested. The two rows were inserted but suffered from the same issue where the string column was inserted as blank. Inserting one row worked.

Ah... Yes,

I changed the bindParameters() code to use STRING(0) by default on my test system. But I have not updated the code in this repo yet. I will add the patch in the next version.

Thanks for the reminder!

dmabupt commented 4 years ago

Commit 48d4963 fixed the bindParameters() issue. Close this issue for now.