# JPA Criteria: Invalid SQL with IN predicate

JPA Criteria is awesome when you need to compose complex queries from multiple blocks, but it can cause a lot of troubles when the underlying SQL code fails to check some obvious scenarios.

## The concept

When dealing with Java `List`s, we can almost always assume (if written correctly) that code will work as expected with empty lists if we respect the contract (don't pass `null` as a parameter!).

Imagine the following code:

```java
private static final List<Person> persons = new ArrayList<>();

static {
	persons.add(new Person(1L));
	persons.add(new Person(2L));
	persons.add(new Person(3L));
}

public static void main(String[] args) {
	List<Person> personsQueryOne = findPersonsByIds(Arrays.asList(1L, 2L));
	System.out.println(personsQueryOne);
	List<Person> personsQueryTwo = findPersonsByIds(Arrays.asList());
	System.out.println(personsQueryTwo);
}

private static List<Person> findPersonsByIds(List<Long> idsList) {
	return persons.stream().filter(person -> idsList.contains(person.getId())).collect(Collectors.toList());
}
```

We have:

* A static list of persons;
    
* A method that searches the list of persons when provided with a list of ids.
    

This is the console output:

```plaintext
[Person [id=1], Person [id=2]]
[]
```

Nothing special, right? If we pass an empty list of ids, the method won't have anything to search for!

## The problem

Now, lets imagine the same kind of situation, but working with:

* JPA 2.1 with Criteria;
    
* Hibernate as JPA provider;
    
* Postgres as database.
    

The following query should search for the `Person` entity that matches a list of ids (but we are bad boys and provided an empty `List`):

```java
CriteriaBuilder criteriaBuilder = entityManager.getCriteriaBuilder();
CriteriaQuery<MyEntity> criteriaQuery = criteriaBuilder.createQuery(MyEntity.class);
Root<MyEntity> myEntity = criteriaQuery.from(MyEntity.class);
Predicate inPredicate = myEntity.in(new ArrayList<>()); // an empty arraylist!
criteriaQuery.where(inPredicate);
TypedQuery<MyEntity> typedQuery = entityManager.createQuery(criteriaQuery);
typedQuery.getResultList();
```

The call to `typedQuery.getResultList()` should return an empty resultset. The generated SQL is (attention to the where clause):

```sql
select
	person0_.id as id1_0_,
	person0_.name as name2_0_,
	person0_.type as type3_0_
from
	person person0_
where
	person0_.id in()
```

And the execution causes:

```plaintext
javax.persistence.PersistenceException: org.hibernate.exception.SQLGrammarException: could not extract ResultSet
...
Caused by: org.postgresql.util.PSQLException: ERROR: syntax error at or near ")"
  Posição: 127
	at org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2477)
	...
```

Indeed, the generated SQL is invalid. One of the approaches (and the most performatic one) would be to check for empty `List`s all the time, but we may end up with a lot of boilerplate code.

## Solution(s)

We can solve this problem with creative ways, and I came up with two, depending on the situation.

### #1 - Create your own IN method or utility

This one is easy: instead of using `Expression#in(Collection)`, we can create a method that will handle the empty collection case for us, returning a `Predicate`:

```java
private Predicate in(Expression<?> entityValue, Collection<?> valuesToMatch) {
	if (valuesToMatch == null || valuesToMatch.isEmpty()) {
		return falsyCondition();
	}
	return entityValue.in(valuesToMatch);
}

private Predicate falsyCondition() {
	CriteriaBuilder criteriaBuilder = entityManager.getCriteriaBuilder();
	return criteriaBuilder.isTrue(criteriaBuilder.literal(false));
}
```

What's happenning here: If `List.isEmpty() == true`, we return an condition that is **always** false. In this case: `true=false`.

Here's the relevant part using this code:

```java
Predicate inPredicate = in(personEntity, new ArrayList<>());
criteriaQuery.where(inPredicate);
List<Person> result = entityManager.createQuery(criteriaQuery).getResultList();
```

And here is the generated SQL (? is a placeholder for `false`):

```sql
select
	person0_.id as id1_0_,
	person0_.name as name2_0_,
	person0_.type as type3_0_
from
	person person0_
where
	?= true
```

No more exceptions!

### #2 - Decorator pattern

The solution #1 is great when you have something like a `AbstractRepository` or `AbstractDAO` that you can put the created reusable method. But when that's not an option, we can come up with a `Decorator` that will only be applied to the `Path` or `Expression` that we are checking for matches in the list.

First, a new class must be created:

```java
public class SafeInDecorator<T> implements Expression<T> {
	private CriteriaBuilder criteriaBuilder;
	private Expression<T> decorated;

	public SafeInDecorator(CriteriaBuilder criteriaBuilder, Expression<T> decorated) {
		this.criteriaBuilder = criteriaBuilder;
		this.decorated = decorated;
	}

	@Override
	public Predicate in(Collection<?> values) {
		if (values.isEmpty()) {
			return falsyCondition();
		}
		return decorated.in(values);
	}

	@Override
	public Predicate in(Object... values) {
		return in(Arrays.asList(values));
	}

	private Predicate falsyCondition() {
		return criteriaBuilder.isTrue(criteriaBuilder.literal(false));
	}

	@Override
	public Predicate isNull() {
		return decorated.isNull();
	}

  /** all other methods must be implemented delegating to the decorated object **/
}
```

Then we can decorate the entity `Path` object and use the `#in(Collection)` method that adheres to our contract:

```java
Root<Person> personEntity = criteriaQuery.from(Person.class);
Expression<Person> personDecorated = new SafeInDecorator<>(criteriaBuilder, personEntity);
Predicate inPredicate = personDecorated.in(new ArrayList<>());
```

No more exceptions!²

## Conclusion

In the end, you should pick the option that suits better your project's situation.

It would be nice to have the **#1** solution in an `AbstractRepository` where it can be reused in subclasses. But if this problem was discovered when that's not an option, the **#2** solution also solves the problem and is less intrusive to the class hierarchy.

Which one to choose is up to you!
