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

Wrong Escaping of array in Postgres #1733

Closed
kayoub5 opened this issue Oct 10, 2016 · 11 comments
Closed

Wrong Escaping of array in Postgres #1733

kayoub5 opened this issue Oct 10, 2016 · 11 comments

Comments

@kayoub5
Copy link

@kayoub5 kayoub5 commented Oct 10, 2016

Latest version of knex (0.12.3) does not escape array of strings correctly:

var assert = require('assert');
var pg = require('knex')({client: 'pg'});

var sql = pg.raw('SELECT ?::text[]', [['foo', 'bar']]).toString();
console.log(sql); // "SELECT '{'foo','bar'}'::text[]"
assert.equal(sql, "SELECT '{''foo'', ''bar''}'::text[]");
@xizhao
Copy link

@xizhao xizhao commented Oct 11, 2016

+1, we're having this problem as well.

@tgriesser tgriesser closed this in 5890e4d Oct 12, 2016
tgriesser added a commit that referenced this issue Oct 12, 2016
* 0.12.x:
  release 0.12.4
  Updating/pruning some deps
  Fix #1733, #920, incorrect postgres array bindings
@megavoltua
Copy link

@megavoltua megavoltua commented Nov 7, 2016

Seems like array of strings is not applicable for WHERE IN:

knex.raw('SELECT * FROM docs WHERE type IN (?::text[])', [['account']]);

Produces

SELECT * FROM docs WHERE type IN ('{"account"}'::text[])

When running in Postgres 9.4:

Unhandled rejection error: SELECT * FROM docs WHERE type IN ($1::text[]) - operator does not exist: text = text[]

@elhigu
Copy link
Member

@elhigu elhigu commented Nov 7, 2016

@megavoltua what should be correct SQL in your case? Why are you using knex raw there instead of .whereIn ? Looks like you are putting an array inside of array, which would explain type error postgres is throwing...

knex('docs').whereIn('type', ['account']);
@megavoltua
Copy link

@megavoltua megavoltua commented Nov 7, 2016

@elhigu Yes, I know I can put .whereIn, but knex.raw is used widely in my current project and I must support it...

So I'm wondering, is there a way to use array binding for raw where in?

@elhigu
Copy link
Member

@elhigu elhigu commented Nov 7, 2016

@megavoltua knex doesn't know context of SQL string where replacement is being done, so array is always replaced with '{ "item" }' format.

I can think of two options if you really cannot avoid this situation (I have needed this maybe once during the last 3 years and probably it was unnecessary even then):

Unroll your string array to (?,?,?,?) in raw string and pass every item separately so you would end up with:

knex.raw('SELECT * FROM docs WHERE type IN (?)', ['account']);

or for bigger table something like:

knex.raw('SELECT * FROM docs WHERE type IN (?,?,?)', ['account','foo','bar']);

Or you can use textColumn = ANY(array) instead of where in which actually works for postgres array type:

knex.raw('SELECT * FROM docs WHERE type = ANY(?::text[])', [['account']]);
@megavoltua
Copy link

@megavoltua megavoltua commented Nov 7, 2016

@elhigu I think ANY(array) is the best solution here, I forgot about it 😄
Thanks!

RubenSlabbert added a commit to RubenSlabbert/knex that referenced this issue Nov 9, 2016
@TangMonk
Copy link

@TangMonk TangMonk commented Apr 23, 2017

How to update array?

@elhigu
Copy link
Member

@elhigu elhigu commented Apr 23, 2017

@TangMonk What is the raw SQL query you are trying to write to make your array update?

@TangMonk
Copy link

@TangMonk TangMonk commented Apr 24, 2017

@elhigu

like this sql:

update users set agents_array = ARRAY[1,2,3]::text[];

How to do this in knex?

@elhigu
Copy link
Member

@elhigu elhigu commented Apr 24, 2017

something like this:

knex('users').update({ 
  agents_array: knex.raw(`ARRAY[${arrayItems.map(() => '?').join(',')}]::text[]`, arrayItems)
}).then(res => console.log('results', res));
@TangMonk
Copy link

@TangMonk TangMonk commented Apr 24, 2017

@elhigu thank you!

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
5 participants