Add support for --hex-blob option in mysqldump

[vdechef]

Fix for https://github.com/dumblob/mysql2sqlite/issues/69

It should also solve https://github.com/dumblob/mysql2sqlite/issues/54

[vdechef]

Thanks for the PR. It look good to me. Please try to address my comments or discuss them and then I'll merge it 😉.

OK, I will fix this later this week. I will also try to improve unit tests.

[dumblob]

@vdechef take your time (mine is also a "scarcity").

[vdechef]

I think I'm done.

• I updated the code with your remarks, and added the comments you asked for
• I replaced BIT with BLOB (but I did not remove the function used to handle conversion, just in case ... )
• I also fixed a bug on ENUM regex, and cleaned some \ escaping as you told me to
• I added some unit tests, in a way that it should be easy to extend (this can be discussed actually)

[vdechef]

So I just changed the unit tests to match what you wanted.

I also took one of the examples you had written, just to be sure that it would work with the helpers functions I added. And thanks to this I realized that the BIT type default values needed hexstring conversion. So I wrote a function for this and it seems like it is fixed now.

[vdechef]

While testing a Node.js replacement for mysqldump, I realized that the regex used to handle VALUES(...) statement could easily be improved to be compatible with this library. So I made a small patch.

@dumblob I know you don't have much time, but do you think this PR has a chance to be accepted someday ?

[dumblob]

While testing a Node.js replacement for mysqldump, I realized that the regex used to handle VALUES(...) statement could easily be improved to be compatible with this library. So I made a small patch.

It's fine, the difference is negligible.

@dumblob I know you don't have much time, but do you think this PR has a chance to be accepted someday ?

I'm sorry I'm really out of time now, but skimming the PR it looks very good - it's a lot of hard work and I'm glad you devoted so much time and effort to this. To answer your question - yes, I definitely want to merge this PR!

One minor nit (I repeat I didn't read the changes thoroughly, so there might be other things once I'll find some more time) would be my preference to not "rename" utilities (with "rename" I'm referring to storing their names in variables and subsequently using these variables). Any good reasons for that? If we really needed to "shadow" a real utility (e.g. to increase cross-platform support or speed or whatever), then I'd definitely rather introduce a differently named function as a wrapper with an easy interface (e.g. fixed number of args) to make the need explicit.

[vdechef]

To answer your question - yes, I definitely want to merge this PR!

Nice, I will continue to improve it when needed.

Any good reasons for that? If we really needed to "shadow" a real utility (e.g. to increase cross-platform support or speed or whatever), then I'd definitely rather introduce a differently named function as a wrapper with an easy interface (e.g. fixed number of args) to make the need explicit.

I totally agree. I just tried to mimic the style of existing code, but I didn't feel comfortable with this because the args of those utilities prevent their replacement without code refactoring.
I just updated the code to remove those declarations.

[vdechef]

Hi @dumblob, More than 3 years later, I rebased my PR and updated it with a CI file that will run the unit tests on Pull-Requests. Feel free to review/merge this.

[vdechef]

I added a few more test cases. Thanks to this I found that:

1. hex integers longer than 16 characters are not truncated anymore (an error is raised when creating the database)
2. bit fields set with integer values are not correctly parsed (we get ASCII instead of int)

Honestly I don't have enough energy to investigate this

[vdechef]

I just realized that the previous points were invalid:

1. by digging in our old discussions, it appear that we stopped truncating hex integers, because this causes dataloss
2. bit-fields are parsed correctly as long as one does not omit the --hex-blob option during mysqldump operation, as stated in the README

So I added a few more tests and pushed the code.

For the remaining test cases, I don't understand what they are supposed to test so I cannot implement them.