cognitom / momy

MongoDB to MySQL replication
https://www.npmjs.com/package/momy
MIT License
98 stars 37 forks source link

adding support for TEXT fields, remove non-standard ascii characters #2

Closed odedelharar closed 7 years ago

odedelharar commented 7 years ago

Added a very basic TEXT field support, while removing non-ascii characters from such a field. Updated the date and datetime fields to support string representations of unix time.

cognitom commented 7 years ago

@odedelharar thank you for your PR! LGTM about TEXT type. I like it 👍

I assumed that DATE and DATETIME type are string like 2016-11-07 basically. Could you change the code to pass such values?

The code could be like this:

convert: val => {
  if (!val) return 'NULL'
  if (typeof val == 'number') val = moment(val).format('YYYY-MM-DD')
  if (typeof val == 'string') {
    if (/\d+/.test(val)) val = moment(parseInt(val)).format('YYYY-MM-DD')
    if (!/\d{4}-\d{2}-\d{2}/.test(val)) return 'NULL'
  }
  return `"${ val }"`
}
cognitom commented 7 years ago

Could you pls add documentation about TEXT to here, too? Thanks.

odedelharar commented 7 years ago

Updated, now can allow: date format string (YYYY-MM-DD), unix time (number), unix time (string). Can make more options viable, maybe in a future change

cognitom commented 7 years ago

Great! Thank you.

odedelharar commented 7 years ago

Maybe add an expression like: /[\x00-\x1F\x7F\uFFFD\xEF\xBF\xBD]/g so it also covers the cases I saw

cognitom commented 7 years ago

@odedelharar unfortunately, it seems not a good idea. These chars like \xBF \xBD are actually valid letters in some languages.

odedelharar commented 7 years ago

OK, so maybe we can use /[\x00-\x1F\x7F\uFFFD]/g since \uFFFD (�) is not OK anyway.