custom typeCast handlers in generated parser about node-mysql2

[danielsantiago]

Possible solution:

In mysql2/lib/compile_text_parser.js at line 12 replace to (and pass typeCast or the full connection configuration as parameter to the compile function):

if (typeof typeCast == "function") {
var value = typeCast.call(this, fields[i]);
else {
var value = readCodeFor(fields[i].columnType, fields[i].characterSet));
}

result.push(' this['+ srcEscape(fields[i].name) + '] = ' + value;

[sidorares]

Can you post very small self contained example with your use case? I'll try to add typecasting code to generated parser. Note that it has to be invoked at packet parsing time, not when parser is generated

[danielsantiago]

This is my custom typecasting function in felixge/node-mysql: https://gist.github.com/hlwebs/6373184

• I remove the DATETIME case, so now every date is treated as a string
• Line 48 I convert all the binary and varbinary columns to hexadecimal
• Line 52 is a custom application login to decrypt some strings

[sidorares]

This should be in application layer imho.
I'll think how to integrate this into parser generation. The whole point of generated parser is to avoid big switch as type is known before you start receiving rows (and if you have many of them they all have same types)

[danielsantiago]

The last two points could be in the application layer without any problem, but the datetime is a big problem for me because of the 2,000 calendars events that I retrieve in one query and I don't need to parse as a Date object.

In felixge/node-mysql the typeCast is executed when the parser is generated right?

[sidorares]

felixge/node-mysql does not generate parser as far as I know. There is one generic parser for any packet type.
If you are really focused on performance, we need some way to integrate you typeCast function into parser generator, e.i your function generates inline javascript source to be included inside compile_text_parser.js

[sidorares]

Note that I'm trying to do as little type conversions as possible by default. For date/time types string is returned because I receive it as string over network (in text protocol). Ints and floats converted to Number because it's faster to parse directly to number from buffer than convert buffer slice to string (and later parse string to Number)

[sidorares]

what if I add API like

connection.registerTypeParser(type, function(field) {
   // generate javascript here
});

your (simplified) example:

[Types.STRING, Types.VAR_STRING, Types.TINY_BLOB, Types.TINY_BLOB, Types.BLOB].forEach(function(type) {
   connection.registerTypeParser(type, function(field, context) {
      context.decrypt = crypt.decrypt;
      if (field.characterSet === Charsets.BINARY) {
        if (field.length > 16)
          return "decrypt(packet.readLengthCodedBuffer().toString('hex'));"
        else
          return "packet.readLengthCodedBuffer().toString('hex');"
      } else {
        return "packet.readLengthCodedString();"
      }
   });  
});

[danielsantiago]

I think this is a good idea and now I understand what do you mean by the "generated parser". With this new possible API, how are you planning to declare the default ones so they can be replaced. Example:

  case Types.DATETIME:
    return "new Date(packet.readLengthCodedString());";

to:

    return "packet.readLengthCodedString();";

Also I see another API incompatibility with felixge/node-mysql. The Type object is not public, but in the felixge code it's: https://github.com/felixge/node-mysql/blob/master/index.js (Line 23).

English is not my primary language, sorry if I'm not being clear enough

[sidorares]

it is public - https://github.com/sidorares/node-mysql2/blob/master/index.js (line 11)

I think user handlers should just override default. Not sure if we need to support stack of parsers ('next parser' reference). Maybe just an option to generate default parser conditionally from your code.

Also might worth to require generation of the whole column assignment - including ' this['+ srcEscape(fields[i].name) + '] = part. That way you can have user supplied name mangling etc

[sidorares]

answer to your question:

connection.registerTypeParser(Types.DATETIME, function(field, context) {
  return "packet.readLengthCodedString();"
});

if there is no 'parsers stack' maybe setTypeParser is better name (also need to have 2 versions for binary and text protocol. setTextRowTypeParser + setBinaryRowTypeParser?)

[danielsantiago]

setTextRowTypeParser and setBinaryRowTypeParser sounds good to me.

About the generation of the whole column assignment, I don't see any use case for this, if anyone need to change the column name, they can do it in the SQL with "column AS new-name".

[sidorares]

https://github.com/felixge/node-mysql#joins-with-overlapping-column-names currently not supported. Might be a use case for generation of the whole column assignment

[danielsantiago]

I don't follow you, you're going to take multiples JOINS (or FROMS) and treat them as column types? I thought that the parser generator was only for column types.

[sidorares]

with joins field.columnName might be not unique across columns. node-mysql allows you to prefix it with table name

[danielsantiago]

I understand this, but If someone need to replicate this feature with the type parser API you propose they are going to need to override every type.

I think this should be done in another place and not in the parser generator. Right now felixge/node-mysql is doing it after the type casting: https://github.com/felixge/node-mysql/blob/master/lib/protocol/packets/RowDataPacket.js#L30

[sidorares]

yes, you are right

[danielsantiago]

What do you think about create a new parent object for execute and query where we can move the getFieldsKey function (right now is the same code for both) and add the nestTables logic here too (I think is the same logic for binary and text). Also we can refactor some code from both objects to make them leaner.

[sidorares]

execute and query definitely need refactoring - they share a lot of common code. Yes, I want to move it to parent class.

[sidorares]

also, In execute command I actually have prepare + execute sequence. This was probably a bad idea, might be better to make 'composable' sequence and two separate commands. execute response itself is very similar to query response, the difference is in rows serialisation.

[sushantdhiman]

Implemented in #351