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

Comments

Projects
None yet
5 participants
@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

This comment has been minimized.

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

Merge branch '0.12.x'
* 0.12.x:
  release 0.12.4
  Updating/pruning some deps
  Fix #1733, #920, incorrect postgres array bindings
@megavoltua

This comment has been minimized.

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

This comment has been minimized.

Collaborator

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

This comment has been minimized.

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

This comment has been minimized.

Collaborator

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

This comment has been minimized.

megavoltua commented Nov 7, 2016

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

@TangMonk

This comment has been minimized.

TangMonk commented Apr 23, 2017

How to update array?

@elhigu

This comment has been minimized.

Collaborator

elhigu commented Apr 23, 2017

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

@TangMonk

This comment has been minimized.

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

This comment has been minimized.

Collaborator

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

This comment has been minimized.

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