wankdanker / node-odbc

ODBC bindings for node
MIT License
174 stars 79 forks source link

Bug with numeric values #60

Closed qlecler closed 5 years ago

qlecler commented 5 years ago

I don't know why the reported closed #223. However it's a bug of NodeJs odbc. Is using this code to set the type (probably if you don't have decimals it's using integers):

    else if (value->IsNumber()) {
      double *number   = new double(value->NumberValue());

      params[i].ValueType         = SQL_C_DOUBLE;
      params[i].ParameterType     = SQL_DECIMAL;
      params[i].ParameterValuePtr = number;
      params[i].BufferLength      = sizeof(double);
      params[i].StrLen_or_IndPtr  = params[i].BufferLength;
      params[i].DecimalDigits     = 7;
      params[i].ColumnSize        = sizeof(double);

      DEBUG_PRINTF("ODBC::GetParametersFromArray - IsNumber(): params[%i] c_type=%i type=%i buffer_length=%lli size=%lli length=%lli value=%f\n",
                    i, params[i].ValueType, params[i].ParameterType,
                    params[i].BufferLength, params[i].ColumnSize, params[i].StrLen_or_IndPtr,
                                *number);
    }

Then is using as

        ret = SQLBindParameter(
          hSTMT,                    //StatementHandle
          i + 1,                    //ParameterNumber
          SQL_PARAM_INPUT,          //InputOutputType
          prm.ValueType,
          prm.ParameterType,
          prm.ColumnSize,
          prm.DecimalDigits,
          prm.ParameterValuePtr,
          prm.BufferLength,
          &params[i].StrLen_or_IndPtr);

basically is passing 8 as precision and 7 as decimal that is a number like X.XXXXXXX. Correctly 33.44 is not accepted as 33 cannot fit in X. They probably want SQL_REAL instead of SQL_DOUBLE.

See https://github.com/FreeTDS/freetds/issues/285

qlecler commented 5 years ago

Hi there,

Any news on this ? It's huge problem !

markdirish commented 5 years ago

Hello! Sorry, I have been away at a conference and vacation for the last little bit, and haven't had a chance to work on this. I have read the freetds#285 thread and I think I have a sense for what is wrong. I will try to get something pushed out to odbc 2.0 branch today

qlecler commented 5 years ago

Hi !

Thanks, it would be very helpful 👍

markdirish commented 5 years ago

Hi @qlecler,

Sorry, I initially misunderstood

From the code posted, I assume you are using v1.x of the odbc package. I actually haven't worked much on this on. I rewrote it to a v2.0 that is currently in beta, that uses the ODBC function SQLDescribeCol, which gets a lot of the data about the columns so it doesn't have to be assumed like in:

params[i].DecimalDigits     = 7;
params[i].ColumnSize        = sizeof(double);

I can try to go back and write a patch for v1.x, but it might be fragile. Would you consider trying to download v2.0 beta with:

npm install odbc@beta

and let me know if the issue persists in the new version? When I get done with the backlog on v2.0 work, I can look to see if I can easily change the code causing the issue in v1.x, but when 2.0 is released soon I probably won't support 1.x anymore

qlecler commented 5 years ago

Hi markdirish,

I'm using the v1 for a project in production.

I can't really use the v2 beta except for testing purposes.

When do you expect to release a stable version of the v2?

Also, are there breaking changes between those two versions?

Thanks 👍

markdirish commented 5 years ago

v2.0 is out now, can be installed with

npm install odbc

The repo for 2.0 is at https://github.com/markdirish/node-odbc

There are many breaking changes: The API has been completely rewritten, but it should all be documented in the README (which should display on npm and on GitHub)