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

Add support for typed arrays in SqlString.escape and SqlString.arrayToList #891

Merged
merged 4 commits into from Sep 6, 2013

Conversation

2 participants
@LJ1102
Contributor

LJ1102 commented Sep 6, 2013

When doing something like

setDataValue("position", new Float32Array([1.23, 4.2]))

Sequelize has stringified the typed array for database usage and by that broke the query.

With this commit typed arrays are handled like arrays, and can be saved to the database.

@durango

This comment has been minimized.

Member

durango commented Sep 6, 2013

This is a really awesome PR and a ton of thanks man :) I hate to be nitpicky, but two things:

  1. Could you add a test case for this specific PR / demonstrate the problem that you were experiencing
  2. Could you remove the semi-colons that aren't needed for the styleguide? :}

Hate to be like this :(

@LJ1102

This comment has been minimized.

Contributor

LJ1102 commented Sep 6, 2013

  1. You mean into the unit tests, or as an issue here on github?
  2. Sure I can do this, currently my code matches the overall style in the file, should i still convert it to your style?
@durango

This comment has been minimized.

Member

durango commented Sep 6, 2013

1, Unit tests
2. Please :)

@LJ1102

This comment has been minimized.

Contributor

LJ1102 commented Sep 6, 2013

There we go, i just converted the whole file to your style and added unit tests for typed array insertion and update.
Hope you like it :)

@durango

This comment has been minimized.

Member

durango commented Sep 6, 2013

Restarting a travis job and once it's green I'll merge, thanks for the hard work man! :)

@LJ1102

This comment has been minimized.

Contributor

LJ1102 commented Sep 6, 2013

I should rather thank you for your hard work 👍

durango added a commit that referenced this pull request Sep 6, 2013

Merge pull request #891 from LJ1102/master
Add support for typed arrays in SqlString.escape and SqlString.arrayToList

@durango durango merged commit de91a8f into sequelize:master Sep 6, 2013

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment