Query builder regression in 0.2

[vesse]

Issue type:

[ ] question
[X] bug report
[ ] feature request
[ ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[ ] mssql
[ ] mysql / mariadb
[ ] oracle
[X] postgres
[ ] sqlite
[ ] sqljs
[ ] react-native

TypeORM version:

[X] latest
[ ] @next
[X] 0.1.20 (or put your version here)

Steps to reproduce or a small repository showing the problem:

Query builder generated SQL changed after upgrade to 0.2 in a way that affects results. This

connection
  .getRepository(Model)
  .createQueryBuilder('model')
  .select('id')
  .whereInIds([ 1, 2 ])
  .andWhere('column = 1')
  .getMany());

produces

WHERE ("model"."id" = $1) OR ("model"."id" = $2) AND column = 1 -- PARAMETERS: [1,2]

in 0.2.5, and

WHERE ("model"."id"=$1 OR "model"."id"=$2) AND column = 1 -- PARAMETERS: [1,2]

in 0.1.20. Because of the parentheses over id conditions have changed, a different set of results is returned. If the order of the conditions is changed to

connection
  .getRepository(Model)
  .createQueryBuilder('model')
  .select('id')
  .where('column = 1')
  .andWhereInIds([ 1, 2 ])
  .getMany());

the generated SQL is

WHERE column = 1 AND (("model"."id" = $1) OR ("model"."id" = $2)) -- PARAMETERS: [1,2]

for 0.2.5 and

WHERE column = 1 AND ("model"."id"=$1 OR "model"."id"=$2) -- PARAMETERS: [1,2]

for 0.1.20, and both thus return the same results.

[pleerock]

but where is a bug? Everything looks correct in 0.2.x and it works as expected. I would say that in 0.1.x it did not work as expected.

[vesse]

Maybe it's then a difference of opinion, but I don't see any practical use for the query that

.whereInIds([1, 2, 3])
.andWhere('x = 1')

produces.

Especially if we consider SQL IN query. Consider the following table:

db=# select * from items;
 id | x
----+---
  1 | 1
  2 | 1
  3 | NULL
  4 | NULL
(4 rows)

Now perform the query I would expect .whereInIds([3, 2]).andWhere('x = 1') to do:

db=# select * from items where id in (3, 2) and x = 1;
 id | x
----+---
  2 | 1
(1 row)

But, the query performed by TypeORM 0.2 is:

db=# select * from items where (id = 3) or (id = 2) and x = 1;
 id | x
----+---
  2 | 1
  3 | NULL
(2 rows)

To me this looks like a bug, but if it's not I can live with it since where('x = 1').andWhereInIds([3, 2]) performs the query I need:

db=# select * from items where x = 1 and ((id = 3) or (id = 2));
 id | x
----+---
  2 | 1
(1 row)

[pleerock]

okay, so if it does following query:

select * from items where (id = 3) or (id = 2) and x = 1;

then we shall make a fix to make it this way:

select * from items where ((id = 3) or (id = 2)) and x = 1;

[silentroach]

partially for this case it was fixed in #3431

[pleerock]

we shall add tests with the same use case as original author and check if they pass

[silentroach]

yep, I know how to fix it and will try to do this in few days :)

[silentroach]

also it is not just a postgres driver bug, but all

[silentroach]

@vesse it is fixed now in master, can you check it?

[pleerock]

Since tests are passing this issue should be resolved now. @silentroach thank you very much!