Rewrite call procedure

[markdirish]

This PR changes the procedure workflow from:

SQLProcedures -> SQLProcedureColumns -> SQLExecDirect

to

SQLPrepare -> SQLDescribeParam -> SQLBind -> SQLExecDirect

This will allow users to call a procedure with just the procedure name, even if that procedure exists in multiple schemas/libraries.

[brandonp42]

Will this allow CLOB parameters on procedure calls? See #355 for context

[brandonp42]

Hi Mark,

Just curious how close you are to releasing this change?

Thanks!
Brandon

[kadler]

Looks like it should be ok, at least for the IBM i Access driver, but we may need to do some extra work to enable automatic IPD support on other drivers and for drivers that don't support it, we still don't have a solution.

[kadler]

While the above does work, it's inefficient. Though it likely doesn't matter in the grand scheme of things.

char* buffer = parameterString;

for (int i = 0; i < data->parameterCount; i++) {
  buffer[0] = '?';
  buffer[1] = ',';
  buffer += 2;
}

if (data->parameterCount) {
  buffer[-1] = '\0';
}
else {
  buffer[0] = '\0';
}

Or using a C++ string would be even easier, since you could get rid of all the memory management:

string parameterString;
parameterString.reserve(parameterStringSize);

for (int i = 0; i < data->parameterCount; i++) {
  parameterString += "?,";
}

if (data->parameterCount) {
  // remove trailing comma
  parameterString.resize(parameterStringSize-1);
}

[kadler]

A lot of this code could be simplified by using a std::string as well. Could just reserve say 256 or 512 bytes which should be enough in the majority of cases for the whole CALL statement and then just use std::string appends to put it all together (or std::wstring with UNICODE). In the event that not enough space was reserved, the string will automatically resize itself and continue on and then it will automatically free it's memory when it destructs too.

So code is simpler and no manual memory management.

[kadler]

For an IPD, the field is set to SQL_PARAM_INPUT by default if the IPD is not automatically populated by the driver (the SQL_ATTR_ENABLE_AUTO_IPD statement attribute is SQL_FALSE). An application should set this field in the IPD for parameters that are not input parameters.

I don't know that we can rely on this for all drivers, unfortunately. Looks like it's supposed to be off by default, so it seems our driver is behaving contrary to spec: https://learn.microsoft.com/en-us/sql/odbc/reference/develop-app/automatic-population-of-the-ipd

[brandonp42]

Would it make sense to use the old code path for drivers that don't support IPD?

[kadler]

Given the problems we've had with it, I think it'd be a better idea to design a way to have the user specify the direction as-needed.

Maybe something like:

// IN, OUT, INOUT exported by the node-odbc package
params = [
    { data: 1, direction: IN },
    { data: undefined, direction: OUT },
    { data: 'hello', direction: INOUT },
}
const result = await connection.callProcedure(null, null, 'MY_PROC', params);

Not sure if it actually matters on OUT vs INOUT. If not, it could be simplified by just binding them both as INOUT and then the direction field could be changed to a boolean is_output. Although, now that I'm thinking about it could just bind everything as INOUT in that case and not even worry about it. But that all hinges on whether database drivers are ok with binding that way.