Skip to content

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