Skip to content
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

Parameter cached as incorrect type for Boolean when null value is queried for first on field with AttributeConverter #3060

Closed
adrianhj opened this issue Jul 8, 2023 · 10 comments
Labels
for: external-project For an external project and not something we can fix

Comments

@adrianhj
Copy link

adrianhj commented Jul 8, 2023

Background

We are encountering the below issue as part of our upgrade from Spring Boot 2.7.13 to Spring Boot 3.1.1 whereby if we perform an initial query against a repository of an entity with a boolean -> Y/N converter using a null Boolean parameter for a where :param is null or x.field = :param clause, subsequent invocations with non-null values result in ORA-01722: invalid number due to what seems to be an incorrectly cached parameter type.

Existing code is behaving fine under Spring Boot 2.7.13, Hibernate 5.x.

I am unsure whether this is a Spring Data JPA issue or a Hibernate 6.x issue, however in my very basic reproduction using EntityManager.createQuery() within the repository linked to below the issue does not occur whereas with the JpaRepository backed one it does.

Steps to Reproduce the Problem

A repository re-producing the issue can be found here: adrianhj/spring-data-jpa-3060

We have a simple entity, specifying a boolean field with an AttributeConverter mapping to Y/N in the database:

@Entity
@Table(name = "test")
public class TestEntity {

    @Id
    private long id;

    @Column(name = "is_enabled")
    @Convert(converter = YesNoConverter.class)
    private boolean enabled;
    
    ...
    
}

This entity is backed by the following repository with a query short-circuiting on null values to optionally apply the filter:

@Repository
public interface TestEntityRepository extends JpaRepository<TestEntity, Long> {

    @Query("""
        select
            t
        from
            TestEntity t
        where
            :enabled is null
        or t.enabled = :enabled
    """)
    List<TestEntity> list(Boolean enabled);

}

Invoke the repository twice, first passing null, then passing true:

repository.list(null);
repository.list(true);

Expected Behaviour

  • Queries are successfully executed

Actual Behaviour

  • Second invocation using true fails with java.sql.SQLSyntaxErrorException: ORA-01722: invalid number

Specifications

  • Spring Data JPA 3.1.1
  • Oracle Database 19c/21c

Additional Information

For the re-producible example, the following snippets from the log may be of interest:

Generated SQL looks fine:

    select
        t1_0.id,
        t1_0.is_enabled 
    from
        test t1_0 
    where
        ? is null 
        or t1_0.is_enabled=?

When using a Spring Data JPA repository, it looks like when invoking with null first it is resolving the parameter to a BOOLEAN type at Hibernate level:

# repository.list(null)
2023-07-08T15:27:28.050+01:00 TRACE 58209 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [1] as [BOOLEAN] - [null]
2023-07-08T15:27:28.050+01:00 TRACE 58209 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [2] as [BOOLEAN] - [null]

This seems to be cached as the subsequent invocation with a non-null value then re-uses the type:

# repository.list(true)
2023-07-08T15:27:28.115+01:00 TRACE 58209 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [1] as [BOOLEAN] - [true]
2023-07-08T15:27:28.115+01:00 TRACE 58209 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [2] as [BOOLEAN] - [true]

Whereby Oracle falls over with ORA-01722: invalid number.

Reversing the order of invoking using a non-null value first the type seems to be correctly resolved and cached:

# repository.list(true)
2023-07-08T15:27:26.613+01:00 TRACE 58209 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [1] as [CHAR] - [Y]
2023-07-08T15:27:26.613+01:00 TRACE 58209 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [2] as [CHAR] - [Y]
...
# repository.list(null)
2023-07-08T15:27:26.670+01:00 TRACE 58209 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [1] as [CHAR] - [null]
2023-07-08T15:27:26.670+01:00 TRACE 58209 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [2] as [CHAR] - [null]

Utilising an EntityManager to run the same query via the same JPQL invoking with a null boolean, then a non-null boolean seem to behave correctly:

em.createQuery("""
    select
        t
    from
        TestEntity t
    where
        :enabled is null
    or t.enabled = :enabled
""").getResultList()
# query.setParameter("enabled", null).getResultList()
2023-07-08T15:27:24.864+01:00 TRACE 58209 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [1] as [CHAR] - [null]
2023-07-08T15:27:24.864+01:00 TRACE 58209 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [2] as [CHAR] - [null]
...
# query.setParameter("enabled", true).getResultList()
2023-07-08T15:27:24.944+01:00 TRACE 58209 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [1] as [CHAR] - [Y]
2023-07-08T15:27:24.944+01:00 TRACE 58209 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [2] as [CHAR] - [Y]
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 8, 2023
@mp911de
Copy link
Member

mp911de commented Jul 10, 2023

AttributeConverter are applied by Hibernate. Spring Data is built on top of JPA. If a JPA provider behaves incorrectly, then Spring Data cannot do much about it. Please report your bug against Hibernate.

@mp911de mp911de closed this as not planned Won't fix, can't repro, duplicate, stale Jul 10, 2023
@mp911de mp911de added for: external-project For an external project and not something we can fix and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 10, 2023
@adrianhj
Copy link
Author

@mp911de may not have been clear from the last sentence within the background—using the EntityManager directly behaves correctly from within the repo I shared (https://github.com/adrianhj/spring-data-jpa-3060/blob/main/src/test/java/com/example/jpa/JpaTests.java#L52), I am only seeing the issue when interacting through a Spring Data JPA repository.

Would you still suggest this is more suited within the Hibernate project under those circumstances?

@mp911de mp911de reopened this Jul 12, 2023
@mp911de mp911de added status: waiting-for-triage An issue we've not yet triaged for: external-project For an external project and not something we can fix and removed for: external-project For an external project and not something we can fix status: waiting-for-triage An issue we've not yet triaged labels Jul 12, 2023
@mp911de
Copy link
Member

mp911de commented Jul 12, 2023

The reproducer doesn't actually apply what Spring Data is doing. We use TypedParameterValue to represent null values as Hibernate requires in some arrangements further type hints to avoid JDBC binding failures (see #2370)

When I rewrite your reproducer to hand in TypedParameterValue first, then I can reproduce the problem you're seeing through Spring Data usage:

BasicTypeRegistry typeHelper = em.getEntityManagerFactory() //
		.unwrap(SessionFactoryImplementor.class) //
		.getTypeConfiguration() //
		.getBasicTypeRegistry();

BasicType<Boolean> registeredType = typeHelper.getRegisteredType(Boolean.class);

assertThatCode(() -> query.setParameter("enabled", new TypedParameterValue<>(registeredType, null)).getResultList()).doesNotThrowAnyException();
assertThatCode(() -> query.setParameter("enabled", true).getResultList()).doesNotThrowAnyException();

Would you still suggest this is more suited within the Hibernate project under those circumstances?

Yeah, I think that isn't a Spring Data bug.

@mp911de mp911de closed this as not planned Won't fix, can't repro, duplicate, stale Jul 12, 2023
@adrianhj
Copy link
Author

Thank you very much for the follow-up; I was just the stepping through HibernateJpaParametersParameterAccessor funnily enough!

I need to educate myself on the expectation around converters/behaviours in these scenarios.

Talking to members from the Hibernate team yesterday it sounded like the the caching of the binding that seems to be occuring for the second invocation was potentially unexpected so going to follow-up on that front.

@solita-kattelus
Copy link

Hi @adrianhj we have similar kind of problem. Is there another open bug on hibernate side or did you found out work around for this?

@mp911de
Copy link
Member

mp911de commented Aug 31, 2023

Thanks for the update. Feel free to drop a link here to the Hibernate ticket if you have one.

@adrianhj
Copy link
Author

I ended up discussing the issue with Gavin on the Hibernate side based on what Spring Data JPA was doing around the null handling with TypedParameterValue (as the type then fails to be recognised by the converter).

The main view was that the behaviour was incorrect Spring Data JPA side in its interaction with Hibernate (as passing null directly into the EntityManager works OK).

If I recall right from hunting around issue tracker here and Hibernate at the time the introduction of the behaviour was based on another issue seen with native queries and null handling where it was the recommended approach to ensure things were catered for properly when executing the query via those code paths.

Disabling plan caching removes the issue as each invocation is reparsed, but didn't feel right.

For the areas where we were affected we are dropping down to our own implementation and calling the EntityManager direct.

@mp911de
Copy link
Member

mp911de commented Aug 31, 2023

That's an interesting spin, that Spring Data does things supposedly wrong but disabling a Hibernate caching feature fixes the problem.

@adrianhj
Copy link
Author

https://hibernate.zulipchat.com/#narrow/stream/132096-hibernate-user/topic/.E2.9C.94.20Parameter.20binding.20as.20incorrect.20type.20on.20null.20with.20Converter/near/374576975 is the context if it is of interest.

As #2370 and its related Hibernate issues of https://hibernate.atlassian.net/browse/HHH-14411 and https://hibernate.atlassian.net/browse/HHH-14778 seem scoped to the native query path only, would constraining the mapping of null to a TypedParameterValue only in the case of native queries be a potential option (compromise?)?

@mp911de
Copy link
Member

mp911de commented Sep 1, 2023

Thanks a lot for the background. With the additional details it makes sense to constrain TypedParameterValue usage to native-queries only. I filed #3137 to follow up with that change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project For an external project and not something we can fix
Projects
None yet
Development

No branches or pull requests

4 participants