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

0.5.12 has a regression for bindings which are arrays #228

Closed
xaka opened this issue Mar 31, 2014 · 6 comments
Closed

0.5.12 has a regression for bindings which are arrays #228

xaka opened this issue Mar 31, 2014 · 6 comments

Comments

@xaka
Copy link
Contributor

@xaka xaka commented Mar 31, 2014

If you have a binding which is an array which is totally legal and supported by postgresql client, it gets flatten and leads to error like bind message supplies 5 parameters, but prepared statement "..." requires 3.

For instance if you have the following code:

knex.raw('select "stored_procedure"(?, ?, ?)', [1, 2, [a, b, c]])

You'll end up with the final bindings list as [1, 2, a, b, c] instead of [1, 2, [a, b, c]] and that makes postgresql server mad at you.

The regression is introduced by merging #219 into the master.

@tgriesser
Copy link
Member

@tgriesser tgriesser commented Apr 1, 2014

I see the issue, do you have a reproducible case of where it actually breaks... Not able to reproduce with the raw example (that example actually doesn't work currently, working on fixing that).

@xaka
Copy link
Contributor Author

@xaka xaka commented Apr 1, 2014

@tgriesser Sorry, I've fixed the example and now it's the exact copy of what I have in the code. I'm using PostgreSQL as a database and pg.js as a driver.

@ericeslinger
Copy link
Contributor

@ericeslinger ericeslinger commented Apr 1, 2014

I also had errors when trying to do a knex('table').insert({array_col: ['text', 'text'], normal_col: 'text'}, 'id') call. Here are some errors:

(using postgres driver)

When the array had two values (expertise was ['leverage', 'leverage']:

{ [Error: bind message supplies 6 parameters, but prepared statement "" requires 5, sql: insert into "profiles" ("created_at", "expertise", "image_url", "location", "nickname", "title", "updated_at") values (CURRENT_TIMESTAMP, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP) returning "id", bindings: leverage,leverage,https://s3.amazonaws.com/uifaces/faces/twitter/adammarsbar/128.jpg,Haagmouth Rhode Island,Carrie Larkin,Cross-group]
  sql: 'insert into "profiles" ("created_at", "expertise", "image_url", "location", "nickname", "title", "updated_at") values (CURRENT_TIMESTAMP, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP) returning "id"',
  bindings: 
   [ 'leverage',
     'leverage',
     'https://s3.amazonaws.com/uifaces/faces/twitter/adammarsbar/128.jpg',
     'Haagmouth Rhode Island',
     'Carrie Larkin',
     'Cross-group' ],

when the array had one value: obj.expertise = ['leverage']

{ [Error: array value must start with "{" or dimension information, sql: insert into "profiles"      ("created_at", "expertise", "image_url", "location", "nickname", "title", "updated_at") values (CURRENT_TIMESTAMP, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP) returning "id", bindings: orchestration,https://s3.amazonaws.com/uifaces/faces/twitter/lisovsky/128.jpg,Audreannemouth Alaska,Gay Padberg,Focused]
  sql: 'insert into "profiles" ("created_at", "expertise", "image_url", "location", "nickname", "title", "updated_at") values (CURRENT_TIMESTAMP, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP) returning "id"',
  bindings: 
   [ 'orchestration',
     'https://s3.amazonaws.com/uifaces/faces/twitter/lisovsky/128.jpg',
     'Audreannemouth Alaska',
     'Gay Padberg',
     'Focused' ],

I have for the time being just made my project rely on knex 0.5.9 as I don't have time right now to isolate and repro (we're in a big push here), but I'll try to get something more concrete Very Soon.

@point9repeating
Copy link

@point9repeating point9repeating commented Apr 3, 2014

I'm getting this error when executing a raw query with any bindings that include arrays. Here's a simplified example that no longer works in 0.5.12 but does work in 0.5.11

knex.raw('select * from table t where t.id = ANY( $1::int[] )', [[1, 2, 3]])
@tgriesser tgriesser closed this in cb9f849 Apr 3, 2014
@tgriesser
Copy link
Member

@tgriesser tgriesser commented Apr 3, 2014

Alright, sorry about all that, should be fixed in the latest patch release. Please let me know if you see anything else that might be broken. Thanks!

@point9repeating
Copy link

@point9repeating point9repeating commented Apr 3, 2014

Wonderful. It's working again as expected. Thank you!

elliotf pushed a commit to elliotf/knex that referenced this issue Nov 24, 2014
Fixed message for updates that don't affect rows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants