tediousjs / node-mssql

Microsoft SQL Server client for Node.js
https://tediousjs.github.io/node-mssql
MIT License
2.23k stars 466 forks source link

arrayRowMode returns a function for the column type #1622

Closed MarkFarmiloe closed 6 months ago

MarkFarmiloe commented 6 months ago

Expected behaviour:

Expected the data type to be returned in the columns array, but instead get a function returning the data type and extras.

Actual behaviour:

mssql.connect(config, function (err) {
    if (err) console.error("E1:", err);
    else {
        const request = new mssql.Request();
        request.arrayRowMode = true;
        request.query('SELECT * FROM test_table', (err, results) => {
            if (err) console.error("E2", err);
            console.log(results);
        });
    }
});

gives this columns array in the results: image

Configuration:

config.driver = 'mssql'

Software versions

MarkFarmiloe commented 6 months ago

The undefined fields probably shouldn't be getting returned either. All part of the same problem, I expect.

dhensby commented 6 months ago

This is all working as intended.

Do you have any specific questions as I'm not really sure what solution you're looking for here?

MarkFarmiloe commented 6 months ago

Displaying my lack of expertise with JS I expect. However I was using res.send(JSON.stringify(result)) to send this back to the client, which does not stringify the type field. My workround was to replace the type value with the type.declaration value which gives me the JS type. Not sure how I would get the '[sql.VarChar]' value though.

dhensby commented 6 months ago

The result certainly is not intended to be exposed to a front end in that way. The object doesn't implement a toJSON() method and it could contain all sorts of things that you wouldn't want exposed publicly.

It's never good practice just to dump backend data to the frontend without first sanitising it first. You'd want to whitelist and format the data in a predictable way for the frontend otherwise you aren't in control of your API contract and you're delegating that responsibility to this library instead.

MarkFarmiloe commented 6 months ago

I'm writing a generic read-only data viewer which will build the table based on column info, rows (returned and calculated). It will need the datatype to allow column searches to be smart. I will limit what is sent eventually, but I'm in dev mode at the moment 😁 Am I right in thinking that I need

result.columns[0][0].type[Symbol.for('nodejs.util.inspect.custom')]()

to get the '[sql.VarChar]' info out of the type?

dhensby commented 6 months ago

The right way to do it is to have a transformer that will map the internal DB type to a type that is useful in your frontend. You don't want this to be coupled in this way at all.

If I wanted to do this (which isn't how I'd approach it, I'd just use the native JS types in the frontend, no need for the backend to dictate types), I'd use a translation layer to map the internal DB types to a type the frontend will understand (ie: your API schema). A simple switch statement would do.

const colTypes = result.columns[0].map((col) => {
  switch (col.type) {
    case sql.Text:
      return 'string';
    default:
      return 'unknown';
  }
});