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

Comments

Projects
None yet
4 participants
@xaka
Contributor

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

This comment has been minimized.

Owner

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

This comment has been minimized.

Contributor

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

This comment has been minimized.

Contributor

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

This comment has been minimized.

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

This comment has been minimized.

Owner

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

This comment has been minimized.

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

Merge pull request tgriesser#228 from HeinrichFilter/patch-1
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