Order by expression missing in select list after update to Spring Boot 3.0.1 with DISTINCT queries

william00179 commented 1 year ago

Hi there,

Spring Boot: 3.0.1 Database: postgres 14

I am seeing an issue after updating from Spring Boot 2.7x to Spring Boot 3.0.1. I am raising this issue here as I think the issue lies within Spring Data JPA after some debugging, specifically in QueryUtils in converting a Sort to an Expression.

I provide the following Sort as a default for a query

Sort.by(Sort.Order.desc(Study_.DATE), Sort.Order.desc(Study_.TIME), Sort.Order.asc("patient.lastName"), Sort.Order.asc("patient.firstName"))

I pass this through to findAll(Specification, Sort) and the following query is generated

select 
  distinct s1_0.id, 
  s1_0.date, 
  s1_0.description, 
  s1_0.patient_id, 
  s1_0.size, 
  s1_0.study_instance_uid, 
  s1_0.time
from 
  patient.studies s1_0 
  join patient.patients p1_0 on p1_0.id = s1_0.patient_id 
where 
  s1_0.customer_id = ?
order by 
  s1_0.date desc, 
  s1_0.time desc, 
  p1_0.last_name asc, 
  p1_0.first_name asc offset ? rows fetch first ? rows only

Note that this query is DISTINCT. This query will fail with PSQLException: ERROR: for SELECT DISTINCT, ORDER BY expressions must appear in select list. If I remove the distinct and run this the query will execute successfully but will give the wrong results.

If I try to work around this by using an entity graph to fetch the patient record eagerly, Spring Data will not detect the join in QueryUtils and join the record again and then use that alias in the order by, once again running into the same issue.

select 
  distinct s1_0.id, 
  s1_0.date,  
  s1_0.description, 
  p2_0.id, 
  p2_0.attributes_id, 
  p2_0.created_at, 
  p2_0.customer_id, 
  p2_0.deceased, 
  p2_0.deceased_at, 
  p2_0.deleted_at, 
  p2_0.dob, 
  p2_0.first_name, 
  p2_0.last_name, 
  p2_0.middle_name, 
  p2_0.prefix, 
  p2_0.sex, 
  p2_0.suffix, 
  p2_0.updated_at, 
  s1_0.size, 
  s1_0.study_instance_uid, 
  s1_0.time
from 
  patient.studies s1_0 
  join patient.patients p1_0 on p1_0.id = s1_0.patient_id 
  left join patient.patients p2_0 on p2_0.id = s1_0.patient_id 
where 
  s1_0.customer_id = ? 
order by 
  s1_0.date desc, 
  s1_0.time desc, 
  p1_0.last_name asc, 
  p1_0.first_name asc offset ? rows fetch first ? rows only

I think this issue is likely caused by a combination of the new aliasing in Hibernate and the related changes made to QueryUtils about 8 months ago.

From searching through Hibernate bug tracker, there are a few reports of this issue going way back to early 5.x releases of Hibernate. eg https://hibernate.atlassian.net/browse/HHH-13434. The feedback from the maintainers is a fetch() should be used rather than a join in these cases.

gregturn commented 1 year ago

Could you include a link to a reproducer, staged on github, ideally using https://start.spring.io, H2 (or Testcontainers for PostGreSQL, etc.), and the version of Spring Boot aligned with your issue?

william00179 commented 1 year ago

Hi @gregturn

Please find a simple reproducer here https://github.com/william00179/spring-data-reproducer-2756

I think the issue may be specific to Postgres, other databases may be more lenient.

The problem can be fixed by making the following change to QueryUtils. This change requires passing a distinct flag into the utils from the query. Currently this method also doesn't consider fetches, it only checks joins. This cast appears to be safe. There may be a more graceful way to do it that doesn't add the unnecessary select for other databases that don't require it.

private static Join<?, ?> getOrCreateJoin(From<?, ?> from, String attribute, JoinType joinType, boolean distinct) {

        for (Fetch<?, ?> fetch : from.getFetches()) {
            if (fetch.getAttribute().getName().equals(attribute))
                return (Join<?, ?>) fetch;
        }

        if(!distinct)
        {
            for (Join<?, ?> join : from.getJoins()) {
                if (join.getAttribute().getName().equals(attribute)) {
                    return join;
                }
            }
        }

        return distinct ?
                (Join<?, ?>) from.fetch(attribute, joinType) : from.join(attribute, joinType);
    }

reda-alaoui commented 1 year ago

Hello,

We have also encoutered this issue. Should this be fixed on Hibernate or Spring Data side?

william00179 commented 1 year ago

I would think this would be a Hibernate issue but they have previously rejected it being fixed within Hibernate.

Its easily fixed within spring-data by using fetch rather than join conditionally.

reda-alaoui commented 1 year ago

@william00179 , according to https://github.com/spring-projects/spring-data-jpa/issues/2756#issue-1522135203 , the fix you suggest is not a solution for those who use EntityGraph, is it?

william00179 commented 1 year ago

Yes it will still work with an entity graph. We are currently using a forked version of the lib with this fix made.

I havent raised this as a PR as my fix isn't very efficient for other dialects that don't require this change and I don't know enough about the internals of Spring Data to acomplish that currently. That being said, with this change everything now works as expected as it did before the upgrade to Spring Boot 3 and Hibernate 6

Here are the changes that need to be made

diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryCreator.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryCreator.java
index 16074ed2..fe335e8b 100644
--- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryCreator.java
+++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryCreator.java
@@ -191,7 +191,7 @@ public class JpaQueryCreator extends AbstractQueryCreator<CriteriaQuery<? extend
            query = query.select((Root) root);
        }

-       CriteriaQuery<? extends Object> select = query.orderBy(QueryUtils.toOrders(sort, root, builder));
+       CriteriaQuery<? extends Object> select = query.orderBy(QueryUtils.toOrders(sort, root, builder, query.isDistinct()));
        return predicate == null ? select : select.where(predicate);
    }

@@ -380,7 +380,7 @@ public class JpaQueryCreator extends AbstractQueryCreator<CriteriaQuery<? extend
        }

        private <T> Expression<T> getTypedPath(Root<?> root, Part part) {
-           return toExpressionRecursively(root, part.getProperty());
+           return toExpressionRecursively(root, part.getProperty(), false);
        }

        private <T> Expression<T> traversePath(Path<?> root, PropertyPath path) {

diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java
index cdd49c62..bd27cd84 100644
--- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java
+++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java
@@ -70,856 +70,865 @@ import org.springframework.util.StringUtils;
  */
 public abstract class QueryUtils {

-   public static final String COUNT_QUERY_STRING = "select count(%s) from %s x";
-   public static final String DELETE_ALL_QUERY_STRING = "delete from %s x";
-   public static final String DELETE_ALL_QUERY_BY_ID_STRING = "delete from %s x where %s in :ids";
-
-   // Used Regex/Unicode categories (see https://www.unicode.org/reports/tr18/#General_Category_Property):
-   // Z Separator
-   // Cc Control
-   // Cf Format
-   // Punct Punctuation
-   private static final String IDENTIFIER = "[._$[\\P{Z}&&\\P{Cc}&&\\P{Cf}&&\\P{Punct}]]+";
-   static final String COLON_NO_DOUBLE_COLON = "(?<![:\\\\]):";
-   static final String IDENTIFIER_GROUP = String.format("(%s)", IDENTIFIER);
-
-   private static final String COUNT_REPLACEMENT_TEMPLATE = "select count(%s) $5$6$7";
-   private static final String SIMPLE_COUNT_VALUE = "$2";
-   private static final String COMPLEX_COUNT_VALUE = "$3 $6";
-   private static final String COMPLEX_COUNT_LAST_VALUE = "$6";
-   private static final String ORDER_BY_PART = "(?iu)\\s+order\\s+by\\s+.*";
-
-   private static final Pattern ALIAS_MATCH;
-   private static final Pattern COUNT_MATCH;
-   private static final Pattern STARTS_WITH_PAREN = Pattern.compile("^\\s*\\(");
-   private static final Pattern PARENS_TO_REMOVE = Pattern.compile("(\\(.*\\bfrom\\b[^)]+\\))",
-           CASE_INSENSITIVE | DOTALL | MULTILINE);
-   private static final Pattern PROJECTION_CLAUSE = Pattern.compile("select\\s+(?:distinct\\s+)?(.+)\\s+from",
-           Pattern.CASE_INSENSITIVE);
-
-   private static final Pattern NO_DIGITS = Pattern.compile("\\D+");
-
-   private static final String JOIN = "join\\s+(fetch\\s+)?" + IDENTIFIER + "\\s+(as\\s+)?" + IDENTIFIER_GROUP;
-   private static final Pattern JOIN_PATTERN = Pattern.compile(JOIN, Pattern.CASE_INSENSITIVE);
-
-   private static final String EQUALS_CONDITION_STRING = "%s.%s = :%s";
-   private static final Pattern ORDER_BY = Pattern.compile("(order\\s+by\\s+)", CASE_INSENSITIVE);
-   private static final Pattern ORDER_BY_IN_WINDOW_OR_SUBSELECT = Pattern
-           .compile("\\([\\s\\S]*order\\s+by\\s[\\s\\S]*\\)", CASE_INSENSITIVE);
-
-   private static final Pattern NAMED_PARAMETER = Pattern.compile(COLON_NO_DOUBLE_COLON + IDENTIFIER + "|#" + IDENTIFIER,
-           CASE_INSENSITIVE);
-
-   private static final Pattern CONSTRUCTOR_EXPRESSION;
-
-   private static final Map<PersistentAttributeType, Class<? extends Annotation>> ASSOCIATION_TYPES;
-
-   private static final int QUERY_JOIN_ALIAS_GROUP_INDEX = 3;
-   private static final int VARIABLE_NAME_GROUP_INDEX = 4;
-   private static final int COMPLEX_COUNT_FIRST_INDEX = 3;
-
-   private static final Pattern PUNCTATION_PATTERN = Pattern.compile(".*((?![._])[\\p{Punct}|\\s])");
-   private static final Pattern FUNCTION_PATTERN;
-   private static final Pattern FIELD_ALIAS_PATTERN;
-
-   private static final String UNSAFE_PROPERTY_REFERENCE = "Sort expression '%s' must only contain property references or "
-           + "aliases used in the select clause; If you really want to use something other than that for sorting, please use "
-           + "JpaSort.unsafe(ΓÇª)";
-
-   static {
-
-       StringBuilder builder = new StringBuilder();
-       builder.append("(?<=\\bfrom)"); // from as starting delimiter
-       builder.append("(?:\\s)+"); // at least one space separating
-       builder.append(IDENTIFIER_GROUP); // Entity name, can be qualified (any
-       builder.append("(?:\\sas)*"); // exclude possible "as" keyword
-       builder.append("(?:\\s)+"); // at least one space separating
-       builder.append("(?!(?:where|group\\s*by|order\\s*by))(\\w+)"); // the actual alias
-
-       ALIAS_MATCH = compile(builder.toString(), CASE_INSENSITIVE);
-
-       builder = new StringBuilder();
-       builder.append("\\s*");
-       builder.append("(select\\s+((distinct)?((?s).+?)?)\\s+)?(from\\s+");
-       builder.append(IDENTIFIER);
-       builder.append("(?:\\s+as)?\\s+)");
-       builder.append(IDENTIFIER_GROUP);
-       builder.append("(.*)");
-
-       COUNT_MATCH = compile(builder.toString(), CASE_INSENSITIVE | DOTALL);
-
-       Map<PersistentAttributeType, Class<? extends Annotation>> persistentAttributeTypes = new HashMap<>();
-       persistentAttributeTypes.put(ONE_TO_ONE, OneToOne.class);
-       persistentAttributeTypes.put(ONE_TO_MANY, null);
-       persistentAttributeTypes.put(MANY_TO_ONE, ManyToOne.class);
-       persistentAttributeTypes.put(MANY_TO_MANY, null);
-       persistentAttributeTypes.put(ELEMENT_COLLECTION, null);
-
-       ASSOCIATION_TYPES = Collections.unmodifiableMap(persistentAttributeTypes);
-
-       builder = new StringBuilder();
-       builder.append("select");
-       builder.append("\\s+"); // at least one space separating
-       builder.append("(.*\\s+)?"); // anything in between (e.g. distinct) at least one space separating
-       builder.append("new");
-       builder.append("\\s+"); // at least one space separating
-       builder.append(IDENTIFIER);
-       builder.append("\\s*"); // zero to unlimited space separating
-       builder.append("\\(");
-       builder.append(".*");
-       builder.append("\\)");
-
-       CONSTRUCTOR_EXPRESSION = compile(builder.toString(), CASE_INSENSITIVE + DOTALL);
-
-       builder = new StringBuilder();
-       // any function call including parameters within the brackets
-       builder.append("\\w+\\s*\\([\\w\\.,\\s'=:\\\\?]+\\)");
-       // the potential alias
-       builder.append("\\s+[as|AS]+\\s+(([\\w\\.]+))");
-
-       FUNCTION_PATTERN = compile(builder.toString());
-
-       builder = new StringBuilder();
-       builder.append("\\s+"); // at least one space
-       builder.append("[^\\s\\(\\)]+"); // No white char no bracket
-       builder.append("\\s+[as|AS]+\\s+(([\\w\\.]+))"); // the potential alias
-
-       FIELD_ALIAS_PATTERN = compile(builder.toString());
-
-   }
-
-   /**
-    * Private constructor to prevent instantiation.
-    */
-   private QueryUtils() {
-
-   }
-
-   /**
-    * Returns the query string to execute an exists query for the given id attributes.
-    *
-    * @param entityName the name of the entity to create the query for, must not be {@literal null}.
-    * @param countQueryPlaceHolder the placeholder for the count clause, must not be {@literal null}.
-    * @param idAttributes the id attributes for the entity, must not be {@literal null}.
-    */
-   public static String getExistsQueryString(String entityName, String countQueryPlaceHolder,
-           Iterable<String> idAttributes) {
-
-       String whereClause = Streamable.of(idAttributes).stream() //
-               .map(idAttribute -> String.format(EQUALS_CONDITION_STRING, "x", idAttribute, idAttribute)) //
-               .collect(Collectors.joining(" AND ", " WHERE ", ""));
-
-       return String.format(COUNT_QUERY_STRING, countQueryPlaceHolder, entityName) + whereClause;
-   }
-
-   /**
-    * Returns the query string for the given class name.
-    *
-    * @param template must not be {@literal null}.
-    * @param entityName must not be {@literal null}.
-    * @return the template with placeholders replaced by the {@literal entityName}. Guaranteed to be not {@literal null}.
-    */
-   public static String getQueryString(String template, String entityName) {
-
-       Assert.hasText(entityName, "Entity name must not be null or empty");
-
-       return String.format(template, entityName);
-   }
-
-   /**
-    * Adds {@literal order by} clause to the JPQL query. Uses the first alias to bind the sorting property to.
-    *
-    * @param query the query string to which sorting is applied
-    * @param sort the sort specification to apply.
-    * @return the modified query string.
-    */
-   public static String applySorting(String query, Sort sort) {
-       return applySorting(query, sort, detectAlias(query));
-   }
-
-   /**
-    * Adds {@literal order by} clause to the JPQL query.
-    *
-    * @param query the query string to which sorting is applied. Must not be {@literal null} or empty.
-    * @param sort the sort specification to apply.
-    * @param alias the alias to be used in the order by clause. May be {@literal null} or empty.
-    * @return the modified query string.
-    */
-   public static String applySorting(String query, Sort sort, @Nullable String alias) {
-
-       Assert.hasText(query, "Query must not be null or empty");
-
-       if (sort.isUnsorted()) {
-           return query;
-       }
-
-       StringBuilder builder = new StringBuilder(query);
-
-       if (hasOrderByClause(query)) {
-           builder.append(", ");
-       } else {
-           builder.append(" order by ");
-       }
-
-       Set<String> joinAliases = getOuterJoinAliases(query);
-       Set<String> selectionAliases = getFunctionAliases(query);
-       selectionAliases.addAll(getFieldAliases(query));
-
-       for (Order order : sort) {
-           builder.append(getOrderClause(joinAliases, selectionAliases, alias, order)).append(", ");
-       }
-
-       builder.delete(builder.length() - 2, builder.length());
-
-       return builder.toString();
-   }
-
-   /**
-    * Returns {@code true} if the query has {@code order by} clause. The query has {@code order by} clause if there is an
-    * {@code order by} which is not part of window clause.
-    *
-    * @param query the analysed query string
-    * @return {@code true} if the query has {@code order by} clause, {@code false} otherwise
-    */
-   private static boolean hasOrderByClause(String query) {
-       return countOccurrences(ORDER_BY, query) > countOccurrences(ORDER_BY_IN_WINDOW_OR_SUBSELECT, query);
-   }
-
-   /**
-    * Counts the number of occurrences of the pattern in the string
-    *
-    * @param pattern regex with a group to match
-    * @param string analysed string
-    * @return the number of occurrences of the pattern in the string
-    */
-   private static int countOccurrences(Pattern pattern, String string) {
-
-       Matcher matcher = pattern.matcher(string);
-
-       int occurrences = 0;
-       while (matcher.find()) {
-           occurrences++;
-       }
-       return occurrences;
-   }
-
-   /**
-    * Returns the order clause for the given {@link Order}. Will prefix the clause with the given alias if the referenced
-    * property refers to a join alias, i.e. starts with {@code $alias.}.
-    *
-    * @param joinAliases the join aliases of the original query. Must not be {@literal null}.
-    * @param alias the alias for the root entity. May be {@literal null}.
-    * @param order the order object to build the clause for. Must not be {@literal null}.
-    * @return a String containing an order clause. Guaranteed to be not {@literal null}.
-    */
-   private static String getOrderClause(Set<String> joinAliases, Set<String> selectionAlias, @Nullable String alias,
-           Order order) {
-
-       String property = order.getProperty();
-
-       checkSortExpression(order);
-
-       if (selectionAlias.contains(property)) {
-
-           return String.format("%s %s", //
-                   order.isIgnoreCase() ? String.format("lower(%s)", property) : property, //
-                   toJpaDirection(order));
-       }
-
-       boolean qualifyReference = !property.contains("("); // ( indicates a function
-
-       for (String joinAlias : joinAliases) {
-
-           if (property.startsWith(joinAlias.concat("."))) {
-
-               qualifyReference = false;
-               break;
-           }
-       }
-
-       String reference = qualifyReference && StringUtils.hasText(alias) ? String.format("%s.%s", alias, property)
-               : property;
-       String wrapped = order.isIgnoreCase() ? String.format("lower(%s)", reference) : reference;
-
-       return String.format("%s %s", wrapped, toJpaDirection(order));
-   }
-
-   /**
-    * Returns the aliases used for {@code left (outer) join}s.
-    *
-    * @param query a query string to extract the aliases of joins from. Must not be {@literal null}.
-    * @return a {@literal Set} of aliases used in the query. Guaranteed to be not {@literal null}.
-    */
-   static Set<String> getOuterJoinAliases(String query) {
-
-       Set<String> result = new HashSet<>();
-       Matcher matcher = JOIN_PATTERN.matcher(query);
-
-       while (matcher.find()) {
-
-           String alias = matcher.group(QUERY_JOIN_ALIAS_GROUP_INDEX);
-           if (StringUtils.hasText(alias)) {
-               result.add(alias);
-           }
-       }
-
-       return result;
-   }
-
-   /**
-    * Returns the aliases used for fields in the query.
-    *
-    * @param query a {@literal String} containing a query. Must not be {@literal null}.
-    * @return a {@literal Set} containing all found aliases. Guaranteed to be not {@literal null}.
-    */
-   private static Set<String> getFieldAliases(String query) {
-
-       Set<String> result = new HashSet<>();
-       Matcher matcher = FIELD_ALIAS_PATTERN.matcher(query);
-
-       while (matcher.find()) {
-           String alias = matcher.group(1);
-
-           if (StringUtils.hasText(alias)) {
-               result.add(alias);
-           }
-       }
-       return result;
-   }
-
-   /**
-    * Returns the aliases used for aggregate functions like {@code SUM, COUNT, ...}.
-    *
-    * @param query a {@literal String} containing a query. Must not be {@literal null}.
-    * @return a {@literal Set} containing all found aliases. Guaranteed to be not {@literal null}.
-    */
-   static Set<String> getFunctionAliases(String query) {
-
-       Set<String> result = new HashSet<>();
-       Matcher matcher = FUNCTION_PATTERN.matcher(query);
-
-       while (matcher.find()) {
-
-           String alias = matcher.group(1);
-
-           if (StringUtils.hasText(alias)) {
-               result.add(alias);
-           }
-       }
-
-       return result;
-   }
-
-   private static String toJpaDirection(Order order) {
-       return order.getDirection().name().toLowerCase(Locale.US);
-   }
-
-   /**
-    * Resolves the alias for the entity to be retrieved from the given JPA query.
-    *
-    * @param query must not be {@literal null}.
-    * @return Might return {@literal null}.
-    * @deprecated use {@link DeclaredQuery#getAlias()} instead.
-    */
-   @Nullable
-   @Deprecated
-   public static String detectAlias(String query) {
-
-       String alias = null;
-       Matcher matcher = ALIAS_MATCH.matcher(removeSubqueries(query));
-       while (matcher.find()) {
-           alias = matcher.group(2);
-       }
-       return alias;
-   }
-
-   /**
-    * Remove subqueries from the query, in order to identify the correct alias in order by clauses. If the entire query
-    * is surrounded by parenthesis, the outermost parenthesis are not removed.
-    *
-    * @param query
-    * @return query with all subqueries removed.
-    */
-   static String removeSubqueries(String query) {
-
-       if (!StringUtils.hasText(query)) {
-           return query;
-       }
-
-       List<Integer> opens = new ArrayList<>();
-       List<Integer> closes = new ArrayList<>();
-       List<Boolean> closeMatches = new ArrayList<>();
-
-       for (int i = 0; i < query.length(); i++) {
-
-           char c = query.charAt(i);
-           if (c == '(') {
-               opens.add(i);
-           } else if (c == ')') {
-               closes.add(i);
-               closeMatches.add(Boolean.FALSE);
-           }
-       }
-
-       StringBuilder sb = new StringBuilder(query);
-       boolean startsWithParen = STARTS_WITH_PAREN.matcher(query).find();
-       for (int i = opens.size() - 1; i >= (startsWithParen ? 1 : 0); i--) {
-
-           Integer open = opens.get(i);
-           Integer close = findClose(open, closes, closeMatches) + 1;
-
-           if (close > open) {
-
-               String subquery = sb.substring(open, close);
-               Matcher matcher = PARENS_TO_REMOVE.matcher(subquery);
-               if (matcher.find()) {
-                   sb.replace(open, close, new String(new char[close - open]).replace('\0', ' '));
-               }
-           }
-       }
-
-       return sb.toString();
-   }
-
-   private static Integer findClose(final Integer open, final List<Integer> closes, final List<Boolean> closeMatches) {
-
-       for (int i = 0; i < closes.size(); i++) {
-
-           int close = closes.get(i);
-           if (close > open && !closeMatches.get(i)) {
-               closeMatches.set(i, Boolean.TRUE);
-               return close;
-           }
-       }
-
-       return -1;
-   }
-
-   /**
-    * Creates a where-clause referencing the given entities and appends it to the given query string. Binds the given
-    * entities to the query.
-    *
-    * @param <T> type of the entities.
-    * @param queryString must not be {@literal null}.
-    * @param entities must not be {@literal null}.
-    * @param entityManager must not be {@literal null}.
-    * @return Guaranteed to be not {@literal null}.
-    */
-
-   public static <T> Query applyAndBind(String queryString, Iterable<T> entities, EntityManager entityManager) {
-
-       Assert.notNull(queryString, "Querystring must not be null");
-       Assert.notNull(entities, "Iterable of entities must not be null");
-       Assert.notNull(entityManager, "EntityManager must not be null");
-
-       Iterator<T> iterator = entities.iterator();
-
-       if (!iterator.hasNext()) {
-           return entityManager.createQuery(queryString);
-       }
-
-       String alias = detectAlias(queryString);
-       StringBuilder builder = new StringBuilder(queryString);
-       builder.append(" where");
-
-       int i = 0;
-
-       while (iterator.hasNext()) {
-
-           iterator.next();
-
-           builder.append(String.format(" %s = ?%d", alias, ++i));
-
-           if (iterator.hasNext()) {
-               builder.append(" or");
-           }
-       }
-
-       Query query = entityManager.createQuery(builder.toString());
-
-       iterator = entities.iterator();
-       i = 0;
-
-       while (iterator.hasNext()) {
-           query.setParameter(++i, iterator.next());
-       }
-
-       return query;
-   }
-
-   /**
-    * Creates a count projected query from the given original query.
-    *
-    * @param originalQuery must not be {@literal null} or empty.
-    * @return Guaranteed to be not {@literal null}.
-    * @deprecated use {@link DeclaredQuery#deriveCountQuery(String, String)} instead.
-    */
-   @Deprecated
-   public static String createCountQueryFor(String originalQuery) {
-       return createCountQueryFor(originalQuery, null);
-   }
-
-   /**
-    * Creates a count projected query from the given original query.
-    *
-    * @param originalQuery must not be {@literal null}.
-    * @param countProjection may be {@literal null}.
-    * @return a query String to be used a count query for pagination. Guaranteed to be not {@literal null}.
-    * @since 1.6
-    * @deprecated use {@link DeclaredQuery#deriveCountQuery(String, String)} instead.
-    */
-   @Deprecated
-   public static String createCountQueryFor(String originalQuery, @Nullable String countProjection) {
-
-       Assert.hasText(originalQuery, "OriginalQuery must not be null or empty");
-
-       Matcher matcher = COUNT_MATCH.matcher(originalQuery);
-       String countQuery;
-
-       if (countProjection == null) {
-
-           String variable = matcher.matches() ? matcher.group(VARIABLE_NAME_GROUP_INDEX) : null;
-           boolean useVariable = StringUtils.hasText(variable) //
-                   && !variable.startsWith("new") // select [new com.example.User...
-                   && !variable.startsWith(" new") // select distinct[ new com.example.User...
-                   && !variable.startsWith("count(") // select [count(...
-                   && !variable.contains(",");
-
-           String complexCountValue = matcher.matches() && StringUtils.hasText(matcher.group(COMPLEX_COUNT_FIRST_INDEX))
-                   ? COMPLEX_COUNT_VALUE
-                   : COMPLEX_COUNT_LAST_VALUE;
-
-           String replacement = useVariable ? SIMPLE_COUNT_VALUE : complexCountValue;
-
-           String alias = QueryUtils.detectAlias(originalQuery);
-           if ("*".equals(variable) && alias != null) {
-               replacement = alias;
-           }
-
-           countQuery = matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE, replacement));
-       } else {
-           countQuery = matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE, countProjection));
-       }
-
-       return countQuery.replaceFirst(ORDER_BY_PART, "");
-   }
-
-   /**
-    * Returns whether the given {@link Query} contains named parameters.
-    *
-    * @param query Must not be {@literal null}.
-    * @return whether the given {@link Query} contains named parameters.
-    */
-   public static boolean hasNamedParameter(Query query) {
-
-       Assert.notNull(query, "Query must not be null");
-
-       for (Parameter<?> parameter : query.getParameters()) {
-
-           String name = parameter.getName();
-
-           // Hibernate 3 specific hack as it returns the index as String for the name.
-           if (name != null && NO_DIGITS.matcher(name).find()) {
-               return true;
-           }
-       }
-
-       return false;
-   }
-
-   /**
-    * Returns whether the given query contains named parameters.
-    *
-    * @param query can be {@literal null} or empty.
-    * @return whether the given query contains named parameters.
-    */
-   @Deprecated
-   static boolean hasNamedParameter(@Nullable String query) {
-       return StringUtils.hasText(query) && NAMED_PARAMETER.matcher(query).find();
-   }
-
-   /**
-    * Turns the given {@link Sort} into {@link jakarta.persistence.criteria.Order}s.
-    *
-    * @param sort the {@link Sort} instance to be transformed into JPA {@link jakarta.persistence.criteria.Order}s.
-    * @param from must not be {@literal null}.
-    * @param cb must not be {@literal null}.
-    * @return a {@link List} of {@link jakarta.persistence.criteria.Order}s.
-    */
-   public static List<jakarta.persistence.criteria.Order> toOrders(Sort sort, From<?, ?> from, CriteriaBuilder cb) {
-
-       if (sort.isUnsorted()) {
-           return Collections.emptyList();
-       }
-
-       Assert.notNull(from, "From must not be null");
-       Assert.notNull(cb, "CriteriaBuilder must not be null");
-
-       List<jakarta.persistence.criteria.Order> orders = new ArrayList<>();
-
-       for (org.springframework.data.domain.Sort.Order order : sort) {
-           orders.add(toJpaOrder(order, from, cb));
-       }
-
-       return orders;
-   }
-
-   /**
-    * Returns whether the given JPQL query contains a constructor expression.
-    *
-    * @param query must not be {@literal null} or empty.
-    * @return whether the given JPQL query contains a constructor expression.
-    * @since 1.10
-    */
-   public static boolean hasConstructorExpression(String query) {
-
-       Assert.hasText(query, "Query must not be null or empty");
-
-       return CONSTRUCTOR_EXPRESSION.matcher(query).find();
-   }
-
-   /**
-    * Returns the projection part of the query, i.e. everything between {@code select} and {@code from}.
-    *
-    * @param query must not be {@literal null} or empty.
-    * @return the projection part of the query.
-    * @since 1.10.2
-    */
-   public static String getProjection(String query) {
-
-       Assert.hasText(query, "Query must not be null or empty");
-
-       Matcher matcher = PROJECTION_CLAUSE.matcher(query);
-       String projection = matcher.find() ? matcher.group(1) : "";
-       return projection.trim();
-   }
-
-   /**
-    * Creates a criteria API {@link jakarta.persistence.criteria.Order} from the given {@link Order}.
-    *
-    * @param order the order to transform into a JPA {@link jakarta.persistence.criteria.Order}
-    * @param from the {@link From} the {@link Order} expression is based on
-    * @param cb the {@link CriteriaBuilder} to build the {@link jakarta.persistence.criteria.Order} with
-    * @return Guaranteed to be not {@literal null}.
-    */
-   @SuppressWarnings("unchecked")
-   private static jakarta.persistence.criteria.Order toJpaOrder(Order order, From<?, ?> from, CriteriaBuilder cb) {
-
-       PropertyPath property = PropertyPath.from(order.getProperty(), from.getJavaType());
-       Expression<?> expression = toExpressionRecursively(from, property);
-
-       if (order.isIgnoreCase() && String.class.equals(expression.getJavaType())) {
-           Expression<String> upper = cb.lower((Expression<String>) expression);
-           return order.isAscending() ? cb.asc(upper) : cb.desc(upper);
-       } else {
-           return order.isAscending() ? cb.asc(expression) : cb.desc(expression);
-       }
-   }
-
-   static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath property) {
-       return toExpressionRecursively(from, property, false);
-   }
-
-   static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath property, boolean isForSelection) {
-       return toExpressionRecursively(from, property, isForSelection, false);
-   }
-
-   /**
-    * Creates an expression with proper inner and left joins by recursively navigating the path
-    *
-    * @param from the {@link From}
-    * @param property the property path
-    * @param isForSelection is the property navigated for the selection or ordering part of the query?
-    * @param hasRequiredOuterJoin has a parent already required an outer join?
-    * @param <T> the type of the expression
-    * @return the expression
-    */
-   @SuppressWarnings("unchecked")
-   static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath property, boolean isForSelection,
-           boolean hasRequiredOuterJoin) {
-
-       String segment = property.getSegment();
-
-       boolean isLeafProperty = !property.hasNext();
-
-       boolean requiresOuterJoin = requiresOuterJoin(from, property, isForSelection, hasRequiredOuterJoin);
-
-       // if it does not require an outer join and is a leaf, simply get the segment
-       if (!requiresOuterJoin && isLeafProperty) {
-           return from.get(segment);
-       }
-
-       // get or create the join
-       JoinType joinType = requiresOuterJoin ? JoinType.LEFT : JoinType.INNER;
-       Join<?, ?> join = getOrCreateJoin(from, segment, joinType);
-
-       // if it's a leaf, return the join
-       if (isLeafProperty) {
-           return (Expression<T>) join;
-       }
-
-       PropertyPath nextProperty = Objects.requireNonNull(property.next(), "An element of the property path is null");
-
-       // recurse with the next property
-       return toExpressionRecursively(join, nextProperty, isForSelection, requiresOuterJoin);
-   }
-
-   /**
-    * Checks if this attribute requires an outer join. This is the case e.g. if it hadn't already been fetched with an
-    * inner join and if it's an optional association, and if previous paths has already required outer joins. It also
-    * ensures outer joins are used even when Hibernate defaults to inner joins (HHH-12712 and HHH-12999).
-    *
-    * @param from the {@link From} to check for fetches.
-    * @param property the property path
-    * @param isForSelection is the property navigated for the selection or ordering part of the query? if true, we need
-    *          to generate an explicit outer join in order to prevent Hibernate to use an inner join instead. see
-    *          https://hibernate.atlassian.net/browse/HHH-12999
-    * @param hasRequiredOuterJoin has a parent already required an outer join?
-    * @return whether an outer join is to be used for integrating this attribute in a query.
-    */
-   private static boolean requiresOuterJoin(From<?, ?> from, PropertyPath property, boolean isForSelection,
-           boolean hasRequiredOuterJoin) {
-
-       String segment = property.getSegment();
-
-       // already inner joined so outer join is useless
-       if (isAlreadyInnerJoined(from, segment))
-           return false;
-
-       Bindable<?> propertyPathModel;
-       Bindable<?> model = from.getModel();
-
-       // required for EclipseLink: we try to avoid using from.get as EclipseLink produces an inner join
-       // regardless of which join operation is specified next
-       // see: https://bugs.eclipse.org/bugs/show_bug.cgi?id=413892
-       // still occurs as of 2.7
-       ManagedType<?> managedType = null;
-       if (model instanceof ManagedType) {
-           managedType = (ManagedType<?>) model;
-       } else if (model instanceof SingularAttribute
-               && ((SingularAttribute<?, ?>) model).getType() instanceof ManagedType) {
-           managedType = (ManagedType<?>) ((SingularAttribute<?, ?>) model).getType();
-       }
-       if (managedType != null) {
-           propertyPathModel = (Bindable<?>) managedType.getAttribute(segment);
-       } else {
-           propertyPathModel = from.get(segment).getModel();
-       }
-
-       // is the attribute of Collection type?
-       boolean isPluralAttribute = model instanceof PluralAttribute;
-
-       boolean isLeafProperty = !property.hasNext();
-
-       if (propertyPathModel == null && isPluralAttribute) {
-           return true;
-       }
-
-       if (!(propertyPathModel instanceof Attribute)) {
-           return false;
-       }
-
-       Attribute<?, ?> attribute = (Attribute<?, ?>) propertyPathModel;
-
-       // not a persistent attribute type association (@OneToOne, @ManyToOne)
-       if (!ASSOCIATION_TYPES.containsKey(attribute.getPersistentAttributeType())) {
-           return false;
-       }
-
-       boolean isCollection = attribute.isCollection();
-       // if this path is an optional one to one attribute navigated from the not owning side we also need an
-       // explicit outer join to avoid https://hibernate.atlassian.net/browse/HHH-12712
-       // and https://github.com/eclipse-ee4j/jpa-api/issues/170
-       boolean isInverseOptionalOneToOne = PersistentAttributeType.ONE_TO_ONE == attribute.getPersistentAttributeType()
-               && StringUtils.hasText(getAnnotationProperty(attribute, "mappedBy", ""));
-
-       if (isLeafProperty && !isForSelection && !isCollection && !isInverseOptionalOneToOne && !hasRequiredOuterJoin) {
-           return false;
-       }
-
-       return hasRequiredOuterJoin || getAnnotationProperty(attribute, "optional", true);
-   }
-
-   @Nullable
-   private static <T> T getAnnotationProperty(Attribute<?, ?> attribute, String propertyName, T defaultValue) {
-
-       Class<? extends Annotation> associationAnnotation = ASSOCIATION_TYPES.get(attribute.getPersistentAttributeType());
-
-       if (associationAnnotation == null) {
-           return defaultValue;
-       }
-
-       Member member = attribute.getJavaMember();
-
-       if (!(member instanceof AnnotatedElement)) {
-           return defaultValue;
-       }
-
-       Annotation annotation = AnnotationUtils.getAnnotation((AnnotatedElement) member, associationAnnotation);
-       return annotation == null ? defaultValue : (T) AnnotationUtils.getValue(annotation, propertyName);
-   }
-
-   /**
-    * Returns an existing join for the given attribute if one already exists or creates a new one if not.
-    *
-    * @param from the {@link From} to get the current joins from.
-    * @param attribute the {@link Attribute} to look for in the current joins.
-    * @param joinType the join type to create if none was found
-    * @return will never be {@literal null}.
-    */
-   private static Join<?, ?> getOrCreateJoin(From<?, ?> from, String attribute, JoinType joinType) {
-
-       for (Join<?, ?> join : from.getJoins()) {
-
-           if (join.getAttribute().getName().equals(attribute)) {
-               return join;
-           }
-       }
-       return from.join(attribute, joinType);
-   }
-
-   /**
-    * Return whether the given {@link From} contains an inner join for the attribute with the given name.
-    *
-    * @param from the {@link From} to check for joins.
-    * @param attribute the attribute name to check.
-    * @return true if the attribute has already been inner joined
-    */
-   private static boolean isAlreadyInnerJoined(From<?, ?> from, String attribute) {
-
-       for (Fetch<?, ?> fetch : from.getFetches()) {
-
-           if (fetch.getAttribute().getName().equals(attribute) //
-                   && fetch.getJoinType().equals(JoinType.INNER)) {
-               return true;
-           }
-       }
-
-       for (Join<?, ?> join : from.getJoins()) {
-
-           if (join.getAttribute().getName().equals(attribute) //
-                   && join.getJoinType().equals(JoinType.INNER)) {
-               return true;
-           }
-       }
-
-       return false;
-   }
-
-   /**
-    * Check any given {@link JpaOrder#isUnsafe()} order for presence of at least one property offending the
-    * {@link #PUNCTATION_PATTERN} and throw an {@link Exception} indicating potential unsafe order by expression.
-    *
-    * @param order
-    */
-   static void checkSortExpression(Order order) {
-
-       if (order instanceof JpaOrder && ((JpaOrder) order).isUnsafe()) {
-           return;
-       }
-
-       if (PUNCTATION_PATTERN.matcher(order.getProperty()).find()) {
-           throw new InvalidDataAccessApiUsageException(String.format(UNSAFE_PROPERTY_REFERENCE, order));
-       }
-   }
+    public static final String COUNT_QUERY_STRING = "select count(%s) from %s x";
+    public static final String DELETE_ALL_QUERY_STRING = "delete from %s x";
+    public static final String DELETE_ALL_QUERY_BY_ID_STRING = "delete from %s x where %s in :ids";
+
+    // Used Regex/Unicode categories (see https://www.unicode.org/reports/tr18/#General_Category_Property):
+    // Z Separator
+    // Cc Control
+    // Cf Format
+    // Punct Punctuation
+    private static final String IDENTIFIER = "[._$[\\P{Z}&&\\P{Cc}&&\\P{Cf}&&\\P{Punct}]]+";
+    static final String COLON_NO_DOUBLE_COLON = "(?<![:\\\\]):";
+    static final String IDENTIFIER_GROUP = String.format("(%s)", IDENTIFIER);
+
+    private static final String COUNT_REPLACEMENT_TEMPLATE = "select count(%s) $5$6$7";
+    private static final String SIMPLE_COUNT_VALUE = "$2";
+    private static final String COMPLEX_COUNT_VALUE = "$3 $6";
+    private static final String COMPLEX_COUNT_LAST_VALUE = "$6";
+    private static final String ORDER_BY_PART = "(?iu)\\s+order\\s+by\\s+.*";
+
+    private static final Pattern ALIAS_MATCH;
+    private static final Pattern COUNT_MATCH;
+    private static final Pattern STARTS_WITH_PAREN = Pattern.compile("^\\s*\\(");
+    private static final Pattern PARENS_TO_REMOVE = Pattern.compile("(\\(.*\\bfrom\\b[^)]+\\))",
+            CASE_INSENSITIVE | DOTALL | MULTILINE);
+    private static final Pattern PROJECTION_CLAUSE = Pattern.compile("select\\s+(?:distinct\\s+)?(.+)\\s+from",
+            Pattern.CASE_INSENSITIVE);
+
+    private static final Pattern NO_DIGITS = Pattern.compile("\\D+");
+
+    private static final String JOIN = "join\\s+(fetch\\s+)?" + IDENTIFIER + "\\s+(as\\s+)?" + IDENTIFIER_GROUP;
+    private static final Pattern JOIN_PATTERN = Pattern.compile(JOIN, Pattern.CASE_INSENSITIVE);
+
+    private static final String EQUALS_CONDITION_STRING = "%s.%s = :%s";
+    private static final Pattern ORDER_BY = Pattern.compile("(order\\s+by\\s+)", CASE_INSENSITIVE);
+    private static final Pattern ORDER_BY_IN_WINDOW_OR_SUBSELECT = Pattern
+            .compile("\\([\\s\\S]*order\\s+by\\s[\\s\\S]*\\)", CASE_INSENSITIVE);
+
+    private static final Pattern NAMED_PARAMETER = Pattern.compile(COLON_NO_DOUBLE_COLON + IDENTIFIER + "|#" + IDENTIFIER,
+            CASE_INSENSITIVE);
+
+    private static final Pattern CONSTRUCTOR_EXPRESSION;
+
+    private static final Map<PersistentAttributeType, Class<? extends Annotation>> ASSOCIATION_TYPES;
+
+    private static final int QUERY_JOIN_ALIAS_GROUP_INDEX = 3;
+    private static final int VARIABLE_NAME_GROUP_INDEX = 4;
+    private static final int COMPLEX_COUNT_FIRST_INDEX = 3;
+
+    private static final Pattern PUNCTATION_PATTERN = Pattern.compile(".*((?![._])[\\p{Punct}|\\s])");
+    private static final Pattern FUNCTION_PATTERN;
+    private static final Pattern FIELD_ALIAS_PATTERN;
+
+    private static final String UNSAFE_PROPERTY_REFERENCE = "Sort expression '%s' must only contain property references or "
+            + "aliases used in the select clause; If you really want to use something other than that for sorting, please use "
+            + "JpaSort.unsafe(ΓÇª)";
+
+    static {
+
+        StringBuilder builder = new StringBuilder();
+        builder.append("(?<=\\bfrom)"); // from as starting delimiter
+        builder.append("(?:\\s)+"); // at least one space separating
+        builder.append(IDENTIFIER_GROUP); // Entity name, can be qualified (any
+        builder.append("(?:\\sas)*"); // exclude possible "as" keyword
+        builder.append("(?:\\s)+"); // at least one space separating
+        builder.append("(?!(?:where|group\\s*by|order\\s*by))(\\w+)"); // the actual alias
+
+        ALIAS_MATCH = compile(builder.toString(), CASE_INSENSITIVE);
+
+        builder = new StringBuilder();
+        builder.append("\\s*");
+        builder.append("(select\\s+((distinct)?((?s).+?)?)\\s+)?(from\\s+");
+        builder.append(IDENTIFIER);
+        builder.append("(?:\\s+as)?\\s+)");
+        builder.append(IDENTIFIER_GROUP);
+        builder.append("(.*)");
+
+        COUNT_MATCH = compile(builder.toString(), CASE_INSENSITIVE | DOTALL);
+
+        Map<PersistentAttributeType, Class<? extends Annotation>> persistentAttributeTypes = new HashMap<>();
+        persistentAttributeTypes.put(ONE_TO_ONE, OneToOne.class);
+        persistentAttributeTypes.put(ONE_TO_MANY, null);
+        persistentAttributeTypes.put(MANY_TO_ONE, ManyToOne.class);
+        persistentAttributeTypes.put(MANY_TO_MANY, null);
+        persistentAttributeTypes.put(ELEMENT_COLLECTION, null);
+
+        ASSOCIATION_TYPES = Collections.unmodifiableMap(persistentAttributeTypes);
+
+        builder = new StringBuilder();
+        builder.append("select");
+        builder.append("\\s+"); // at least one space separating
+        builder.append("(.*\\s+)?"); // anything in between (e.g. distinct) at least one space separating
+        builder.append("new");
+        builder.append("\\s+"); // at least one space separating
+        builder.append(IDENTIFIER);
+        builder.append("\\s*"); // zero to unlimited space separating
+        builder.append("\\(");
+        builder.append(".*");
+        builder.append("\\)");
+
+        CONSTRUCTOR_EXPRESSION = compile(builder.toString(), CASE_INSENSITIVE + DOTALL);
+
+        builder = new StringBuilder();
+        // any function call including parameters within the brackets
+        builder.append("\\w+\\s*\\([\\w\\.,\\s'=:\\\\?]+\\)");
+        // the potential alias
+        builder.append("\\s+[as|AS]+\\s+(([\\w\\.]+))");
+
+        FUNCTION_PATTERN = compile(builder.toString());
+
+        builder = new StringBuilder();
+        builder.append("\\s+"); // at least one space
+        builder.append("[^\\s\\(\\)]+"); // No white char no bracket
+        builder.append("\\s+[as|AS]+\\s+(([\\w\\.]+))"); // the potential alias
+
+        FIELD_ALIAS_PATTERN = compile(builder.toString());
+
+    }
+
+    /**
+     * Private constructor to prevent instantiation.
+     */
+    private QueryUtils() {
+
+    }
+
+    /**
+     * Returns the query string to execute an exists query for the given id attributes.
+     *
+     * @param entityName            the name of the entity to create the query for, must not be {@literal null}.
+     * @param countQueryPlaceHolder the placeholder for the count clause, must not be {@literal null}.
+     * @param idAttributes          the id attributes for the entity, must not be {@literal null}.
+     */
+    public static String getExistsQueryString(String entityName, String countQueryPlaceHolder,
+                                              Iterable<String> idAttributes) {
+
+        String whereClause = Streamable.of(idAttributes).stream() //
+                .map(idAttribute -> String.format(EQUALS_CONDITION_STRING, "x", idAttribute, idAttribute)) //
+                .collect(Collectors.joining(" AND ", " WHERE ", ""));
+
+        return String.format(COUNT_QUERY_STRING, countQueryPlaceHolder, entityName) + whereClause;
+    }
+
+    /**
+     * Returns the query string for the given class name.
+     *
+     * @param template   must not be {@literal null}.
+     * @param entityName must not be {@literal null}.
+     * @return the template with placeholders replaced by the {@literal entityName}. Guaranteed to be not {@literal null}.
+     */
+    public static String getQueryString(String template, String entityName) {
+
+        Assert.hasText(entityName, "Entity name must not be null or empty");
+
+        return String.format(template, entityName);
+    }
+
+    /**
+     * Adds {@literal order by} clause to the JPQL query. Uses the first alias to bind the sorting property to.
+     *
+     * @param query the query string to which sorting is applied
+     * @param sort  the sort specification to apply.
+     * @return the modified query string.
+     */
+    public static String applySorting(String query, Sort sort) {
+        return applySorting(query, sort, detectAlias(query));
+    }
+
+    /**
+     * Adds {@literal order by} clause to the JPQL query.
+     *
+     * @param query the query string to which sorting is applied. Must not be {@literal null} or empty.
+     * @param sort  the sort specification to apply.
+     * @param alias the alias to be used in the order by clause. May be {@literal null} or empty.
+     * @return the modified query string.
+     */
+    public static String applySorting(String query, Sort sort, @Nullable String alias) {
+
+        Assert.hasText(query, "Query must not be null or empty");
+
+        if (sort.isUnsorted()) {
+            return query;
+        }
+
+        StringBuilder builder = new StringBuilder(query);
+
+        if (hasOrderByClause(query)) {
+            builder.append(", ");
+        } else {
+            builder.append(" order by ");
+        }
+
+        Set<String> joinAliases = getOuterJoinAliases(query);
+        Set<String> selectionAliases = getFunctionAliases(query);
+        selectionAliases.addAll(getFieldAliases(query));
+
+        for (Order order : sort) {
+            builder.append(getOrderClause(joinAliases, selectionAliases, alias, order)).append(", ");
+        }
+
+        builder.delete(builder.length() - 2, builder.length());
+
+        return builder.toString();
+    }
+
+    /**
+     * Returns {@code true} if the query has {@code order by} clause. The query has {@code order by} clause if there is an
+     * {@code order by} which is not part of window clause.
+     *
+     * @param query the analysed query string
+     * @return {@code true} if the query has {@code order by} clause, {@code false} otherwise
+     */
+    private static boolean hasOrderByClause(String query) {
+        return countOccurrences(ORDER_BY, query) > countOccurrences(ORDER_BY_IN_WINDOW_OR_SUBSELECT, query);
+    }
+
+    /**
+     * Counts the number of occurrences of the pattern in the string
+     *
+     * @param pattern regex with a group to match
+     * @param string  analysed string
+     * @return the number of occurrences of the pattern in the string
+     */
+    private static int countOccurrences(Pattern pattern, String string) {
+
+        Matcher matcher = pattern.matcher(string);
+
+        int occurrences = 0;
+        while (matcher.find()) {
+            occurrences++;
+        }
+        return occurrences;
+    }
+
+    /**
+     * Returns the order clause for the given {@link Order}. Will prefix the clause with the given alias if the referenced
+     * property refers to a join alias, i.e. starts with {@code $alias.}.
+     *
+     * @param joinAliases the join aliases of the original query. Must not be {@literal null}.
+     * @param alias       the alias for the root entity. May be {@literal null}.
+     * @param order       the order object to build the clause for. Must not be {@literal null}.
+     * @return a String containing an order clause. Guaranteed to be not {@literal null}.
+     */
+    private static String getOrderClause(Set<String> joinAliases, Set<String> selectionAlias, @Nullable String alias,
+                                         Order order) {
+
+        String property = order.getProperty();
+
+        checkSortExpression(order);
+
+        if (selectionAlias.contains(property)) {
+
+            return String.format("%s %s", //
+                    order.isIgnoreCase() ? String.format("lower(%s)", property) : property, //
+                    toJpaDirection(order));
+        }
+
+        boolean qualifyReference = !property.contains("("); // ( indicates a function
+
+        for (String joinAlias : joinAliases) {
+
+            if (property.startsWith(joinAlias.concat("."))) {
+
+                qualifyReference = false;
+                break;
+            }
+        }
+
+        String reference = qualifyReference && StringUtils.hasText(alias) ? String.format("%s.%s", alias, property)
+                : property;
+        String wrapped = order.isIgnoreCase() ? String.format("lower(%s)", reference) : reference;
+
+        return String.format("%s %s", wrapped, toJpaDirection(order));
+    }
+
+    /**
+     * Returns the aliases used for {@code left (outer) join}s.
+     *
+     * @param query a query string to extract the aliases of joins from. Must not be {@literal null}.
+     * @return a {@literal Set} of aliases used in the query. Guaranteed to be not {@literal null}.
+     */
+    static Set<String> getOuterJoinAliases(String query) {
+
+        Set<String> result = new HashSet<>();
+        Matcher matcher = JOIN_PATTERN.matcher(query);
+
+        while (matcher.find()) {
+
+            String alias = matcher.group(QUERY_JOIN_ALIAS_GROUP_INDEX);
+            if (StringUtils.hasText(alias)) {
+                result.add(alias);
+            }
+        }
+
+        return result;
+    }
+
+    /**
+     * Returns the aliases used for fields in the query.
+     *
+     * @param query a {@literal String} containing a query. Must not be {@literal null}.
+     * @return a {@literal Set} containing all found aliases. Guaranteed to be not {@literal null}.
+     */
+    private static Set<String> getFieldAliases(String query) {
+
+        Set<String> result = new HashSet<>();
+        Matcher matcher = FIELD_ALIAS_PATTERN.matcher(query);
+
+        while (matcher.find()) {
+            String alias = matcher.group(1);
+
+            if (StringUtils.hasText(alias)) {
+                result.add(alias);
+            }
+        }
+        return result;
+    }
+
+    /**
+     * Returns the aliases used for aggregate functions like {@code SUM, COUNT, ...}.
+     *
+     * @param query a {@literal String} containing a query. Must not be {@literal null}.
+     * @return a {@literal Set} containing all found aliases. Guaranteed to be not {@literal null}.
+     */
+    static Set<String> getFunctionAliases(String query) {
+
+        Set<String> result = new HashSet<>();
+        Matcher matcher = FUNCTION_PATTERN.matcher(query);
+
+        while (matcher.find()) {
+
+            String alias = matcher.group(1);
+
+            if (StringUtils.hasText(alias)) {
+                result.add(alias);
+            }
+        }
+
+        return result;
+    }
+
+    private static String toJpaDirection(Order order) {
+        return order.getDirection().name().toLowerCase(Locale.US);
+    }
+
+    /**
+     * Resolves the alias for the entity to be retrieved from the given JPA query.
+     *
+     * @param query must not be {@literal null}.
+     * @return Might return {@literal null}.
+     * @deprecated use {@link DeclaredQuery#getAlias()} instead.
+     */
+    @Nullable
+    @Deprecated
+    public static String detectAlias(String query) {
+
+        String alias = null;
+        Matcher matcher = ALIAS_MATCH.matcher(removeSubqueries(query));
+        while (matcher.find()) {
+            alias = matcher.group(2);
+        }
+        return alias;
+    }
+
+    /**
+     * Remove subqueries from the query, in order to identify the correct alias in order by clauses. If the entire query
+     * is surrounded by parenthesis, the outermost parenthesis are not removed.
+     *
+     * @param query
+     * @return query with all subqueries removed.
+     */
+    static String removeSubqueries(String query) {
+
+        if (!StringUtils.hasText(query)) {
+            return query;
+        }
+
+        List<Integer> opens = new ArrayList<>();
+        List<Integer> closes = new ArrayList<>();
+        List<Boolean> closeMatches = new ArrayList<>();
+
+        for (int i = 0; i < query.length(); i++) {
+
+            char c = query.charAt(i);
+            if (c == '(') {
+                opens.add(i);
+            } else if (c == ')') {
+                closes.add(i);
+                closeMatches.add(Boolean.FALSE);
+            }
+        }
+
+        StringBuilder sb = new StringBuilder(query);
+        boolean startsWithParen = STARTS_WITH_PAREN.matcher(query).find();
+        for (int i = opens.size() - 1; i >= (startsWithParen ? 1 : 0); i--) {
+
+            Integer open = opens.get(i);
+            Integer close = findClose(open, closes, closeMatches) + 1;
+
+            if (close > open) {
+
+                String subquery = sb.substring(open, close);
+                Matcher matcher = PARENS_TO_REMOVE.matcher(subquery);
+                if (matcher.find()) {
+                    sb.replace(open, close, new String(new char[close - open]).replace('\0', ' '));
+                }
+            }
+        }
+
+        return sb.toString();
+    }
+
+    private static Integer findClose(final Integer open, final List<Integer> closes, final List<Boolean> closeMatches) {
+
+        for (int i = 0; i < closes.size(); i++) {
+
+            int close = closes.get(i);
+            if (close > open && !closeMatches.get(i)) {
+                closeMatches.set(i, Boolean.TRUE);
+                return close;
+            }
+        }
+
+        return -1;
+    }
+
+    /**
+     * Creates a where-clause referencing the given entities and appends it to the given query string. Binds the given
+     * entities to the query.
+     *
+     * @param <T>           type of the entities.
+     * @param queryString   must not be {@literal null}.
+     * @param entities      must not be {@literal null}.
+     * @param entityManager must not be {@literal null}.
+     * @return Guaranteed to be not {@literal null}.
+     */
+
+    public static <T> Query applyAndBind(String queryString, Iterable<T> entities, EntityManager entityManager) {
+
+        Assert.notNull(queryString, "Querystring must not be null");
+        Assert.notNull(entities, "Iterable of entities must not be null");
+        Assert.notNull(entityManager, "EntityManager must not be null");
+
+        Iterator<T> iterator = entities.iterator();
+
+        if (!iterator.hasNext()) {
+            return entityManager.createQuery(queryString);
+        }
+
+        String alias = detectAlias(queryString);
+        StringBuilder builder = new StringBuilder(queryString);
+        builder.append(" where");
+
+        int i = 0;
+
+        while (iterator.hasNext()) {
+
+            iterator.next();
+
+            builder.append(String.format(" %s = ?%d", alias, ++i));
+
+            if (iterator.hasNext()) {
+                builder.append(" or");
+            }
+        }
+
+        Query query = entityManager.createQuery(builder.toString());
+
+        iterator = entities.iterator();
+        i = 0;
+
+        while (iterator.hasNext()) {
+            query.setParameter(++i, iterator.next());
+        }
+
+        return query;
+    }
+
+    /**
+     * Creates a count projected query from the given original query.
+     *
+     * @param originalQuery must not be {@literal null} or empty.
+     * @return Guaranteed to be not {@literal null}.
+     * @deprecated use {@link DeclaredQuery#deriveCountQuery(String, String)} instead.
+     */
+    @Deprecated
+    public static String createCountQueryFor(String originalQuery) {
+        return createCountQueryFor(originalQuery, null);
+    }
+
+    /**
+     * Creates a count projected query from the given original query.
+     *
+     * @param originalQuery   must not be {@literal null}.
+     * @param countProjection may be {@literal null}.
+     * @return a query String to be used a count query for pagination. Guaranteed to be not {@literal null}.
+     * @since 1.6
+     * @deprecated use {@link DeclaredQuery#deriveCountQuery(String, String)} instead.
+     */
+    @Deprecated
+    public static String createCountQueryFor(String originalQuery, @Nullable String countProjection) {
+
+        Assert.hasText(originalQuery, "OriginalQuery must not be null or empty");
+
+        Matcher matcher = COUNT_MATCH.matcher(originalQuery);
+        String countQuery;
+
+        if (countProjection == null) {
+
+            String variable = matcher.matches() ? matcher.group(VARIABLE_NAME_GROUP_INDEX) : null;
+            boolean useVariable = StringUtils.hasText(variable) //
+                    && !variable.startsWith("new") // select [new com.example.User...
+                    && !variable.startsWith(" new") // select distinct[ new com.example.User...
+                    && !variable.startsWith("count(") // select [count(...
+                    && !variable.contains(",");
+
+            String complexCountValue = matcher.matches() && StringUtils.hasText(matcher.group(COMPLEX_COUNT_FIRST_INDEX))
+                    ? COMPLEX_COUNT_VALUE
+                    : COMPLEX_COUNT_LAST_VALUE;
+
+            String replacement = useVariable ? SIMPLE_COUNT_VALUE : complexCountValue;
+
+            String alias = QueryUtils.detectAlias(originalQuery);
+            if ("*".equals(variable) && alias != null) {
+                replacement = alias;
+            }
+
+            countQuery = matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE, replacement));
+        } else {
+            countQuery = matcher.replaceFirst(String.format(COUNT_REPLACEMENT_TEMPLATE, countProjection));
+        }
+
+        return countQuery.replaceFirst(ORDER_BY_PART, "");
+    }
+
+    /**
+     * Returns whether the given {@link Query} contains named parameters.
+     *
+     * @param query Must not be {@literal null}.
+     * @return whether the given {@link Query} contains named parameters.
+     */
+    public static boolean hasNamedParameter(Query query) {
+
+        Assert.notNull(query, "Query must not be null");
+
+        for (Parameter<?> parameter : query.getParameters()) {
+
+            String name = parameter.getName();
+
+            // Hibernate 3 specific hack as it returns the index as String for the name.
+            if (name != null && NO_DIGITS.matcher(name).find()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Returns whether the given query contains named parameters.
+     *
+     * @param query can be {@literal null} or empty.
+     * @return whether the given query contains named parameters.
+     */
+    @Deprecated
+    static boolean hasNamedParameter(@Nullable String query) {
+        return StringUtils.hasText(query) && NAMED_PARAMETER.matcher(query).find();
+    }
+
+    /**
+     * Turns the given {@link Sort} into {@link jakarta.persistence.criteria.Order}s.
+     *
+     * @param sort the {@link Sort} instance to be transformed into JPA {@link jakarta.persistence.criteria.Order}s.
+     * @param from must not be {@literal null}.
+     * @param cb   must not be {@literal null}.
+     * @return a {@link List} of {@link jakarta.persistence.criteria.Order}s.
+     */
+    public static List<jakarta.persistence.criteria.Order> toOrders(Sort sort, From<?, ?> from, CriteriaBuilder cb, boolean isDistinct) {
+
+        if (sort.isUnsorted()) {
+            return Collections.emptyList();
+        }
+
+        Assert.notNull(from, "From must not be null");
+        Assert.notNull(cb, "CriteriaBuilder must not be null");
+
+        List<jakarta.persistence.criteria.Order> orders = new ArrayList<>();
+
+        for (org.springframework.data.domain.Sort.Order order : sort) {
+            orders.add(toJpaOrder(order, from, cb, isDistinct));
+        }
+
+        return orders;
+    }
+
+    /**
+     * Returns whether the given JPQL query contains a constructor expression.
+     *
+     * @param query must not be {@literal null} or empty.
+     * @return whether the given JPQL query contains a constructor expression.
+     * @since 1.10
+     */
+    public static boolean hasConstructorExpression(String query) {
+
+        Assert.hasText(query, "Query must not be null or empty");
+
+        return CONSTRUCTOR_EXPRESSION.matcher(query).find();
+    }
+
+    /**
+     * Returns the projection part of the query, i.e. everything between {@code select} and {@code from}.
+     *
+     * @param query must not be {@literal null} or empty.
+     * @return the projection part of the query.
+     * @since 1.10.2
+     */
+    public static String getProjection(String query) {
+
+        Assert.hasText(query, "Query must not be null or empty");
+
+        Matcher matcher = PROJECTION_CLAUSE.matcher(query);
+        String projection = matcher.find() ? matcher.group(1) : "";
+        return projection.trim();
+    }
+
+    /**
+     * Creates a criteria API {@link jakarta.persistence.criteria.Order} from the given {@link Order}.
+     *
+     * @param order the order to transform into a JPA {@link jakarta.persistence.criteria.Order}
+     * @param from  the {@link From} the {@link Order} expression is based on
+     * @param cb    the {@link CriteriaBuilder} to build the {@link jakarta.persistence.criteria.Order} with
+     * @return Guaranteed to be not {@literal null}.
+     */
+    @SuppressWarnings("unchecked")
+    private static jakarta.persistence.criteria.Order toJpaOrder(Order order, From<?, ?> from, CriteriaBuilder cb, boolean isDistinct) {
+
+        PropertyPath property = PropertyPath.from(order.getProperty(), from.getJavaType());
+        Expression<?> expression = toExpressionRecursively(from, property, isDistinct);
+
+        if (order.isIgnoreCase() && String.class.equals(expression.getJavaType())) {
+            Expression<String> upper = cb.lower((Expression<String>) expression);
+            return order.isAscending() ? cb.asc(upper) : cb.desc(upper);
+        } else {
+            return order.isAscending() ? cb.asc(expression) : cb.desc(expression);
+        }
+    }
+
+    static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath property, boolean isDistinct) {
+        return toExpressionRecursively(from, property, false, isDistinct);
+    }
+
+    static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath property, boolean isForSelection, boolean isDistinct) {
+        return toExpressionRecursively(from, property, isForSelection, false, isDistinct);
+    }
+
+    /**
+     * Creates an expression with proper inner and left joins by recursively navigating the path
+     *
+     * @param from                 the {@link From}
+     * @param property             the property path
+     * @param isForSelection       is the property navigated for the selection or ordering part of the query?
+     * @param hasRequiredOuterJoin has a parent already required an outer join?
+     * @param <T>                  the type of the expression
+     * @return the expression
+     */
+    @SuppressWarnings("unchecked")
+    static <T> Expression<T> toExpressionRecursively(From<?, ?> from, PropertyPath property, boolean isForSelection,
+                                                     boolean hasRequiredOuterJoin, boolean isDistinct) {
+
+        String segment = property.getSegment();
+
+        boolean isLeafProperty = !property.hasNext();
+
+        boolean requiresOuterJoin = requiresOuterJoin(from, property, isForSelection, hasRequiredOuterJoin);
+
+        // if it does not require an outer join and is a leaf, simply get the segment
+        if (!requiresOuterJoin && isLeafProperty) {
+            return from.get(segment);
+        }
+
+        // get or create the join
+        JoinType joinType = requiresOuterJoin ? JoinType.LEFT : JoinType.INNER;
+        Join<?, ?> join = getOrCreateJoin(from, segment, joinType, isDistinct);
+
+        // if it's a leaf, return the join
+        if (isLeafProperty) {
+            return (Expression<T>) join;
+        }
+
+        PropertyPath nextProperty = Objects.requireNonNull(property.next(), "An element of the property path is null");
+
+        // recurse with the next property
+        return toExpressionRecursively(join, nextProperty, isForSelection, requiresOuterJoin, isDistinct);
+    }
+
+    /**
+     * Checks if this attribute requires an outer join. This is the case e.g. if it hadn't already been fetched with an
+     * inner join and if it's an optional association, and if previous paths has already required outer joins. It also
+     * ensures outer joins are used even when Hibernate defaults to inner joins (HHH-12712 and HHH-12999).
+     *
+     * @param from                 the {@link From} to check for fetches.
+     * @param property             the property path
+     * @param isForSelection       is the property navigated for the selection or ordering part of the query? if true, we need
+     *                             to generate an explicit outer join in order to prevent Hibernate to use an inner join instead. see
+     *                             https://hibernate.atlassian.net/browse/HHH-12999
+     * @param hasRequiredOuterJoin has a parent already required an outer join?
+     * @return whether an outer join is to be used for integrating this attribute in a query.
+     */
+    private static boolean requiresOuterJoin(From<?, ?> from, PropertyPath property, boolean isForSelection,
+                                             boolean hasRequiredOuterJoin) {
+
+        String segment = property.getSegment();
+
+        // already inner joined so outer join is useless
+        if (isAlreadyInnerJoined(from, segment))
+            return false;
+
+        Bindable<?> propertyPathModel;
+        Bindable<?> model = from.getModel();
+
+        // required for EclipseLink: we try to avoid using from.get as EclipseLink produces an inner join
+        // regardless of which join operation is specified next
+        // see: https://bugs.eclipse.org/bugs/show_bug.cgi?id=413892
+        // still occurs as of 2.7
+        ManagedType<?> managedType = null;
+        if (model instanceof ManagedType) {
+            managedType = (ManagedType<?>) model;
+        } else if (model instanceof SingularAttribute
+                && ((SingularAttribute<?, ?>) model).getType() instanceof ManagedType) {
+            managedType = (ManagedType<?>) ((SingularAttribute<?, ?>) model).getType();
+        }
+        if (managedType != null) {
+            propertyPathModel = (Bindable<?>) managedType.getAttribute(segment);
+        } else {
+            propertyPathModel = from.get(segment).getModel();
+        }
+
+        // is the attribute of Collection type?
+        boolean isPluralAttribute = model instanceof PluralAttribute;
+
+        boolean isLeafProperty = !property.hasNext();
+
+        if (propertyPathModel == null && isPluralAttribute) {
+            return true;
+        }
+
+        if (!(propertyPathModel instanceof Attribute)) {
+            return false;
+        }
+
+        Attribute<?, ?> attribute = (Attribute<?, ?>) propertyPathModel;
+
+        // not a persistent attribute type association (@OneToOne, @ManyToOne)
+        if (!ASSOCIATION_TYPES.containsKey(attribute.getPersistentAttributeType())) {
+            return false;
+        }
+
+        boolean isCollection = attribute.isCollection();
+        // if this path is an optional one to one attribute navigated from the not owning side we also need an
+        // explicit outer join to avoid https://hibernate.atlassian.net/browse/HHH-12712
+        // and https://github.com/eclipse-ee4j/jpa-api/issues/170
+        boolean isInverseOptionalOneToOne = PersistentAttributeType.ONE_TO_ONE == attribute.getPersistentAttributeType()
+                && StringUtils.hasText(getAnnotationProperty(attribute, "mappedBy", ""));
+
+        if (isLeafProperty && !isForSelection && !isCollection && !isInverseOptionalOneToOne && !hasRequiredOuterJoin) {
+            return false;
+        }
+
+        return hasRequiredOuterJoin || getAnnotationProperty(attribute, "optional", true);
+    }
+
+    @Nullable
+    private static <T> T getAnnotationProperty(Attribute<?, ?> attribute, String propertyName, T defaultValue) {
+
+        Class<? extends Annotation> associationAnnotation = ASSOCIATION_TYPES.get(attribute.getPersistentAttributeType());
+
+        if (associationAnnotation == null) {
+            return defaultValue;
+        }
+
+        Member member = attribute.getJavaMember();
+
+        if (!(member instanceof AnnotatedElement)) {
+            return defaultValue;
+        }
+
+        Annotation annotation = AnnotationUtils.getAnnotation((AnnotatedElement) member, associationAnnotation);
+        return annotation == null ? defaultValue : (T) AnnotationUtils.getValue(annotation, propertyName);
+    }
+
+    /**
+     * Returns an existing join for the given attribute if one already exists or creates a new one if not.
+     *
+     * @param from      the {@link From} to get the current joins from.
+     * @param attribute the {@link Attribute} to look for in the current joins.
+     * @param joinType  the join type to create if none was found
+     * @return will never be {@literal null}.
+     */
+    private static Join<?, ?> getOrCreateJoin(From<?, ?> from, String attribute, JoinType joinType, boolean distinct) {
+
+        for (Fetch<?, ?> fetch : from.getFetches()) {
+            if (fetch.getAttribute().getName().equals(attribute))
+                return (Join<?, ?>) fetch;
+        }
+
+        if(!distinct)
+        {
+            for (Join<?, ?> join : from.getJoins()) {
+                if (join.getAttribute().getName().equals(attribute)) {
+                    return join;
+                }
+            }
+        }
+
+        return distinct ?
+                (Join<?, ?>) from.fetch(attribute, joinType) : from.join(attribute, joinType);
+    }
+
+    /**
+     * Return whether the given {@link From} contains an inner join for the attribute with the given name.
+     *
+     * @param from      the {@link From} to check for joins.
+     * @param attribute the attribute name to check.
+     * @return true if the attribute has already been inner joined
+     */
+    private static boolean isAlreadyInnerJoined(From<?, ?> from, String attribute) {
+
+        for (Fetch<?, ?> fetch : from.getFetches()) {
+
+            if (fetch.getAttribute().getName().equals(attribute) //
+                    && fetch.getJoinType().equals(JoinType.INNER)) {
+                return true;
+            }
+        }
+
+        for (Join<?, ?> join : from.getJoins()) {
+
+            if (join.getAttribute().getName().equals(attribute) //
+                    && join.getJoinType().equals(JoinType.INNER)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Check any given {@link JpaOrder#isUnsafe()} order for presence of at least one property offending the
+     * {@link #PUNCTATION_PATTERN} and throw an {@link Exception} indicating potential unsafe order by expression.
+     *
+     * @param order
+     */
+    static void checkSortExpression(Order order) {
+
+        if (order instanceof JpaOrder && ((JpaOrder) order).isUnsafe()) {
+            return;
+        }
+
+        if (PUNCTATION_PATTERN.matcher(order.getProperty()).find()) {
+            throw new InvalidDataAccessApiUsageException(String.format(UNSAFE_PROPERTY_REFERENCE, order));
+        }
+    }
 }

diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java
index 0927afc2..c1bdf2f9 100644
--- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java
+++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/SimpleJpaRepository.java
@@ -742,7 +742,7 @@ public class SimpleJpaRepository<T, ID> implements JpaRepositoryImplementation<T
        query.select(root);

        if (sort.isSorted()) {
-           query.orderBy(toOrders(sort, root, builder));
+           query.orderBy(toOrders(sort, root, builder, query.isDistinct()));
        }

        return applyRepositoryMethodMetadata(em.createQuery(query));

william00179 commented 1 year ago

Hi @gregturn, just checking in to see if you've had an opportunity to have a look at this reproducer I provided?

william00179 commented 1 year ago

Hi @gregturn, following up to see if you've had a moment to check out my reproducer for this issue?

https://github.com/william00179/spring-data-reproducer-2756

The fix (seems) to be quite simple but I've not submitted a PR as I'm not certain on the most elegant way to make this change.

dapriett commented 1 year ago

Hi, running into this as well. Are there any known workarounds for this issue?

william00179 commented 1 year ago

Hi @dapriett,

Unfortunately the only workaround I've found thus far is to make the patches above to spring data jpa and used a forked version of the library.

gregturn commented 1 year ago

I have looked at some parts of this reproducer. I tried tinkering with either .select or .multiselect, and saw no changes. I'm not sure if adjusting the Specification's DISTINCT settings is by design or a lucky side effect, but altering the SELECT's projection doesn't seem adjustable.

Postgres docs clearly show that adding an ORDER BY and a DISTINCT demands that any ordering columns be included in the SELECT clause, which is what is causing the final query to blow up.

As for the forked version of QueryUtils, I can't read amidst that giant patch file what was really changed to assess it. To be clear, QueryUtils is a beast and we are trying to minimize any/all future changes to that class. That's the reason we introduced an HQL/JPQL query parser for non-native queries and have that as our main solution for @Query-based solution.

Which makes me wonder how easy it would be to use @Query on your particular scenario (requires Spring Boot 3.1+).

william00179 commented 1 year ago

Hi Greg,

Unfortunately it's a complicated dynamic query, if I could have done it with @Query I certainly would have.

Basically, the only change required to make it work in Query Utils is this

private static Join<?, ?> getOrCreateJoin(From<?, ?> from, String attribute, JoinType joinType, boolean distinct) {

        for (Fetch<?, ?> fetch : from.getFetches()) {
            if (fetch.getAttribute().getName().equals(attribute))
                return (Join<?, ?>) fetch;
        }

        if (!distinct) {
            for (Join<?, ?> join : from.getJoins()) {

                if (join.getAttribute().getName().equals(attribute)) {
                    return join;
                }
            }
        }

        return distinct ?
                (Join<?, ?>) from.fetch(attribute, joinType) : from.join(attribute, joinType);
    }

The other changes are all just allowing that isDistinct param to be passed around up to where we have access to the query variable and can get query.isDistinct()

With this change made, all the tests pass however there are a few method signatures that need to be updated to pass isDistinct through to this method.

All of our more complicated or dynamic queries are done via a Specification and we are currently having this issue in 3.1 but with this small patch its working well.

Mom0aut commented 1 year ago

Hey @gregturn, we got the same issue any chance this will be fixed in some future release ?

Mom0aut commented 1 year ago

We updated to Spring-Boot 3.1.1 and the order by expression is now generated correctly only for the joined object field ID. Any other properties from anjoined object still throws an error.

phuongnq1995 commented 10 months ago

I'm really interested and looking forward to this feature. I would like to contribute the generic version of getOrCreateJoin.

public static <T, R> Join<T, R> getOrCreateJoin(Root<T> from, SingularAttribute<T, R> attribute, JoinType joinType, boolean distinct) {
    for (Fetch<?, ?> fetch : from.getFetches()) {
      if (fetch.getAttribute().getName().equals(attribute.getName())) {
        return (Join<T, R>) fetch;
      }
    }
    for (Join<T, ?> join : from.getJoins()) {
      if (join.getAttribute().getName().equals(attribute.getName())) {
        return (Join<T, R>) join;
      }
    }
    return distinct ?
        (Join<T, R>) from.fetch(attribute, joinType) : from.join(attribute, joinType);
  }

goetzingert commented 8 months ago

Same issue for me. Order By on an ManyToOne Entity property that is also fetched in the Specification leads to above error.

Main issue is like mentioned above, that fetched relations are not being used and an other join is created

Vladimir-Buttn commented 7 months ago

+1 here

christophstrobl commented 7 months ago

Thank you all for sharing additional information. Looking at the already provided reproducer from @william00179 it seems that switching from join to fetch in the Specification passed into the find method does pass the test without any modifications of QueryUtils that would potentially have implications on other vendors. @goetzingert could you please provide a (failing) sample for the fetch scenario described that would be solved by chaning getOrCreateJoin to also look at getFetches().

fabioformosa commented 6 months ago

Any recent updates on this? It's still present in the latest version.

william00179 commented 5 months ago

This issue doesn't seem to be happening in Spring Boot 3.4.2 / Hibernate 6.4.4.

@christophstrobl I would think switching from join to fetch in all cases would lead to a lot more columns selected and could negatively affect performance?

I think it may still be worthwhile to implement the check to see if there is already a fetch or join to reuse as it will remove unnecessary joins that are currently occurring.

fabioformosa commented 5 months ago

This issue doesn't seem to be happening in Spring Boot 3.4.2 / Hibernate 6.4.4.

Did you mean Spring Boot 3.2.4 ?

We're facing the double join issue with Spring Boot 3.2.2

william00179 commented 5 months ago

Yes sorry I do mean 3.2.4, I transposed the numbers.

While we no longer get the error in this version, we do still see the same table being joined twice in some cases where it has been fetched by an entity graph and then also joined in a specification.

If the fetches were also considered in getOrCreateJoin it would remove this issue.

tiennguyen0494 commented 3 months ago

Hi @william00179 I have fixed my case by overriding the query before the Hibernate executes it Here is my solution:

import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.hibernate.cfg.AvailableSettings; import org.springframework.boot.autoconfigure.orm.jpa.HibernatePropertiesCustomizer;

@Configuration public class HibernateConfig { @Bean public HibernatePropertiesCustomizer hibernateCustomizer() { return (properties) -> properties.put(AvailableSettings.STATEMENT_INSPECTOR, new NytStatementInspector()); } }

import java.util.ArrayList; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern;

import org.hibernate.resource.jdbc.spi.StatementInspector; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.stereotype.Component;

@Component @ConditionalOnProperty(prefix = "athena", name = "statement_inspector", havingValue = "true", matchIfMissing = true) public class NytStatementInspector implements StatementInspector {

@Override
public String inspect(String sql) {
    // Regular expression to match SELECT and ORDER BY clauses
    String selectPattern = "\\bSELECT\\b(.*?)\\bFROM\\b";
    String orderByPattern = "\\bORDER\\s+BY\\b(.*?)$";

    Pattern selectRegex = Pattern.compile(selectPattern, Pattern.CASE_INSENSITIVE);
    Pattern orderByRegex = Pattern.compile(orderByPattern, Pattern.CASE_INSENSITIVE);

    Matcher selectMatcher = selectRegex.matcher(sql);
    Matcher orderByMatcher = orderByRegex.matcher(sql);

    // Check if both SELECT and ORDER BY clauses are present in the SQL
    if (selectMatcher.find() && orderByMatcher.find()) {
        // Extract column names from SELECT and ORDER BY clauses
        String selectColumns = selectMatcher.group(1).trim();
        String orderByColumns = orderByMatcher.group(1).trim();

        // Split the column names
        String[] selectColumnArray = selectColumns.split(",");
        String[] tmpOrderByColumnArray = orderByColumns.split(",");
        List<String> orderByColumnArray = new ArrayList<String>();

        for (String orderByItem : tmpOrderByColumnArray) {
            // The first space is a column.
            // For example: saktype3_.tittel asc limit ?
            var column = orderByItem.trim().split(" ")[0];
            orderByColumnArray.add(column);
        }

        List<String> columnsWillBeAppened = new ArrayList<String>();
        // Check if each column in ORDER BY is present in SELECT, if not, append it to
        // SELECT
        for (String orderByColumn : orderByColumnArray) {
            boolean found = false;
            for (String selectColumn : selectColumnArray) {
                if (selectColumn.trim().equalsIgnoreCase(orderByColumn.trim())) {
                    found = true;
                    break;
                }
            }
            if (!found) {
                // Append the column to SELECT
                columnsWillBeAppened.add(orderByColumn.trim());
            }
        }

        String joinedColumns = String.join(", ", columnsWillBeAppened);
        if (joinedColumns.endsWith(", ")) {
            joinedColumns = new StringBuffer(joinedColumns).substring(0, joinedColumns.length() - 2).toString();
        }

        int index = sql.indexOf("from");
        sql = new StringBuilder(sql).insert(index - 1, ", " + joinedColumns).toString();
        System.out.println("New SQL: " + sql);
    }

    return sql;
}

}

Thank you!

spring-projects / spring-data-jpa

Order by expression missing in select list after update to Spring Boot 3.0.1 with DISTINCT queries #2756

Hi @william00179 I have fixed my case by overriding the query before the Hibernate executes it Here is my solution: