Skip to content

fix: Update HQL/JPQL queries for Hibernate 6 strict SQM parser compatibility#11

Open
devin-ai-integration[bot] wants to merge 2 commits intodevelop-7.0.xfrom
devin/ticket-1.3-hql-strict-parser
Open

fix: Update HQL/JPQL queries for Hibernate 6 strict SQM parser compatibility#11
devin-ai-integration[bot] wants to merge 2 commits intodevelop-7.0.xfrom
devin/ticket-1.3-hql-strict-parser

Conversation

@devin-ai-integration
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot commented Apr 17, 2026

Overview

Hibernate 6 introduces a stricter query parser (SQM — Semantic Query Model) that rejects several HQL/JPQL patterns that Hibernate 5 silently accepted. This PR audits and fixes all named queries and inline HQL across the entire BroadleafCommerce codebase to ensure compatibility.

Labels: Enhancement, Status: ready-for-code-review

Milestone: 7.0.x

Changes

1. Implicit joins → explicit JOINs (highest volume)

Hibernate 6 rejects multi-level implicit path navigation like entity.association.property. These are converted to explicit JOIN clauses.

Files: Product.orm.xml, Category.orm.xml, FulfillmentGroup.orm.xml, Solr.orm.xml, Page.orm.xml, CustomerAddress.orm.xml, CustomerPhone.orm.xml, Rating.orm.xml, Store.orm.xml, SandBoxDaoImpl.java, OfferAuditDaoImpl.java, AdminTestHelper.java

2. Interface → Impl entity class names in FROM clauses

Hibernate 6 requires the actual @Entity-annotated class (the Impl), not the interface, in FROM clauses.

Files: Sku.orm.xml, CategoryDaoImpl.java, OfferCodeDaoImpl.java, AdminNavigationDaoImpl.java

3. Missing alias prefix fix

CategoryXref.orm.xml had WHERE category.id = :categoryId — the alias categoryXref was missing. This was likely a latent bug that Hibernate 5 resolved implicitly.

4. Reserved keyword aliases & cast syntax

Order.orm.xml renames the order alias (SQL reserved keyword) and updates cast() type names for Hibernate 6.

Updates since last revision

  • Page.orm.xml: Changed LEFT JOIN page.pageTemplate to inner JOIN page.pageTemplate in BC_READ_PAGE_BY_URI. The original implicit navigation in Hibernate 5 produced an inner join, so using LEFT JOIN would have changed query semantics by returning pages without a template. (Thanks to Devin Review for catching this.)

⚠️ Key areas for reviewer attention

  1. JOIN vs LEFT JOIN selection — All implicit join conversions now use inner JOIN, except for Solr.orm.xml's LEFT JOIN subCat.defaultParentCategory defaultParent (where the default parent category is genuinely nullable). Please verify that all other converted associations are truly non-nullable — using inner JOIN on a nullable FK would silently filter out rows.

  2. OfferAuditDaoImpl.java — The OMS-conditional branch was refactored from LEFT JOIN OrderImpl o2 ON o.embeddedOmsOrder.parentOrder.id = o2.id to association-based navigation: LEFT JOIN o.embeddedOmsOrder embOms LEFT JOIN embOms.parentOrder o2. The WHERE clause was also simplified from o.embeddedOmsOrder.parentOrder.id IS NULL to o2.id IS NULL. This should be semantically equivalent but is the most complex change in the PR.

  3. SandBoxDaoImpl.java — Uses mixed syntax: an explicit JOIN sb.sandBox sandBox alongside a comma-separated cross join to AdminUserImpl. This preserves the original cross-join intent while fixing the implicit path navigation.

  4. Single-level entity.association.id patterns left as-is — Patterns like cp.customer.id, payment.order.id, and categoryXref.category.id were not changed because they resolve to FK columns and are supported in Hibernate 6. Worth a quick sanity check that none of these reference non-ID properties.

  5. No local build verification — Maven was not available in the dev environment, so these changes have not been compiled locally. CI will be the first build validation. All changes are query-string-only modifications with no Java logic changes.

Human review checklist

  • Verify that defaultSku, subCategory, phone, address, and locale associations are non-nullable (inner JOIN is correct)
  • Confirm OfferAuditDaoImpl OMS branch produces equivalent SQL
  • Confirm SandBoxDaoImpl mixed JOIN/cross-join syntax is valid in Hibernate 6
  • Spot-check that no entity.association.nonIdProperty patterns were missed

Additional context

All modules were audited: common, core/broadleaf-profile, core/broadleaf-framework, admin/broadleaf-open-admin-platform, admin/broadleaf-contentmanagement-module, integration. Native SQL queries (createNativeQuery) were reviewed and do not need changes. Criteria API queries were also reviewed and are unaffected.

Link to Devin session: https://app.devin.ai/sessions/984ad487258d4e17bb4c7273980b449d
Requested by: @Colhodm


Open with Devin

…ibility

- Add explicit JOINs to replace implicit multi-level path navigation
- Fix entity name mismatches (interface -> Impl class names)
- Add missing alias prefixes in WHERE clauses
- Rename reserved keyword aliases (e.g., 'order' -> 'ord')
- Fix cast() syntax for Hibernate 6 type names

ORM XML files fixed:
- Order.orm.xml: renamed 'order' alias, changed to Impl, fixed cast syntax
- Product.orm.xml: explicit JOINs for defaultSku paths
- Category.orm.xml: explicit JOINs for subCategory paths
- CategoryXref.orm.xml: fixed missing alias prefix
- Sku.orm.xml: changed interface to Impl
- FulfillmentGroup.orm.xml: explicit JOINs for order.auditable paths
- Solr.orm.xml: explicit JOINs for multi-level paths
- Page.orm.xml: explicit JOINs for locale and pageTemplate
- CustomerAddress.orm.xml: explicit JOINs for address paths
- Rating.orm.xml: explicit JOIN for ratingSummary.itemId
- Store.orm.xml: explicit JOIN for address path
- CustomerPhone.orm.xml: explicit JOINs for phone paths

Java DAO files fixed:
- CategoryDaoImpl.java: changed interface to Impl
- OfferCodeDaoImpl.java: changed interface to Impl
- OfferAuditDaoImpl.java: explicit JOINs for embeddedOmsOrder paths
- SandBoxDaoImpl.java: explicit JOIN for sandBox path
- AdminNavigationDaoImpl.java: changed interface to Impl
- AdminTestHelper.java: explicit JOIN for sandBox path

Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>
@devin-ai-integration
Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

devin-ai-integration[bot]

This comment was marked as resolved.

…nal inner join semantics

Devin Review correctly identified that LEFT JOIN changes query behavior -
pages without a template would now be returned, which the original implicit
join excluded.

Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>
Copy link
Copy Markdown
Author

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 7 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Double getResultList() call executes query twice instead of returning cached result

In readAdminSectionForClassName, the query result is fetched into result on line 140, checked for emptiness on line 141, but then a second q.getResultList() call is made on line 144 instead of returning the already-fetched result. This causes the JPA query to be executed a second time unnecessarily. While the query hint HINT_CACHEABLE may mitigate the performance cost via the query cache, this is clearly wrong — it should return result. Additionally, if the underlying data changed between the two calls (or if the query cache is not active), the second call could return different results than the first check, leading to inconsistent behavior. This is a pre-existing bug in the method being modified by this PR.

(Refers to line 144)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged — this is a valid observation, but as the comment notes, it's a pre-existing bug unrelated to the Hibernate 6 query migration. This PR is scoped to query string changes only, so I'll leave this for a separate fix to avoid scope creep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant