HHH-15766 Add query spec parenthesis also when order by is used within query group

This commit is contained in:
Christian Beikov 2023-03-03 15:25:20 +01:00
parent 1d17719af9
commit 0dc8b9eadd
9 changed files with 54 additions and 18 deletions

View File

@ -335,7 +335,16 @@ public class OracleLegacySqlAstTranslator<T extends JdbcOperation> extends Abstr
public void visitOffsetFetchClause(QueryPart queryPart) { public void visitOffsetFetchClause(QueryPart queryPart) {
if ( !isRowNumberingCurrentQueryPart() ) { if ( !isRowNumberingCurrentQueryPart() ) {
if ( supportsOffsetFetchClause() ) { if ( supportsOffsetFetchClause() ) {
renderOffsetFetchClause( queryPart, true ); if ( getQueryPartStack().depth() > 1 && queryPart.hasSortSpecifications()
&& getQueryPartStack().peek( 1 ) instanceof QueryGroup
&& ( queryPart.isRoot() && !hasLimit() || !queryPart.hasOffsetOrFetchClause() ) ) {
// If the current query part has a query group parent, no offset/fetch clause, but an order by clause,
// then we must render "offset 0 rows" as that is needed for the SQL to be valid
appendSql( " offset 0 rows" );
}
else {
renderOffsetFetchClause( queryPart, true );
}
} }
else { else {
assertRowsOnlyFetchClauseType( queryPart ); assertRowsOnlyFetchClauseType( queryPart );

View File

@ -323,6 +323,12 @@ public class SQLServerLegacySqlAstTranslator<T extends JdbcOperation> extends Ab
else if ( offsetFetchClauseMode == OffsetFetchClauseMode.EMULATED ) { else if ( offsetFetchClauseMode == OffsetFetchClauseMode.EMULATED ) {
renderTopClause( querySpec, isRowsOnlyFetchClauseType( querySpec ), true ); renderTopClause( querySpec, isRowsOnlyFetchClauseType( querySpec ), true );
} }
else if ( getQueryPartStack().depth() > 1 && querySpec.hasSortSpecifications()
&& getQueryPartStack().peek( 1 ) instanceof QueryGroup ) {
// If the current query spec has a query group parent, no offset/fetch clause, but an order by clause,
// then we must render "top 100 percent" as that is needed for the SQL to be valid
appendSql( "top 100 percent " );
}
super.visitSqlSelections( selectClause ); super.visitSqlSelections( selectClause );
} }

View File

@ -339,7 +339,16 @@ public class OracleSqlAstTranslator<T extends JdbcOperation> extends SqlAstTrans
public void visitOffsetFetchClause(QueryPart queryPart) { public void visitOffsetFetchClause(QueryPart queryPart) {
if ( !isRowNumberingCurrentQueryPart() ) { if ( !isRowNumberingCurrentQueryPart() ) {
if ( supportsOffsetFetchClause() ) { if ( supportsOffsetFetchClause() ) {
renderOffsetFetchClause( queryPart, true ); if ( getQueryPartStack().depth() > 1 && queryPart.hasSortSpecifications()
&& getQueryPartStack().peek( 1 ) instanceof QueryGroup
&& ( queryPart.isRoot() && !hasLimit() || !queryPart.hasOffsetOrFetchClause() ) ) {
// If the current query part has a query group parent, no offset/fetch clause, but an order by clause,
// then we must render "offset 0 rows" as that is needed for the SQL to be valid
appendSql( " offset 0 rows" );
}
else {
renderOffsetFetchClause( queryPart, true );
}
} }
else { else {
assertRowsOnlyFetchClauseType( queryPart ); assertRowsOnlyFetchClauseType( queryPart );

View File

@ -302,6 +302,12 @@ public class SQLServerSqlAstTranslator<T extends JdbcOperation> extends SqlAstTr
else if ( offsetFetchClauseMode == OffsetFetchClauseMode.EMULATED ) { else if ( offsetFetchClauseMode == OffsetFetchClauseMode.EMULATED ) {
renderTopClause( querySpec, isRowsOnlyFetchClauseType( querySpec ), true ); renderTopClause( querySpec, isRowsOnlyFetchClauseType( querySpec ), true );
} }
else if ( getQueryPartStack().depth() > 1 && querySpec.hasSortSpecifications()
&& getQueryPartStack().peek( 1 ) instanceof QueryGroup ) {
// If the current query spec has a query group parent, no offset/fetch clause, but an order by clause,
// then we must render "top 100 percent" as that is needed for the SQL to be valid
appendSql( "top 100 percent " );
}
super.visitSqlSelections( selectClause ); super.visitSqlSelections( selectClause );
} }

View File

@ -146,9 +146,7 @@ public class SybaseASESqlAstTranslator<T extends JdbcOperation> extends Abstract
@Override @Override
protected void visitSqlSelections(SelectClause selectClause) { protected void visitSqlSelections(SelectClause selectClause) {
if ( supportsTopClause() ) { renderTopClause( (QuerySpec) getQueryPartStack().getCurrent(), true, false );
renderTopClause( (QuerySpec) getQueryPartStack().getCurrent(), true, false );
}
super.visitSqlSelections( selectClause ); super.visitSqlSelections( selectClause );
} }
@ -192,7 +190,7 @@ public class SybaseASESqlAstTranslator<T extends JdbcOperation> extends Abstract
public void visitOffsetFetchClause(QueryPart queryPart) { public void visitOffsetFetchClause(QueryPart queryPart) {
assertRowsOnlyFetchClauseType( queryPart ); assertRowsOnlyFetchClauseType( queryPart );
if ( !queryPart.isRoot() && queryPart.hasOffsetOrFetchClause() ) { if ( !queryPart.isRoot() && queryPart.hasOffsetOrFetchClause() ) {
if ( queryPart.getFetchClauseExpression() != null && !supportsTopClause() || queryPart.getOffsetClauseExpression() != null ) { if ( queryPart.getFetchClauseExpression() != null && queryPart.getOffsetClauseExpression() != null ) {
throw new IllegalArgumentException( "Can't emulate offset fetch clause in subquery" ); throw new IllegalArgumentException( "Can't emulate offset fetch clause in subquery" );
} }
} }
@ -387,11 +385,6 @@ public class SybaseASESqlAstTranslator<T extends JdbcOperation> extends Abstract
return true; return true;
} }
@Override
protected boolean needsMaxRows() {
return !supportsTopClause();
}
@Override @Override
protected boolean supportsRowValueConstructorSyntax() { protected boolean supportsRowValueConstructorSyntax() {
return false; return false;
@ -412,10 +405,6 @@ public class SybaseASESqlAstTranslator<T extends JdbcOperation> extends Abstract
return " from (select 1) dual(c1)"; return " from (select 1) dual(c1)";
} }
private boolean supportsTopClause() {
return true;
}
private boolean supportsParameterOffsetFetchExpression() { private boolean supportsParameterOffsetFetchExpression() {
return false; return false;
} }

View File

@ -35,6 +35,11 @@ public interface Stack<T> {
*/ */
T getCurrent(); T getCurrent();
/**
* The element at the given offset, relative to the top of the stack
*/
T peek(int offsetFromTop);
/** /**
* The element currently at the bottom of the stack * The element currently at the bottom of the stack
*/ */

View File

@ -72,6 +72,14 @@ public final class StandardStack<T> implements Stack<T> {
return elements[top - 1]; return elements[top - 1];
} }
@Override
public T peek(int offsetFromTop) {
if ( isEmpty() ) {
return null;
}
return elements[top - offsetFromTop - 1];
}
@Override @Override
public T getRoot() { public T getRoot() {
if ( isEmpty() ) { if ( isEmpty() ) {

View File

@ -2992,9 +2992,9 @@ public abstract class AbstractSqlAstTranslator<T extends JdbcOperation> implemen
} }
String queryGroupAlias = null; String queryGroupAlias = null;
if ( currentQueryPart instanceof QueryGroup ) { if ( currentQueryPart instanceof QueryGroup ) {
// We always need query wrapping if we are in a query group and this query spec has a fetch clause // We always need query wrapping if we are in a query group and this query spec has a fetch or order by
// because of order by precedence in SQL // clause, because of order by precedence in SQL
if ( querySpec.hasOffsetOrFetchClause() ) { if ( querySpec.hasOffsetOrFetchClause() || querySpec.hasSortSpecifications() ) {
queryGroupAlias = ""; queryGroupAlias = "";
// If the parent is a query group with a fetch clause we must use a select wrapper, // If the parent is a query group with a fetch clause we must use a select wrapper,
// or if the database does not support simple query grouping, we must use a select wrapper // or if the database does not support simple query grouping, we must use a select wrapper

View File

@ -2,10 +2,13 @@ package org.hibernate.orm.test.query.hql;
import java.util.List; import java.util.List;
import org.hibernate.dialect.SybaseASEDialect;
import org.hibernate.testing.TestForIssue; import org.hibernate.testing.TestForIssue;
import org.hibernate.testing.orm.junit.DomainModel; import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope; import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.hibernate.testing.orm.junit.SkipForDialect;
import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
@ -22,6 +25,7 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
) )
@SessionFactory @SessionFactory
@TestForIssue(jiraKey = "HHH-15766") @TestForIssue(jiraKey = "HHH-15766")
@SkipForDialect(dialectClass = SybaseASEDialect.class, reason = "Sybase ASE does not support order by in subqueries")
public class HqlUnionTest { public class HqlUnionTest {
@BeforeAll @BeforeAll