Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide an option to disable JSON parsing #1072

Open
bunchjesse opened this issue Nov 27, 2019 · 13 comments · May be fixed by #2642
Open

Provide an option to disable JSON parsing #1072

bunchjesse opened this issue Nov 27, 2019 · 13 comments · May be fixed by #2642

Comments

@bunchjesse
Copy link

In both binary_parser.js and text_parser.js we're parsing the JSON string to a native object:

case Types.JSON:
  // Since for JSON columns mysql always returns charset 63 (BINARY),
  // we have to handle it according to JSON specs and use "utf8",
  // see https://github.com/sidorares/node-mysql2/issues/409
  return 'JSON.parse(packet.readLengthCodedString("utf8"))';

Similarly to the configuration option dateStrings, it would be nice to provide something like jsonStrings which would avoid this and just return the utf8 string.

@bunchjesse bunchjesse changed the title Provide an option to disable JSON type casting Provide an option to disable JSON parsing Nov 27, 2019
@sidorares
Copy link
Owner

curious what's your reason to have it? If it for perf reasons, or you want to use custom json deserialisation or something else?

@bunchjesse
Copy link
Author

I’m dumping the rows to a JSON file (for seeding the DB later) and this transformation causes JSON columns to be returned as objects rather than a string of JSON.

@sidorares
Copy link
Owner

sidorares commented Nov 27, 2019

gotcha. Right now you can do this via typeCast (docs)

I'm probably OK with adding jsonStrings ( similar to dateStrings )

@bunchjesse
Copy link
Author

bunchjesse commented Nov 27, 2019

Yeah, here is what I settled on to get unblocked (in case anyone else finds it helpful):

async function performWithTypeCastsDisabled(ctx: AppContext, fn: () => Promise<any>) {
  const oldTypeCast = ctx.db.client.connectionSettings.typeCast;
  const oldDateStrings = ctx.db.client.connectionSettings.dateStrings;
  // Provide a custom typeCast functionm that disables JSON parsing
  // See https://github.com/sidorares/node-mysql2/issues/1072
  ctx.db.client.connectionSettings.typeCast = function (field: any, next: any, packet: any) {
    if (field.type === "JSON") {
      return field.string();
    } else {
      return next();
    }
  }
  // Disable parsing to date object because this causes the format
  // to change and it isn't directly re-importable after.
  ctx.db.client.connectionSettings.dateStrings = true;
  await ctx.db.destroy();
  await ctx.db.initialize();
  await fn();
  ctx.db.client.connectionSettings.typeCast = oldTypeCast;
  ctx.db.client.connectionSettings.dateStrings = oldDateStrings;
  await ctx.db.destroy();
  return ctx.db.initialize();
}

@Diluka
Copy link

Diluka commented Mar 2, 2021

typeorm mysql driver will try to parse string data from json type. but pure string is json too. if stored string data in json column will crash in typeorm typeorm/typeorm#7445 (comment)

@OrkhanAlikhanov
Copy link

I need to disable this as well. It's for perf reasons. Also mysql package has this disabled (probably not implemented at all). Switching to mysql2 was not smooth for me because of this feature. I'd be happy to see option to disable this

@zachrip
Copy link

zachrip commented Apr 11, 2023

For those finding this issue - you should probably use field.buffer().toString('utf8') - in my case just using field.string() was breaking unicode characters.

@Ark42
Copy link

Ark42 commented Apr 13, 2023

For the reasons listed at #1287 I think adding jsonStrings would be super userful :)

@sidorares
Copy link
Owner

For the reasons listed at #1287 I think adding jsonStrings would be super userful :)

Agree, unfortunately would have to be true by default. In the hindsight I should have probably introduced this feature toggled off by default but too late to change now

@bacebu4
Copy link

bacebu4 commented Sep 21, 2023

Any updates on it?

@wlarch
Copy link

wlarch commented Apr 25, 2024

We have encountered the same issue while upgrading to MySQL 8 and using mysql2 driver with knex. We make use of @wwwouter/typed-knex to add a typing layer to our knex operations :

@Table('user')
class UserRow {
  @Column({ primary: true })
  id!: string;
  @Column()
  name!: string;
  @Column()
  metadata!: string; // this is a JSON column type
}

The metadata is parsed using JSON.parse() when we return our User type with a toEntity() function :

 private toEntity(row: UserRow): User {
    return {
      id: row.id,
      name: row.name,
      metadata: JSON.parse(metadata) as UserMetadata
   }
}

This will cause a "[object Object]" is not valid JSON error.

I was able to overcome this challenge by adding a typeCast() function and retrieve the behaviour we previously had using the mysqldriver :

connection: {
  user: config.user,
  password: config.password,
  host: config.host,
  port: config.port,
  database: config.database,
  timezone: '+00:00',
  dateStrings: true,
  charset: 'utf8mb4',
  typeCast: (field: TypeCastField, next: TypeCastNext) => {
    if (field.type === 'JSON') {
      return field.string('utf8');
    }
    return next();
  },
}

@rubenmorim
Copy link

rubenmorim commented Apr 30, 2024

Thanks @wlarch ! These approach worked for me also!

Any plans to add an setting to remove the auto json parse ?

We have encountered the same issue while upgrading to MySQL 8 and using mysql2 driver with knex. We make use of @wwwouter/typed-knex to add a typing layer to our knex operations :

@Table('user')
class UserRow {
  @Column({ primary: true })
  id!: string;
  @Column()
  name!: string;
  @Column()
  metadata!: string; // this is a JSON column type
}

The metadata is parsed using JSON.parse() when we return our User type with a toEntity() function :

 private toEntity(row: UserRow): User {
    return {
      id: row.id,
      name: row.name,
      metadata: JSON.parse(metadata) as UserMetadata
   }
}

This will cause a "[object Object]" is not valid JSON error.

I was able to overcome this challenge by adding a typeCast() function and retrieve the behaviour we previously had using the mysqldriver :

connection: {
  user: config.user,
  password: config.password,
  host: config.host,
  port: config.port,
  database: config.database,
  timezone: '+00:00',
  dateStrings: true,
  charset: 'utf8mb4',
  typeCast: (field: TypeCastField, next: TypeCastNext) => {
    if (field.type === 'JSON') {
      return field.string('utf8');
    }
    return next();
  },
}

@rubenmorim rubenmorim linked a pull request May 2, 2024 that will close this issue
@rubenmorim
Copy link

rubenmorim commented May 2, 2024

Hi guys,
i've added jsonStrings configuration can someone please review the PR @sidorares / @wellwelwel , this will allow us to decide if we want to parse it or just return as it is from mysql.

#2642

@wellwelwel wellwelwel linked a pull request May 2, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants