HHH-15531 Use dense_rank instead of row_number when query uses distinct

This commit is contained in:
Christian Beikov 2022-10-05 14:45:18 +02:00
parent 8193fe6792
commit fa89e3e5fa
7 changed files with 200 additions and 58 deletions

View File

@ -329,6 +329,8 @@ public class OracleLegacySqlAstTranslator<T extends JdbcOperation> extends Abstr
protected void renderRowNumber(SelectClause selectClause, QueryPart queryPart) {
if ( !queryPart.hasSortSpecifications() ) {
// Oracle doesn't allow an empty over clause for the row_number() function
// For regular window function usage, we render a constant order by,
// but since this is used for emulating limit/offset anyway, this is fine
appendSql( "rownum" );
}
else {
@ -344,8 +346,9 @@ public class OracleLegacySqlAstTranslator<T extends JdbcOperation> extends Abstr
&& over.getStartKind() == FrameKind.UNBOUNDED_PRECEDING
&& over.getEndKind() == FrameKind.CURRENT_ROW
&& over.getExclusion() == FrameExclusion.NO_OTHERS ) {
// Oracle doesn't allow an empty over clause for the row_number() function
append( "rownum" );
// Oracle doesn't allow an empty over clause for the row_number() function,
// so we order by a constant
append( "row_number() over(order by 1)" );
return;
}
}

View File

@ -329,6 +329,8 @@ public class OracleSqlAstTranslator<T extends JdbcOperation> extends AbstractSql
protected void renderRowNumber(SelectClause selectClause, QueryPart queryPart) {
if ( !queryPart.hasSortSpecifications() ) {
// Oracle doesn't allow an empty over clause for the row_number() function
// For regular window function usage, we render a constant order by,
// but since this is used for emulating limit/offset anyway, this is fine
appendSql( "rownum" );
}
else {
@ -344,8 +346,9 @@ public class OracleSqlAstTranslator<T extends JdbcOperation> extends AbstractSql
&& over.getStartKind() == FrameKind.UNBOUNDED_PRECEDING
&& over.getEndKind() == FrameKind.CURRENT_ROW
&& over.getExclusion() == FrameExclusion.NO_OTHERS ) {
// Oracle doesn't allow an empty over clause for the row_number() function
append( "rownum" );
// Oracle doesn't allow an empty over clause for the row_number() function,
// so we order by a constant
append( "row_number() over(order by 1)" );
return;
}
}

View File

@ -0,0 +1,98 @@
/*
* Hibernate, Relational Persistence for Idiomatic Java
*
* License: GNU Lesser General Public License (LGPL), version 2.1 or later
* See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
*/
package org.hibernate.query.sqm.mutation.internal;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.query.sqm.SortOrder;
import org.hibernate.query.sqm.function.SelfRenderingWindowFunctionSqlAstExpression;
import org.hibernate.sql.ast.spi.SqlSelection;
import org.hibernate.sql.ast.tree.expression.Expression;
import org.hibernate.sql.ast.tree.expression.Over;
import org.hibernate.sql.ast.tree.expression.SqlSelectionExpression;
import org.hibernate.sql.ast.tree.select.QuerySpec;
import org.hibernate.sql.ast.tree.select.SortSpecification;
import org.hibernate.type.BasicType;
/**
* @author Christian Beikov
*/
public final class SqmInsertStrategyHelper {
private SqmInsertStrategyHelper() {
}
/**
* Creates a row numbering expression, that can be added to the select clause of the query spec.
*/
public static Expression createRowNumberingExpression(
QuerySpec querySpec,
SessionFactoryImplementor sessionFactory) {
final BasicType<Integer> resultType = sessionFactory.getTypeConfiguration()
.getBasicTypeForJavaType( Integer.class );
final Expression functionExpression;
final List<SortSpecification> orderList;
if ( querySpec.getSelectClause().isDistinct() ) {
assert sessionFactory.getJdbcServices().getDialect().supportsWindowFunctions();
functionExpression = new SelfRenderingWindowFunctionSqlAstExpression(
"dense_rank",
(appender, args, walker) -> appender.appendSql( "dense_rank()" ),
Collections.emptyList(),
null,
null,
null,
resultType,
resultType
);
final List<SortSpecification> sortSpecifications = querySpec.getSortSpecifications();
final List<SqlSelection> sqlSelections = querySpec.getSelectClause().getSqlSelections();
if ( sortSpecifications == null ) {
orderList = new ArrayList<>( sqlSelections.size() );
}
else {
orderList = new ArrayList<>( sortSpecifications.size() + sqlSelections.size() );
orderList.addAll( sortSpecifications );
}
for ( SqlSelection sqlSelection : sqlSelections ) {
if ( containsSelectionExpression( orderList, sqlSelection ) ) {
continue;
}
orderList.add( new SortSpecification( sqlSelection.getExpression(), SortOrder.ASCENDING ) );
}
}
else {
functionExpression = new SelfRenderingWindowFunctionSqlAstExpression(
"row_number",
(appender, args, walker) -> appender.appendSql( "row_number()" ),
Collections.emptyList(),
null,
null,
null,
resultType,
resultType
);
orderList = Collections.emptyList();
}
return new Over<>( functionExpression, Collections.emptyList(), orderList );
}
private static boolean containsSelectionExpression(List<SortSpecification> orderList, SqlSelection sqlSelection) {
final Expression expression = sqlSelection.getExpression();
for ( SortSpecification sortSpecification : orderList ) {
final Expression sortExpression = sortSpecification.getSortExpression();
if ( sortExpression == expression || sortExpression instanceof SqlSelectionExpression
&& ( (SqlSelectionExpression) sortExpression ).getSelection() == sqlSelection ) {
return true;
}
}
return false;
}
}

View File

@ -39,6 +39,7 @@ import org.hibernate.persister.entity.EntityPersister;
import org.hibernate.persister.entity.Joinable;
import org.hibernate.query.sqm.BinaryArithmeticOperator;
import org.hibernate.query.sqm.ComparisonOperator;
import org.hibernate.query.sqm.mutation.internal.SqmInsertStrategyHelper;
import org.hibernate.spi.NavigablePath;
import org.hibernate.query.SemanticException;
import org.hibernate.query.sqm.SortOrder;
@ -267,23 +268,13 @@ public class CteInsertHandler implements InsertHandler {
);
}
if ( !assignsId && entityDescriptor.getIdentifierGenerator() instanceof PostInsertIdentifierGenerator ) {
final BasicType<Integer> rowNumberType = sessionFactory.getTypeConfiguration()
.getBasicTypeForJavaType( Integer.class );
querySpec.getSelectClause().addSqlSelection(
new SqlSelectionImpl(
1,
0,
new Over<>(
new SelfRenderingFunctionSqlAstExpression(
"row_number",
(appender, args, walker) -> appender.appendSql(
"row_number()" ),
Collections.emptyList(),
rowNumberType,
rowNumberType
),
Collections.emptyList(),
Collections.emptyList()
SqmInsertStrategyHelper.createRowNumberingExpression(
querySpec,
sessionFactory
)
)
);
@ -1000,23 +991,13 @@ public class CteInsertHandler implements InsertHandler {
idColumnReference
)
);
final BasicType<Integer> rowNumberType = sessionFactory.getTypeConfiguration()
.getBasicTypeForJavaType( Integer.class );
finalResultQuery.getSelectClause().addSqlSelection(
new SqlSelectionImpl(
1,
0,
new Over<>(
new SelfRenderingFunctionSqlAstExpression(
"row_number",
(appender, args, walker) -> appender.appendSql(
"row_number()" ),
Collections.emptyList(),
rowNumberType,
rowNumberType
),
Collections.emptyList(),
Collections.emptyList()
SqmInsertStrategyHelper.createRowNumberingExpression(
querySpec,
sessionFactory
)
)
);

View File

@ -31,6 +31,7 @@ import org.hibernate.query.sqm.internal.DomainParameterXref;
import org.hibernate.query.sqm.internal.SqmJdbcExecutionContextAdapter;
import org.hibernate.query.sqm.mutation.internal.InsertHandler;
import org.hibernate.query.sqm.mutation.internal.MultiTableSqmMutationConverter;
import org.hibernate.query.sqm.mutation.internal.SqmInsertStrategyHelper;
import org.hibernate.query.sqm.sql.BaseSqmToSqlAstConverter;
import org.hibernate.query.sqm.tree.expression.SqmParameter;
import org.hibernate.query.sqm.tree.insert.SqmInsertSelectStatement;
@ -216,23 +217,13 @@ public class TableBasedInsertHandler implements InsertHandler {
);
insertStatement.getTargetColumnReferences().add( columnReference );
targetPathColumns.add( new Assignment( columnReference, columnReference ) );
final BasicType<Integer> rowNumberType = sessionFactory.getTypeConfiguration()
.getBasicTypeForJavaType( Integer.class );
querySpec.getSelectClause().addSqlSelection(
new SqlSelectionImpl(
1,
0,
new Over<>(
new SelfRenderingFunctionSqlAstExpression(
"row_number",
(appender, args, walker) -> appender.appendSql(
"row_number()" ),
Collections.emptyList(),
rowNumberType,
rowNumberType
),
Collections.emptyList(),
Collections.emptyList()
SqmInsertStrategyHelper.createRowNumberingExpression(
querySpec,
sessionFactory
)
)
);

View File

@ -136,6 +136,7 @@ import org.hibernate.query.sqm.function.SelfRenderingAggregateFunctionSqlAstExpr
import org.hibernate.query.sqm.function.SelfRenderingFunctionSqlAstExpression;
import org.hibernate.query.sqm.internal.DomainParameterXref;
import org.hibernate.query.sqm.internal.SqmMappingModelHelper;
import org.hibernate.query.sqm.mutation.internal.SqmInsertStrategyHelper;
import org.hibernate.query.sqm.produce.function.internal.PatternRenderer;
import org.hibernate.query.sqm.spi.BaseSemanticQueryWalker;
import org.hibernate.query.sqm.sql.internal.BasicValuedPathInterpretation;
@ -1402,22 +1403,10 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
if ( !sessionFactory.getJdbcServices().getDialect().supportsWindowFunctions() ) {
return false;
}
final BasicType<Integer> rowNumberType = sessionFactory.getTypeConfiguration()
.getBasicTypeForJavaType( Integer.class );
identifierSelection = new SqlSelectionImpl(
1,
0,
new Over<>(
new SelfRenderingFunctionSqlAstExpression(
"row_number",
(appender, args, walker) -> appender.appendSql( "row_number()" ),
Collections.emptyList(),
rowNumberType,
rowNumberType
),
Collections.emptyList(),
Collections.emptyList()
)
SqmInsertStrategyHelper.createRowNumberingExpression( querySpec, sessionFactory )
);
selectClause.addSqlSelection( identifierSelection );
return true;

View File

@ -6,25 +6,52 @@
*/
package org.hibernate.orm.test.query.hql;
import org.hibernate.dialect.DerbyDialect;
import org.hibernate.testing.TestForIssue;
import org.hibernate.testing.jdbc.SQLStatementInspector;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.ServiceRegistry;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.hibernate.testing.orm.junit.SkipForDialect;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.Id;
import static org.junit.jupiter.api.Assertions.assertEquals;
@ServiceRegistry
@DomainModel( annotatedClasses = {
InsertSelectTests.EntityEntry.class
InsertSelectTests.EntityEntry.class,
InsertSelectTests.EntitySource.class
})
@SessionFactory(statementInspectorClass = SQLStatementInspector.class)
public class InsertSelectTests {
@BeforeEach
public void prepareTestData(SessionFactoryScope scope) {
scope.inTransaction(
session -> {
session.persist( new EntitySource( "A" ) );
session.persist( new EntitySource( "A" ) );
}
);
}
@AfterEach
public void cleanupTestData(SessionFactoryScope scope) {
scope.inTransaction(
session -> {
session.createMutationQuery( "delete from java.lang.Object" ).executeUpdate();
}
);
}
@Test
@TestForIssue( jiraKey = "HHH-15527")
public void testInsertSelectGeneratedAssigned(SessionFactoryScope scope) {
@ -32,12 +59,47 @@ public class InsertSelectTests {
scope.inTransaction(
session -> {
statementInspector.clear();
session.createMutationQuery("insert into EntityEntry (id, name) select 1, 'abc' from EntityEntry e").executeUpdate();
session.createMutationQuery(
"insert into EntityEntry (id, name) " +
"select 1, 'abc' from EntityEntry e"
).executeUpdate();
statementInspector.assertExecutedCount( 1 );
}
);
}
@Test
@TestForIssue( jiraKey = "HHH-15531")
@SkipForDialect(dialectClass = DerbyDialect.class, reason = "Derby doesn't really support window functions, " +
"but this requires the use of a dense_rank window function. We could emulate this, but don't think it's worth it")
public void testInsertSelectDistinct(SessionFactoryScope scope) {
scope.inTransaction(
session -> {
final int rows = session.createMutationQuery(
"insert into EntityEntry (name) " +
"select distinct e.name from EntitySource e"
).executeUpdate();
assertEquals( 1, rows );
}
);
}
@Test
@TestForIssue( jiraKey = "HHH-15531")
@SkipForDialect(dialectClass = DerbyDialect.class, reason = "Derby doesn't really support window functions and " +
"its attempt at a row_number function fails to deliver the desired semantics")
public void testInsertSelectGroupBy(SessionFactoryScope scope) {
scope.inTransaction(
session -> {
final int rows = session.createMutationQuery(
"insert into EntityEntry (name) " +
"select e.name from EntitySource e group by e.name"
).executeUpdate();
assertEquals( 1, rows );
}
);
}
@Entity(name = "EntityEntry")
public static class EntityEntry {
@Id
@ -45,4 +107,19 @@ public class InsertSelectTests {
Integer id;
String name;
}
@Entity(name = "EntitySource")
public static class EntitySource {
@Id
@GeneratedValue
Integer id;
String name;
public EntitySource() {
}
public EntitySource(String name) {
this.name = name;
}
}
}