Skip to content

Commit

Permalink
HHH-15531 Use dense_rank instead of row_number when query uses distinct
Browse files Browse the repository at this point in the history
  • Loading branch information
@beikov
beikov committed Oct 6, 2022
1 parent 5f88f1b commit 519cc24
Show file tree
Hide file tree
Showing 6 changed files with 195 additions and 56 deletions.
7 changes: 5 additions & 2 deletions hibernate-core/src/main/java/org/hibernate/dialect/OracleSqlAstTranslator.java
View file
Expand Up @@ -329,6 +329,8 @@ public void visitOffsetFetchClause(QueryPart queryPart) {
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 {
Expand All @@ -344,8 +346,9 @@ public void visitOver(Over<?> over) {
&& 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;
}
}
Expand Down
98 changes: 98 additions & 0 deletions ...core/src/main/java/org/hibernate/query/sqm/mutation/internal/SqmInsertStrategyHelper.java
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;
}
}
33 changes: 7 additions & 26 deletions ...te-core/src/main/java/org/hibernate/query/sqm/mutation/internal/cte/CteInsertHandler.java
View file
Expand Up @@ -39,6 +39,7 @@
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;
Expand Down Expand Up @@ -267,23 +268,13 @@ public int execute(DomainQueryExecutionContext executionContext) {
);
}
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
)
)
);
Expand Down Expand Up @@ -1000,23 +991,13 @@ protected String addDmlCtes(
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
)
)
);
Expand Down
17 changes: 4 additions & 13 deletions ...ain/java/org/hibernate/query/sqm/mutation/internal/temptable/TableBasedInsertHandler.java
View file
Expand Up @@ -31,6 +31,7 @@
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;
Expand Down Expand Up @@ -216,23 +217,13 @@ else if ( entityDescriptor.getIdentifierGenerator() instanceof OptimizableGenera
);
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
)
)
);
Expand Down
15 changes: 2 additions & 13 deletions hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java
View file
Expand Up @@ -136,6 +136,7 @@
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;
Expand Down Expand Up @@ -1402,22 +1403,10 @@ public boolean applySelections(QuerySpec querySpec, SessionFactoryImplementor se
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;
Expand Down
81 changes: 79 additions & 2 deletions hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/InsertSelectTests.java
View file
Expand Up @@ -6,43 +6,120 @@
*/
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) {
final SQLStatementInspector statementInspector = (SQLStatementInspector) scope.getStatementInspector();
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
@GeneratedValue
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;
}
}
}

0 comments on commit 519cc24

Please sign in to comment.