Fix order by rendering for queries containing UNION #3429
Conversation
Make sure to append space after order by clause and fix alias detection for wrapped sub select. Also make sure to ignore alias used in subselect so they do not conflict with root ones.
| void sortShouldBeAppendedWithSpacingInCaseOfSetOperator() { | ||
|
|
||
| String source = "SELECT tb FROM Test tb WHERE (tb.type='A') UNION SELECT tb FROM Test tb WHERE (tb.type='B')"; | ||
| String target = createQueryFor(source, Sort.by("Type").ascending()); |
There was a problem hiding this comment.
After this line target has the value
SELECT tb
FROM Test tb
WHERE (tb.type = 'A')
order by tb.Type asc
UNION
SELECT tb
FROM Test tb
WHERE (tb.type = 'B')
order by tb.Type asc`i.e. the order by is applied twice which is illegal, since according to answers here https://stackoverflow.com/questions/4715820/how-to-order-by-with-union-in-sql the later order by applies to the full union and an order by on the individual selects might be even illegal (probably depends on the database)
There was a problem hiding this comment.
since we do not know the target db nor how it handles order by we cannot imply ordering the last is the correct behaviour, nor do we have the means to define which of the selects to decorate. an alternative would be to raise an error and demand wrapping the entire thing as it's done in one of the follow up tests.
| String source = "SELECT tb FROM Test tb WHERE (tb.type='A') UNION SELECT tb FROM Test tb WHERE (tb.type='B')"; | ||
| String target = createQueryFor(source, Sort.by("Type").ascending()); | ||
|
|
||
| assertThat(target).contains(" UNION SELECT ").doesNotContainPattern(Pattern.compile(".*\\SUNION")); |
There was a problem hiding this comment.
Since we put in an exact SQL statement we should be able to assert on a precise output and not rely on pattern matching and simiar.
| } | ||
|
|
||
| @ParameterizedTest // GH-3427 | ||
| @ValueSource(strings = {"", "res"}) |
There was a problem hiding this comment.
A comment would be nice explaining why we need to run this with different prefixes.
There was a problem hiding this comment.
deemed to be obvious - apparently isn't
| @ValueSource(strings = {"", "res"}) | ||
| void sortShouldBeAppendedToSubSelectWithSetOperatorInSubselect(String alias) { | ||
|
|
||
| String prefix = StringUtils.hasText(alias) ? (alias + ".") : ""; |
There was a problem hiding this comment.
Again I think we should be able to and prefer a simple equals-assertion.
There was a problem hiding this comment.
I'm not a fan of duplication.
|
we should additionally check the count query creation for |
Make sure to append space after order by clause and fix alias detection for wrapped sub select.
Also make sure to ignore alias used in subselect so they do not conflict with root ones.
Closes: #3427