sidorares / node-mysql2

:zap: fast mysqljs/mysql compatible mysql driver for node.js
https://sidorares.github.io/node-mysql2/
MIT License
4.07k stars 618 forks source link

Type casting not working as in node-mysql #338

Closed techana closed 8 years ago

techana commented 8 years ago

The example for type-casting boolean values given in node-mysql documentation (https://github.com/mysqljs/mysql#type-casting) does not work with node-mysql2.

sidorares commented 8 years ago

thanks, I'll add it with next rc!

sidorares commented 8 years ago

https://github.com/sidorares/node-mysql2/tree/custom-type-cast

In my opinion, there is more harm from having custom typeCast option than benefits it gets to you. @dougwilson - what do you think about deprecating typeCast (custom function) ? Or maybe change api so that instead of interacting with parser you just do what you want with raw data (that is already allocated by parser)?

   // rawValue -> parser.readLengthCodedBuffer()
   function typeCast(field, rawValue, next) {
      if (field.type == 'JSON') {
        return JSON.parse(rawValue.toString('utf8'));
      }
      next();
   }
sidorares commented 8 years ago

@techana published as v1.0.0-rc.7

chrisveness commented 8 years ago

@sidorares if you're not keen on typeCast, is there an alternative / better way of casting TINY(1) to boolean? Or have you decided to stick with typeCast?

If typeCast is the way to go, could I also ask – I use something like the following to set up a connection (in Koa):

const mysql = require('mysql-co');
const db = { host: 'localhost', database: 'db', user: 'usr', password: 'pw' };
const pool = mysql.createPool(db);
app.use(function*(next) {
    this.db = yield pool.getConnection();
    yield this.db.query('SET SESSION sql_mode = "TRADITIONAL"');
    yield next;
    this.db.release();
});

Where/how would I pass a typeCast function? (I'd prefer to do it on connection configuration than on every query()).

Thanks

sidorares commented 8 years ago

@chrisveness typeCast is supported now ( only text protocol, see https://github.com/sidorares/node-mysql2/blob/313bd02d984ae9a07d82eeaacda92b14c666e5e5/lib/compile_text_parser.js#L73 ) but I'd rather add simple .map() call on result rather using typeCast way

sidorares commented 8 years ago

If you don't want to do on every query you could wrap with your own version of query which looks at rows and fields and transforms all rows where field type is TINY into boolean

sidorares commented 8 years ago

something like this:

var query = (conn, sql, cb) => {
  conn.query(sql, (err, rows, fields) => {
    if (err) return cb(err);
    var myRows = rows.map( row => row.map( (column, index) =>  {
       if (field[index].type == Types.TINY) {
          return column === 1;
       } else {
          return column;
       }
    }) );
    cb(null, myRows);
  })
});
sidorares commented 8 years ago

oops, this is wrong, I rows is array of objects, not array of arrays. One minute, I'll make another version

sidorares commented 8 years ago

@chrisveness this one:

var query = (conn, sql, cb) => {
  conn.query(sql, function(err, rows, fields) {
    if (err) {
      cb(err);
      return;
    }
    var res = rows.map( row => {
      var typeCastRow = {};
      fields.forEach( f => {
        var name = f.name
        var value = row[name];
        if (f.columnType == mysql.Types.TINY) {
          typeCastRow[name] = value == 1;
        } else {
          typeCastRow[name] = value;
        }
      });
      return typeCastRow;
    });
    cb(null, res, fields);
  });
};
chrisveness commented 8 years ago

Thanks so much for such a prompt response.

I've adapted your suggestion slightly (don't know if it's better or worse!), to attach the alternate query() to the connection, and also to cater for nulls:

const mysql = require('mysql-co');
const db = { host: 'localhost', database: 'db', user: 'usr', password: 'pw' };
const pool = mysql.createPool(db);
app.use(function*(next) {
    db = yield pool.getConnection();
    db.queryCastBool = function*(options, callback) {
        const [rows, fields] = yield db.query(options, callback);
        const rowsCast = rows.map(row => {
            fields.forEach(field => {
                const bool = field.columnType==0x01 && field.columnLength==1;
                if (bool) row[field.name] = row[field.name]===null ? null : row[field.name]==1;
            });
            return row;
        });
        return [rows, fields];
    };
    yield next;
    db.release();
});

Which then gets invoked off the connection, paralleling the standard query():

const [rows,fields] = yield db.queryCastBool('Select * From MyTable Where Id = ?', id);

Do you think there might be merit, in v1.0.0, to cast TINYINT(1) to true/false as default?

sidorares commented 8 years ago

TINYINT is one byte value can be anything 0..255 for unsigned version and -128 to +127 for signed

chrisveness commented 8 years ago

Doh! of course, TINYINT is one byte, and TINYINT(1) only displays one digit (e.g. 0..9). For some reason, the fact that BOOLEAN is a synonym for TINYINT(1) set me thinking the (1) referred to 1 bit. I really should know these things (slap-wrist!).

For boolean values, would BIT(1) make more sense than TINYINT(1) (ignoring the standard BOOLEAN synonym)? And if so, would it make sense to map BIT(1) to true/false as a default?

sidorares commented 8 years ago

I would prefer to keep this library a thin layer on top of mysql protocol, but don't mind most common utilities to be included. Default behaviour should be "what you get is what server sends you" (but wrapped in a nice js form). If BIT(1) is used exclusively as boolean flag - yes, maybe, but I need to think if that might break any other code

chrisveness commented 8 years ago

I agree entirely – I'll leave that to your greater experience and better judgement.

chrisveness commented 8 years ago

Just a quick FYI: having been through my code, I've found the impact is less than I had feared.

For regular web pages, where specific fields are being handled, JavaScript is perfectly good at casting 1/0 to true/false, so it's not an issue.

The problem was more with an API, which was simply passing through all fields in the table. In this case, I wanted to convert boolean 1/0/Null to fld:true/fld:false/fld:null (for JSON; <fld>true</fld>, <fld>false</fld>, <fld xsi:nil="true"></fld> for XML).

For these purposes, the db.queryCastBool() above works well for me (alongside a companion function for the inverse insert/update casts).

So I'm quite happy if you prefer to stick with treating BIT(1) as numeric rather than true/false.

Long journey to end up where we started (as far as changes to mysql2 are concerned). Thanks for your help for changes I needed to make!

igortas commented 5 years ago

@chrisveness this one:

var query = (conn, sql, cb) => {
  conn.query(sql, function(err, rows, fields) {
    if (err) {
      cb(err);
      return;
    }
    var res = rows.map( row => {
      var typeCastRow = {};
      fields.forEach( f => {
        var name = f.name
        var value = row[name];
        if (f.columnType == mysql.Types.TINY) {
          typeCastRow[name] = value == 1;
        } else {
          typeCastRow[name] = value;
        }
      });
      return typeCastRow;
    });
    cb(null, res, fields);
  });
};

How I can use this example when I use async await? I'm having difficulties with casting bit from db to boolean. f,columnType is not recognized per object and now there are some other keys in the object which all of them are decimal numbers or hex, not sure.

catalog: 'def',
  schema: 'b2s_local',
  name: 'user_isadmin',
  orgName: 'user_isadmin',
  table: 'tb_user',
  orgTable: 'tb_user',
  characterSet: 63,
  columnLength: 1,
  columnType: 16,
  flags: 33,
  decimals: 0 
sidorares commented 5 years ago

@igortas your example would look like this:

var query = async (conn, sql, cb) => {
  const [rows, fields] = await conn.query(sql);  
  return rows.map( row => {
      var typeCastRow = {};
      fields.forEach( f => {
        var name = f.name
        var value = row[name];
        if (f.columnType == mysql.Types.TINY) {
          typeCastRow[name] = value == 1;
        } else {
          typeCastRow[name] = value;
        }
      });
      return typeCastRow;
    });
  });
};

have you trued connection level option for this? https://github.com/mysqljs/mysql#type-casting

const conn = await mysql.createConnection({
  //...
    typeCast: (field, next) => {
      if (field.type == 'TINY' && field.length == 1) {
        return (field.string() == '1'); // 1 = true, 0 = false
      }
      return next();
    }
});
igortas commented 5 years ago

@igortas your example would look like this:

var query = async (conn, sql, cb) => {
  const [rows, fields] = await conn.query(sql);  
  return rows.map( row => {
      var typeCastRow = {};
      fields.forEach( f => {
        var name = f.name
        var value = row[name];
        if (f.columnType == mysql.Types.TINY) {
          typeCastRow[name] = value == 1;
        } else {
          typeCastRow[name] = value;
        }
      });
      return typeCastRow;
    });
  });
};

have you trued connection level option for this? https://github.com/mysqljs/mysql#type-casting

const conn = await mysql.createConnection({
  //...
    typeCast: (field, next) => {
      if (field.type == 'TINY' && field.length == 1) {
        return (field.string() == '1'); // 1 = true, 0 = false
      }
      return next();
    }
});

Tnx for the info, yest I was playing with that solution, and here is how I do it, btw i have to check for BIT, not tiny, cuz everywhere i have in db BIT for boolean...

 typeCast: (field, next) => {
        if (field.type === "BIT") {

          const bytes = field.buffer();

          if (bytes === null || bytes === undefined) {
            return null;
          }

          return !!bytes.readUIntLE(); // the function return to integer and then the double bang return to boolean
        }
        return next();
      }
sidorares commented 5 years ago

@igortas if you use .query() result is a string, so field.string() should give you "0" or "1" ( same for field.buffer()

igortas commented 5 years ago

@igortas if you use .query() result is a string, so field.string() should give you "0" or "1" ( same for field.buffer()

Yes, I'm using query, but when i'm using field.string I'm getting true all the time and file.buffer just returning <Buffer 00> or <Buffer 01>. So that's why still using the old way with typecast.

sidorares commented 5 years ago

file.buffer just returning <Buffer 00> or <Buffer 01>

in that case you just read first byte with [0] and convert to true / false:

if (field.type === "BIT") {
  return field.buffer()[0] == 1;
}
return next()