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

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

Merged
merged 4 commits into from
Sep 6, 2013
Merged

Conversation

LJ1102
Copy link
Contributor

@LJ1102 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
Copy link
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
Copy link
Contributor Author

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
Copy link
Member

durango commented Sep 6, 2013

1, Unit tests
2. Please :)

@LJ1102
Copy link
Contributor Author

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
Copy link
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
Copy link
Contributor Author

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
Add support for typed arrays in SqlString.escape and SqlString.arrayToList
@durango durango merged commit de91a8f into sequelize:master Sep 6, 2013
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 this pull request may close these issues.

2 participants