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

feat: Add bindParameters() that accepts an array of values #111

Closed abmusse closed 4 years ago

abmusse commented 4 years ago

Is your feature request related to a problem? Please describe.

Currently whenever we want to bind a parameter we need to pass a 2D array to dbstmt.bindParam

statement.bindParam([
    [9997, IN, NUMERIC],
    ['Johnson', IN, CHAR],
    ['A J', IN, CHAR],
    ['453 Example', IN, CHAR],
    ['Fort', IN, CHAR],
  ]);

This works ok but i think we can simplify things a bit.

Describe the solution you'd like

I purpose we add another method dbstmt.bindParameters() to accept an array of values instead of array of arrays.

dbstmt.bindParameters([9997, 'Johnson', 'A J', 453 Example','Fort'])

We can use SQLDescribeParam to get back the type of the parameter.

In fact we already make the call here

I think we can set the inout to IN unless we are binding a parameter to a procedure.

In that case we can use SQLProcedureColumns to get a list of input and output parameters associated with a procedure.

Or maybe we could always set the inout type to SQL_PARAM_INPUT_OUTPUT.

That way the user does not have to specify the inout type or data type when using dbstmt.bindParameters.

dmabupt commented 4 years ago

This would be great but I still have some concerns according to the CLI doc --

Can we always use SQL_PARAM_INPUT_OUTPUT without problem?

kadler commented 4 years ago

@abmusse please fix your example to use bindParameters instead of bindParameter.

kadler commented 4 years ago

@dmabupt There should be no harm in always binding SQL_PARAM_INPUT_OUTPUT. Anything that's input only will just leave the buffer unmodified.

kadler commented 4 years ago

I think we could get by without making a new API. Instead, for each parameter passed, check whether an array or a value was passed in and act appropriately.

While I think we can get by without making a new API, I'd recommend we still do and just make bindParam a deprecated alias of it.

dmabupt commented 4 years ago

I think we could get by without making a new API. Instead, for each parameter passed, check whether an array or a value was passed in and act appropriately.

While I think we can get by without making a new API, I'd recommend we still do and just make bindParam a deprecated alias of it.

PR #112 is ready for your review.

BTW, the behavior has changed in the new API.

Previously we only get the out & in_out parameters output. But now the new API can not distinguish them, so all the parameters will be in the output.

For example, the most famous procedure -- call QXMLSERV.iPLUG512K(?,?,?,?) Its parameter list is --

[ipc, IN, CHAR],
[ctl, IN, CHAR],
[xmlIn, IN, CLOB],
[xmlOut, OUT, CLOB],

When we run execute() against it.

Previously the output array has ONLY 1 element - the last output parameter xmlOut. Now ALL the 4 parameters will be in the output array!

dmabupt commented 4 years ago

I think we could get by without making a new API. Instead, for each parameter passed, check whether an array or a value was passed in and act appropriately.

While I think we can get by without making a new API, I'd recommend we still do and just make bindParam a deprecated alias of it.

Hello @kadler , Seems the code of DbStmt::bindParams is too much now.

So I think maybe we should keep the old DbStmt::bindParams() untouched for best back-ward compatibility. And add a brand new C function DbStmt::bindParameters() only for the new JS API bindParameters().

JS API C API Parameters Output
bindParams() DbStmt::bindParams() 2-D array ONLY output params
bindParameters() DbStmt::bindParameters() 1-D array ALL params
bindParam() & bindParameters() BIG DbStmt::bindParams() 1-D/2-D array depends on input

Do you think it better? Or shall we keep using current BIG DbStmt::bindParams()?

dmabupt commented 4 years ago

PR https://github.com/IBM/nodejs-idb-connector/pull/112 has been merged. Close this.