Route JPAInsertClause.execute() through native SQL when values contain function templates (#1757)#1758
Conversation
…n function templates (OpenFeign#1757) Detect TemplateExpression values via a shared hasTemplateValue() helper and route execute() to JpaNativeInsertSerializer + JDBC executeUpdate when present, falling back to the existing JPQL path otherwise. Same change applied to HibernateInsertClause. Adds executeUpdate(Connection, String, Object[]) to JpaInsertNativeHelper and regression tests covering both routing branches. Closes OpenFeign#1757
velo
left a comment
There was a problem hiding this comment.
Thanks for the fix and tests. I’m requesting changes due to a backwards-compatibility concern.
Gate results:
- Test coverage: PASS (new tests added for template and non-template execute paths in both JPA and Hibernate variants).
- Backwards compatibility: FAIL
- Security: PASS (uses prepared statements with bound parameters; no obvious injection risk introduced).
Blocking issue (backwards compatibility):
- In querydsl-jpa JPAInsertClause.execute(), when hasTemplateValue() is true, the new implementation unconditionally unwraps org.hibernate.Session and calls doReturningWork(...).
This hard-requires Hibernate at runtime for that code path. querydsl-jpa is used with non-Hibernate JPA providers (e.g., EclipseLink/OpenJPA), so this can fail at runtime (unwrap failure) where previously execute() stayed provider-neutral.
Please add provider-safe handling, e.g.:
- guard unwrap with capability checks and provide a provider-neutral fallback, or
- fail fast with a clear, documented exception only for unsupported providers and preserve previous behavior otherwise.
Once this is addressed, I’m happy to re-review.
Use the JPA-standard EntityManager.createNativeQuery instead of unwrapping a Hibernate Session, so the native routing for INSERTs with function templates also works under EclipseLink/OpenJPA. Addresses the backwards- compatibility review on OpenFeign#1758. HibernateInsertClause is intentionally unchanged — its native path continues to use Session.doReturningWork since that class is Hibernate-specific by design.
|
Instead of guarding the unwrap, I switched
3238 tests pass locally on |
velo
left a comment
There was a problem hiding this comment.
Thanks for the update and for addressing the provider-compatibility concern. This now includes focused regression tests for both routing paths, preserves existing behavior for non-template inserts, and checks are green.
Summary
JPAInsertClause.execute()andHibernateInsertClause.execute()now route throughJpaNativeInsertSerializer(native SQL) when value expressions contain aTemplateExpression— typically a schema-qualified function call fromSQLExpressions.function/stringFunction/numberFunctionor a raw SQL fragment. Plain path/literal/parameter INSERTs continue to use the existingJPQLSerializerpath so JPA semantics (cascade behaviour for paths, callbacks where applicable) are preserved.Aligns
execute()with the native routing thatexecuteWithKey() / executeWithKeys()(#1693) already use, so callers no longer need to invoke a key-returning method purely to dodge the JPQL parser when their values contain function templates.Fixes #1757.
Changes
JPAInsertClause.execute()— detect template values via a newhasTemplateValue()helper and dispatch to native path when present, falling back to JPQL otherwise.HibernateInsertClause.execute()— same change applied withSession.doReturningWork(...)instead ofEntityManager.unwrap.JpaInsertNativeHelper— addedexecuteUpdate(Connection, String, Object[])for the non-key-returning native path. Class javadoc generalized.JPAExecuteWithKeyTestandHibernateExecuteWithKeyTest:execute_with_function_template_routes_through_native_sql— verifies thatupper({0})inside a value expression is preserved and the function is evaluated by the DB.execute_without_template_uses_jpql_path— verifies plain INSERTs still go through JPQL.Test plan
./mvnw -Pdev -pl querydsl-libraries/querydsl-jpa -am test— 3238 tests, 0 failuresJPAExecuteWithKeyTest(13 tests, includes 2 newexecute_*cases)HibernateExecuteWithKeyTest(includes 2 newexecute_*cases)INSERTwith a schema-qualified external SQL function — pre-patch reproduced theSemanticExceptionfrom JPAInsertClause.execute() does not route to native SQL when values contain function templates / SQL fragments #1757; post-patch the INSERT succeeds via the native path.