my2iu / Jinq

LINQ-style queries for Java 8
Other
660 stars 71 forks source link

OR ing together expressions that involve NULL doesn't work properly #17

Open my2iu opened 8 years ago

my2iu commented 8 years ago

Bien Ly Ngoc noted that:

stream.where(n -> n.getAccount().getId() == accoutId || n.getAccount() == null) ...

is being translated as the HQL

WHERE A.account.id = :param0 OR A.account.id <> :param1 AND A.account IS NULL

which gives the resulting SQL:

WHERE ACCOUNT_ID = 1 OR ACCOUNT_ID <> 1 AND ACCOUNT_ID IS NULL;

Which is not correct since when ACCOUNT_ID is NULL, ACCOUNT_ID <> 1 evaluates to UNKNOWN (ie FALSE)

my2iu commented 8 years ago

It turns out that JPQL doesn't support IS FALSE and IS TRUE, which is what would be needed to fix this issue properly and with minimal changes.

Instead, I'm going to need to do some sort of more extensive fix involving rewriting how expressions are handled. I don't really have the time to do that, so for now, I've implemented a partial mitigation. Jinq will now recognizing when you're ORing together a series of clauses, and it won't invert any of the conditions there, avoiding this problem of having clauses A OR NOT(A) appearing in the generated expression.

zapov commented 8 years ago

Do you have any plans on fixing this issue in a more general way?

I'm having hard time in JINQ with non-trivial queries since they are not translated correctly. I dug into code and saw that core issue is that where is result of sum of all paths in Java bytecode, which doesn't translate correctly into SQL. It doesn't seem likely that you can rewrite such queries after they are translated into expressions.

my2iu commented 8 years ago

No, not in the immediate future. Coming up with a general algorithm will require 2-3 months of full time work to figure out. I can't really justify putting in that amount on this project at the moment.

There are several ways to structure more complex queries and still get Jinq to generate reasonable output.

If you're ANDing lots of more complex expressions together, you can split these expressions into separate where() lambdas.

stream.where( house -> house.getColor() == Color.GREEN || house.getColor() == Color.BLUE)
   .where(house -> house.getPrice() < 200000)
   .where(house -> house.getCity().equals("London") || house.getCity().equals("Paris") )

If you're ORing things together, you should put your simplest ORs at the beginning because Jinq is able to optimize that case.

stream.where( car -> 
   car.getType() == Type.ELECTRIC
   || car.getType() == Type.HYBRID
   || (car.getType() == Type.GAS && car.getMpg() > 40))

If you have certain types of queries that you can't Jinq to express well, just send me the queries, and I can put in a couple of days tweaking Jinq to handle those types of queries.

In terms of implementing a general solution though, the amount of work involved exceeds what I can afford to put into a hobby project.