Sorting doesn't work when using an alias on two or more functions

[darioseidl]

As per the comments on this closed issue, I'm opening a new issue, because sorting still doesn't work for aliases in all cases.

The regex patterns in org.springframework.data.jpa.repository.query.QueryUtils are not broad enough.

The FUNCTION_PATTERN in QueryUtils looks like this:

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());

which only considers a single pair of parenthesis, but not two or more. So, for example, the earlierstBundleStart is not detected in this query:

SELECT DISTINCT(event.id) as id,
event.name as name,
event.top_event as topEvent,
event.ranking as ranking,
MIN(bundle.base_price_amount) as cheapestBundlePrice,
MIN(DATE(bundle.start)) as earliestBundleStart,
..

There was also a question on SO about this here, where many people state that they are facing the same issue.

There are and have been several issues (at least #1404, #1724, #1919) with the regex patterns in QueryUtils. I'm wondering, isn't there any more reliable way to parse the queries than with a regex (possibly an AST that is used also to execute the query)?

Edit: here's another closely related issue: #2079

[gregturn]

I drafted this test case to check things out...

@Test
void foo() {

    String query = "SELECT MIN(bundle.base_price_amount) as cheapestBundlePrice, MIN(DATE(bundle.start)) as earliestBundleStart from example_table";
    Sort sort = Sort.by("earliestBundleStart");

    String fullQuery = applySorting(query, sort);

    assertThat(fullQuery).isEqualTo(
            "SELECT MIN(bundle.base_price_amount) as cheapestBundlePrice, MIN(DATE(bundle.start)) as earliestBundleStart from example_table order by earliestBundleStart asc");
}

Funny thing is, it passed with no issue. But I put a breakpoint inside QueryUtils to see when it would pattern match on FUNCTION_PATTERN. As is, it only catches the alias for the single-level function call (cheapestBundlePrice). Not quite sure what a proper test case would be

I tweaked FUNCTION_PATTERN to builder.append("\\w+\\s*\\(+[\\w\\.,\\s'=]+\\)+"); (notice the extra + next to open and closed parens), and the same breakpoint showed it now catching the earliestBundleStart alias.

But of course we know that this pattern is insufficient based on your hint of an AST instead of regex solution. It would indeed capture such patterns, but it won't ensure a proper grammar. Regex isn't qualified to handle stacking of symbols.

And that's where I'm not yet sold on needing an AST with a lexical analyzer and a proper grammar. I have submitted #2399 to properly handle ignoring case on aliased fields. And it doesn't involve digging into the regex patterns.

I'd frankly prefer solving as many of these aliasing issues as possible before undertaking a complete rewrite based on grammar parsing. Not sure how much bang we'd get for spending such a large buck. Let alone the new level of maintenance for anyone having to maintain it in the future.

[darioseidl]

Thank you for looking into this. I tried your test case and indeed it passes. It seems the problem only appears on a slightly more complicated case:

@Test
void foo() {

    String query = "SELECT DISTINCT(event.id) as id, event.name as name, MIN(bundle.base_price_amount) as cheapestBundlePrice, MIN(DATE(bundle.start)) as earliestBundleStart, FROM event event LEFT JOIN bundle bundle ON event.id = bundle.event_id GROUP BY event.id";
    Sort sort = Sort.by(Sort.Direction.ASC, "cheapestBundlePrice")
            .and(Sort.by(Sort.Direction.ASC, "earliestBundleStart"))
            .and(Sort.by(Sort.Direction.ASC, "name"));

    String fullQuery = QueryUtils.applySorting(query, sort);

    assertThat(fullQuery).isEqualTo(
            query + " order by cheapestBundlePrice asc, earliestBundleStart asc, name asc");
}

In this case, applySorting prepends event. to earliestBundleStart, ending up with " order by cheapestBundlePrice asc, event.earliestBundleStart asc, name asc") although earliestBundleStart is an alias and not a column of the event table.

[darioseidl]

In applySorting, where the order by is appended,

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java

Lines 265 to 271 in 8c4123f

	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(", ");
	}

earliestBundleStart is not in selectionAliases. That in turn seems to mess with the logic in getOrderClause, which decides to prepend the root alias (in this case event) or not (based on some logic that I don't fully understand yet).

[gregturn]

You want to revisit this in light of 1336809.

[darioseidl]

Thanks for the pointer. I tried with the latest QueryUtils from that commit, unfortunately, it did not change anything about the test case posted above.