Conflict with Postgresql jsonb operators

[metainfa]

Hi!

I have sql:

SELECT id, name, price, files->'photos'->0->>'name' as photo, annotation
FROM product
WHERE tags ?& array['17']
ORDER BY (tags->>'17')::int
LIMIT 10

If i run this sql direct in postgresql client it work fine

If i run this sql through node-postgres it work fine again:

var pg = require('pg');

var connPg = "postgres://user:password@localhost/mydbname";
var clientPg = new pg.Client(connPg);
var sqlProductsByTags = [
  "SELECT id, name, price, files->'photos'->0->>'name' as photo, annotation ",
  "FROM product ",
  "WHERE tags ?& array['17'] ",
  "ORDER BY (tags->>'17')::int ",
  "LIMIT 10 "
];

clientPg.connect(function(err) {
  if(err) {
    return console.error('could not connect to postgres', err);
  }
  //console.log('SQL = ' + sqlProductsByTags.join(''));

  clientPg.query(sqlProductsByTags.join(''), function(err, result) {
    if(err) {
      return console.error('error select products by tags', err);
    }

    for(var i = 0, max = result.rows.length; i < max; i++) {
      console.log('name: ' +  result.rows[i].name);
    }

    clientPg.end();
  });
});

If i run raw sql by knex, it fail with error (below listed):

var connPg = "postgres://user:password@localhost/mydbname"; 
var pg = require('knex')({
  client: 'pg',
  connection: connPg
});
var sqlProductsByTags = [
  "SELECT id, name, price, files->'photos'->0->>'name' as photo, annotation ",
  "FROM product ",
  "WHERE tags ?& array['17'] ",
  "ORDER BY (tags->>'17')::int ",
  "LIMIT 10 "
];

pg.raw(sqlProductsByTags.join(''))

This Error

{ [error: syntax error (at or near "$1")]
  name: 'error',
  length: 142,
  severity: 'ERROR',
  code: '42601',
  detail: undefined,
  hint: undefined,
  position: '98',
  internalPosition: undefined,
  internalQuery: undefined,
  where: undefined,
  file: 'scan.l',
  line: '1053',
  routine: 'scanner_yyerror' }

I think that it fail because of i used jsonb operator '?&', precise only one symbol '?'
Reference to PostgreSQL JSONB operators

[vschoettke]

The problem is that the postgres dialect replaces every question mark with $ for the parameters bindings before executing the query in Client_PG.prototype.positionBindings.

The easiest fix is probably changing the regex to only replace occurrences of a question mark at the end or any followed by space, comma with $. @tgriesser any thoughts?

[tgriesser]

That seemed to be what I was thinking as well, though I'm not sure if the issue is complicated by the changes to raw which splits on ? to allow raw queries as binding values from #379

[vschoettke]

positionBindings is executed right before the query so no problem for the raw bindings.

[vschoettke]

/\?(?=$|[,\)\(\s])/g as regex in positionBindings function will only replace any ? followed by ,,(, ) or any whitespace and solve ?& problem.
However after reading the jsonb prostgres documentation I think this is futile because:

The default GIN operator class for jsonb supports queries with the @>, ?, ?& and ?| operators. (For details of the semantics that these operators implement, see Table 9-41.)

So a single question mark is also a valid operator.

Maybe escaping/replacing the question mark if no param was supplied and converting it back right before execution is a solution.

[tgriesser]

@vschoettke take a look at the PR I just opened and see if that's what you were thinking. It basically does what you described, allow for raw queries to have a ? operator so long as there are no bindings in the same raw query. Replaces them temporarily with $Q and then replaces them in the positionBindings.

[vschoettke]

@tgriesser your PR looks good to me. If somebody wants to use bindings and a question mark as is then $Q can be used in place of the question mark too. I'd prefer getting rid of the question mark for bindings in the future though. e.g. knex.raw('select * from "test" where id=:id", {id: 1});

[whitelynx]

One way of fixing this completely (and keep things future-proof for other dialects) would be to generate the SQL text as an array instead of a flat string, starting a new array element at the position of each binding.

For example, instead of:

{ method: 'select',
  options: undefined,
  bindings: [ 3857, 999999 ],
  sql: 'select "geom", ? as "srid" from "map" where ("id" = ?)' }

...we'd have:

{ method: 'select',
  options: undefined,
  bindings: [ 3857, 999999 ],
  sql: ['select "geom", ', ' as "srid" from "map" where ("id" = ', ')'] }

This means that each dialect would get the appropriate placeholders by just calling .join() with the placeholder (or, in the case where an incrementing number is needed in the placeholder, using .reduce()), and this wouldn't interfere with operators like ?.

[ghost]

Any updates on this issue?
will it reach the master ?
I'm also having several problems using postgresql jsob operators.
Thanks.

[elhigu]

+1 Is there any workarounds how I could make queries with json ?,?|,?& operators already with latest stable knex version?

[whitelynx]

I'm also curious whether this will be fixed soon. I'll need this before I can switch my generic web-based query tool over to using knex, since this would prevent the user from being able to use the JSONB operators.

@tgriesser Your PR looks fine for now, since it would provide a workaround for the time being. Eventually, I think it might make more sense to switch to something like I suggested above, but for now I think it'd be useful to at least get some fix merged.

[rhys-vdw]

Hi @whitelynx. I believe @tgriesser is currently away but I'm happy to get a fix merged. If someone who knows the code base is able to review and update the PR as necessary, I'm happy to facilitate by merging it into master.

[DenisGorbachev]

@tgriesser +1, I use JSONB all over the place :)

[underbluewaters]

This is also a problem with hstore methods.

    ELSIF hstore(NEW) ? 'is_bycatch' THEN

[zacronos]

This is also a problem when using knex.raw on a constant which includes a ?. Below is an oversimplified example that is easy to work around, but in my real situation I can't avoid the knex.raw().

Code:

console.log(knex.table('blah').insert({col1: knex.raw("'http://www.google.com/?q=foobar'"), col2: {a:1}}).toString());

SQL:

insert into "blah" ("col1", "col2") values ('http://www.google.com/'{\"a\":1}'q=foobar', ?)

[whitelynx]

@zacronos said:

This is also a problem when using knex.raw on a constant which includes a ?. Below is an oversimplified example that is easy to work around, but in my real situation I can't avoid the knex.raw().

I believe the correct way to do that with the current version of knex is to provide it as a parameter (then it is automatically escaped correctly, as well as avoiding this issue):

console.log(knex.table('blah').insert({col1: knex.raw('?', [' http://www.google.com/?q=foobar']), col2: {a:1}}).toString()); 

[zacronos]

@whitelynx, ah, that does seem to work. Thanks!

[eknkc]

This has been a big issue for us. Have recently started to use jsonb columns. Any update on the PR?

[rhys-vdw]

@eknkc There is an existing PR. Not sure what the hold up is.

[rhys-vdw]

Actually looks like it's passing, but I don't know the code base well enough to review it.

[zacronos]

@rhys-vdw, the PR passes tests, but it has merge conflicts.

I took a look myself earlier to see what it would take, and unfortunately the code involved has changed quite a bit since the PR was created. I believe at least one of the modified files no longer even exists. It's possible it would be easier for someone to create a new PR from scratch rather than try to merge the existing one, but I don't know the codebase, so I can't say for sure.

[eknkc]

The JDBC driver seems to have solved this using ?? as an escape for literal ?: pgjdbc/pgjdbc#227
One could write this in that case

knex.raw("SELECT * FROM items WHERE tags ?? ?", ["tagname"])

for

SELECT * FROM items WHERE tags ? 'tagname'

[underbluewaters]

I would expect a more general purpose escaping scheme like \? or ?? would be better than just special-casing question mark operators.

[elhigu]

+1 for ??

[elhigu]

Except, looks like ?? is already used for wrapping binding value to be used as an identifier e.g.

raw("??", ['TableName.colName'] => "TableName"."colName"
raw("?", ['TableName.colName'] => 'TableName.colName'

https://github.com/tgriesser/knex/blob/master/test/tape/raw.js#L12

[rhys-vdw]

Good catch, @elhigu.

Perhaps \? would be more appropriate then?

[elhigu]

Looks like we need to do \? actually with \\? since javascript requires that we escape \ char in string. However I added pull request with implementation, test and documentation. Should I run also npm run build and add commit the generated code as well?

[jamoy]

I know it's been a couple of months but I am still in this predicament on 0.9

can anyone let me know what is going on with this query?

qb.whereRaw("tags \\?& array['" + Array(tags.length + 1).join("?").split("").join("','") + "']", tags)

the error i get is

select "id" from "tablename" where tags \$1 array['foo','bar'] and "locale" = $2 order by "created_at" desc limit $3 offset $4 - syntax error at or near "\"

It seems the escape sequence doesn't get applied in any case. I would appreciate it if anyone can give light to this.

[elhigu]

I'm getting same kind of results... looks like e.g. when one calls .toString() for query builder, it cannot handle //? escaping correctly.

knex.from('test_table_two').whereRaw("(json_data->'me')::jsonb \\?& array['keyOne', 'keyTwo']").where('id', '>', 1).toString()

select * from "test_table_two" where (json_data->\'me\')::jsonb \\\'1\'& array[\'keyOne\', \'keyTwo\'] and "id" > ?

Normal case when one just executes query it goes a bit different route to pg and ? replacement is never done by knex, but they are just converted to $1, $2 etc bindings of prepared sql statement for postgres.

I'll write test for this case so that //? escaping is handled for .toString() replacements too.

[elhigu]

@jamoy I added pull request to fix your issue

[jamoy]

thank you very much @elhigu wish I knew the codebase enough to contribute. hoping this would get merged soon 👍

[zacronos]

@SeanCannon, in my codebase I saw this working just fine over a year ago after it was fixed. Chances are very good that if you're seeing a similar issue, it is a new problem, not a continuation of this problem.

I suggest you create a new Issue, complete with the version of knex you're using, example code that demonstrates the problem, and output of the error and/or expected vs actual results.

[qinwei-gong]

?? works!

[m00dy]

Be sure you have jsonb field but not json !!