HHH-16826 - IN-Clause Parameter Padding should grow exponentially for Dialects with InExpressionCountLimit

This commit is contained in:
Adrodoc 2023-06-26 16:15:40 +02:00 committed by Christian Beikov
parent f161386b36
commit 68601df471
4 changed files with 133 additions and 174 deletions

View File

@ -23,4 +23,13 @@ public final class MathHelper {
public static int ceilingPowerOfTwo(int value) {
return 1 << -Integer.numberOfLeadingZeros(value - 1);
}
/**
* Returns the result of dividing a positive {@code numerator} by a positive {@code denominator} rounded up.
* <p>
* For example dividing 5 by 2 ist 2.5, which (when rounded up) gives a result of 3.
*/
public static int divideRoundingUp(int numerator, int denominator) {
return ( numerator + denominator - 1 ) / denominator;
}
}

View File

@ -6874,42 +6874,16 @@ public abstract class AbstractSqlAstTranslator<T extends JdbcOperation> implemen
}
int bindValueCount = listExpressions.size();
int bindValueMaxCount = bindValueCount;
int bindValueCountWithPadding = bindValueCount;
int inExprLimit = dialect.getInExpressionCountLimit();
final boolean inClauseParameterPaddingEnabled = getSessionFactory().
getSessionFactoryOptions().inClauseParameterPaddingEnabled()
&& bindValueCount > 2;
if ( inClauseParameterPaddingEnabled ) {
// bindValueCount: 1005
// bindValuePaddingCount: 1024
int bindValuePaddingCount = MathHelper.ceilingPowerOfTwo( bindValueCount );
// inExprLimit: 1000
if ( inExprLimit > 0 ) {
if ( bindValuePaddingCount > inExprLimit ) {
// bindValueCount % inExprLimit: 5
// bindValuePaddingCount: 8
if ( bindValueCount < inExprLimit ) {
bindValueMaxCount = inExprLimit;
}
else {
bindValueMaxCount = MathHelper.ceilingPowerOfTwo( bindValueCount % inExprLimit );
}
}
else if ( bindValueCount < bindValuePaddingCount ) {
bindValueMaxCount = bindValuePaddingCount;
}
}
else if ( bindValueCount < bindValuePaddingCount ) {
bindValueMaxCount = bindValuePaddingCount;
}
if ( getSessionFactory().getSessionFactoryOptions().inClauseParameterPaddingEnabled() ) {
bindValueCountWithPadding = addPadding( bindValueCount, inExprLimit );
}
final boolean parenthesis = !inListPredicate.isNegated()
&& inExprLimit != 0 && listExpressions.size() > inExprLimit;
&& inExprLimit > 0 && listExpressions.size() > inExprLimit;
if ( parenthesis ) {
appendSql( OPEN_PARENTHESIS );
}
@ -6922,71 +6896,60 @@ public abstract class AbstractSqlAstTranslator<T extends JdbcOperation> implemen
String separator = NO_SEPARATOR;
final Iterator<Expression> iterator = listExpressions.iterator();
int itemNumber = 0;
while ( iterator.hasNext() && ( inExprLimit == 0 || itemNumber < inExprLimit ) ) {
final Expression listExpression = itemAccessor.apply( iterator.next() );
Expression listExpression = null;
for ( int i = 0; i < bindValueCountWithPadding; i++ ) {
if ( inExprLimit > 0 && i % inExprLimit == 0 && i != 0 ) {
appendInClauseSeparator( inListPredicate );
separator = NO_SEPARATOR;
}
if ( iterator.hasNext() ) { // If the iterator is exhausted, reuse the last expression for padding.
listExpression = itemAccessor.apply( iterator.next() );
}
// The only way for expression to be null is if listExpressions is empty,
// but if that is the case the code takes an early exit.
assert listExpression != null;
appendSql( separator );
listExpression.accept( this );
separator = COMMA_SEPARATOR;
itemNumber++;
// If we encounter an expression that is not a parameter or literal, we reset the inExprLimit and bindValueMaxCount
// and just render through the in list expressions as they are without padding/splitting
// If we encounter an expression that is not a parameter or literal, we reset the inExprLimit and
// bindValueMaxCount and just render through the in list expressions as they are without padding/splitting
if ( !( listExpression instanceof JdbcParameter || listExpression instanceof SqmParameterInterpretation || listExpression instanceof Literal ) ) {
inExprLimit = 0;
bindValueMaxCount = bindValueCount;
bindValueCountWithPadding = bindValueCount;
}
}
if ( itemNumber != inExprLimit && bindValueCount == bindValueMaxCount ) {
appendSql( CLOSE_PARENTHESIS );
return;
}
if ( inExprLimit > 0 && bindValueCount > inExprLimit ) {
do {
if ( inListPredicate.isNegated() ) {
append( ") and " );
inListPredicate.getTestExpression().accept( this );
appendSql( " not" );
}
else {
append( ") or " );
inListPredicate.getTestExpression().accept( this );
}
appendSql( " in (" );
separator = NO_SEPARATOR;
itemNumber = 0;
while ( iterator.hasNext() && itemNumber < inExprLimit ) {
final Expression listExpression = iterator.next();
appendSql( separator );
itemAccessor.apply( listExpression ).accept( this );
separator = COMMA_SEPARATOR;
itemNumber++;
}
} while ( iterator.hasNext() );
}
if ( inClauseParameterPaddingEnabled ) {
final Expression lastExpression = itemAccessor.apply( listExpressions.get( listExpressions.size() - 1 ) );
final int end;
if ( inExprLimit > 0 ) {
end = Math.min( bindValueMaxCount, inExprLimit );
}
else {
end = bindValueMaxCount;
}
for ( ; itemNumber < end; itemNumber++ ) {
appendSql( separator );
lastExpression.accept( this );
separator = COMMA_SEPARATOR;
}
}
appendSql( CLOSE_PARENTHESIS );
if ( parenthesis ) {
appendSql( CLOSE_PARENTHESIS );
}
}
private void appendInClauseSeparator(InListPredicate inListPredicate) {
appendSql( CLOSE_PARENTHESIS );
appendSql( inListPredicate.isNegated() ? " and " : " or " );
inListPredicate.getTestExpression().accept( this );
if ( inListPredicate.isNegated() ) {
appendSql( " not" );
}
appendSql( " in " );
appendSql( OPEN_PARENTHESIS );
}
private static int addPadding(int bindValueCount, int inExprLimit) {
int ceilingPowerOfTwo = MathHelper.ceilingPowerOfTwo( bindValueCount );
if ( inExprLimit <= 0 || ceilingPowerOfTwo <= inExprLimit ) {
return ceilingPowerOfTwo;
}
int numberOfInClauses = MathHelper.divideRoundingUp( bindValueCount, inExprLimit );
int numberOfInClausesWithPadding = MathHelper.ceilingPowerOfTwo( numberOfInClauses );
return numberOfInClausesWithPadding * inExprLimit;
}
@Override
public void visitInArrayPredicate(InArrayPredicate inArrayPredicate) {
sqlBuffer.append( "array_contains(" );

View File

@ -9,17 +9,24 @@ package org.hibernate.orm.test.internal.util;
import org.hibernate.internal.util.MathHelper;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.params.provider.Arguments.arguments;
import java.util.stream.Stream;
/**
* @author Vlad Mihalcea
*/
public class MathHelperTest {
@Test
public void ceilingPowerOfTwo() {
assertThat( MathHelper.ceilingPowerOfTwo( 0 ) ).isEqualTo( 1 );
assertThat( MathHelper.ceilingPowerOfTwo( 1 ) ).isEqualTo( 1 );
assertThat( MathHelper.ceilingPowerOfTwo( 2 ) ).isEqualTo( 2 );
assertThat( MathHelper.ceilingPowerOfTwo( 3 ) ).isEqualTo( 4 );
@ -39,4 +46,24 @@ public class MathHelperTest {
assertThat( MathHelper.ceilingPowerOfTwo( 16 ) ).isEqualTo( 16 );
}
static Stream<Arguments> test_divideRoundingUp() {
return Stream.of(
arguments( 0, 1, 0 ),
arguments( 1, 1, 1 ),
arguments( 2, 1, 2 ),
arguments( 0, 2, 0 ),
arguments( 1, 2, 1 ),
arguments( 2, 2, 1 ),
arguments( 3, 2, 2 ),
arguments( 4, 2, 2 ),
arguments( 5, 2, 3 ),
arguments( 9, 3, 3 ),
arguments( 10, 3, 4 ) );
}
@ParameterizedTest
@MethodSource
public void test_divideRoundingUp(int numerator, int denominator, int expected) {
assertThat( MathHelper.divideRoundingUp( numerator, denominator ) ).isEqualTo( expected );
}
}

View File

@ -6,9 +6,11 @@
*/
package org.hibernate.orm.test.query;
import java.util.Iterator;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;
import org.hibernate.cfg.AvailableSettings;
import org.hibernate.dialect.H2Dialect;
@ -22,6 +24,9 @@ import org.hibernate.testing.orm.junit.Setting;
import org.hibernate.testing.orm.junit.SettingProvider;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import jakarta.persistence.Entity;
import jakarta.persistence.Id;
@ -29,6 +34,7 @@ import jakarta.persistence.Id;
import static java.util.stream.Collectors.toList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.params.provider.Arguments.arguments;
/**
* @author Vlad Mihalcea
@ -91,56 +97,75 @@ public class MaxInExpressionParameterPaddingTest {
assertThat( statementInspector.getSqlQueries().get( 0 ) ).endsWith( expectedInClause.toString() );
}
@Test
public void testInClauseParameterNoPaddingAtLimit(EntityManagerFactoryScope scope) {
static Stream<Arguments> testInClauseParameterPaddingUpToLimit() {
return Stream.of(
arguments( 1, 1 ),
arguments( 2, 2 ),
arguments( 3, 4 ),
arguments( 4, 4 ),
arguments( 5, 8 ),
arguments( 6, 8 ),
arguments( 7, 8 ),
arguments( 8, 8 ),
arguments( 9, 15 ),
arguments( 10, 15 ), // Test for HHH-14109
arguments( 11, 15 ),
arguments( 12, 15 ),
arguments( 13, 15 ),
arguments( 14, 15 ),
arguments( 15, 15 ) );
}
@TestForIssue(jiraKey = "HHH-16826")
@ParameterizedTest
@MethodSource
public void testInClauseParameterPaddingUpToLimit(int parameters, int expectedBindVariables, EntityManagerFactoryScope scope) {
final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
statementInspector.clear();
scope.inTransaction( entityManager -> entityManager
.createQuery( "select p from Person p where p.id in :ids" )
.setParameter( "ids", integerRangeList( 0, MAX_COUNT ) )
.setParameter( "ids", integerRangeList( 0, parameters ) )
.getResultList() );
StringBuilder expectedInClause = new StringBuilder();
expectedInClause.append( "where p1_0.id in " );
appendInClause( expectedInClause, MAX_COUNT );
appendInClause( expectedInClause, expectedBindVariables );
assertThat( statementInspector.getSqlQueries().get( 0 ) ).endsWith( expectedInClause.toString() );
}
@TestForIssue(jiraKey = "HHH-14109")
@Test
public void testInClauseParameterPaddingToLimit(EntityManagerFactoryScope scope) {
static Stream<Arguments> testInClauseParameterPaddingAfterLimit() {
return Stream.of(
arguments( 16, 2 ),
arguments( 18, 2 ),
arguments( 33, 4 ),
arguments( 39, 4 ), // Test for HHH-16589
arguments( 4 * MAX_COUNT, 4 ),
arguments( 4 * MAX_COUNT + 1, 8 ),
arguments( 8 * MAX_COUNT, 8 ),
arguments( 8 * MAX_COUNT + 1, 16 ) );
}
@TestForIssue(jiraKey = "HHH-16826")
@ParameterizedTest
@MethodSource
public void testInClauseParameterPaddingAfterLimit(int parameters, int expectedInClauses, EntityManagerFactoryScope scope) {
final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
statementInspector.clear();
scope.inTransaction( entityManager -> entityManager
.createQuery( "select p from Person p where p.id in :ids" )
.setParameter( "ids", integerRangeList( 0, 10 ) )
.getResultList() );
StringBuilder expectedInClause = new StringBuilder();
expectedInClause.append( "where p1_0.id in " );
appendInClause( expectedInClause, MAX_COUNT );
assertThat( statementInspector.getSqlQueries().get( 0 ) ).endsWith( expectedInClause.toString() );
}
@Test
public void testInClauseParameterSplittingAfterLimit(EntityManagerFactoryScope scope) {
final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
statementInspector.clear();
scope.inTransaction( entityManager -> entityManager
.createQuery( "select p from Person p where p.id in :ids" )
.setParameter( "ids", integerRangeList( 0, 16 ) )
.setParameter( "ids", integerRangeList( 0, parameters ) )
.getResultList() );
StringBuilder expectedInClause = new StringBuilder();
expectedInClause.append( "where (p1_0.id in " );
appendInClause( expectedInClause, MAX_COUNT );
expectedInClause.append( " or p1_0.id in " );
appendInClause( expectedInClause, 1 );
for ( int i = 1; i < expectedInClauses; i++ ) {
expectedInClause.append( " or p1_0.id in " );
appendInClause( expectedInClause, MAX_COUNT );
}
expectedInClause.append( ')' );
assertThat( statementInspector.getSqlQueries().get( 0 ) ).endsWith( expectedInClause.toString() );
@ -160,72 +185,7 @@ public class MaxInExpressionParameterPaddingTest {
expectedInClause.append( "where p1_0.id not in " );
appendInClause( expectedInClause, MAX_COUNT );
expectedInClause.append( " and p1_0.id not in " );
appendInClause( expectedInClause, 1 );
assertThat( statementInspector.getSqlQueries().get( 0 ) ).endsWith( expectedInClause.toString() );
}
@Test
public void testInClauseParameterSplittingAfterLimit2(EntityManagerFactoryScope scope) {
final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
statementInspector.clear();
scope.inTransaction( entityManager -> entityManager
.createQuery( "select p from Person p where p.id in :ids" )
.setParameter( "ids", integerRangeList( 0, 18 ) )
.getResultList() );
StringBuilder expectedInClause = new StringBuilder();
expectedInClause.append( "where (p1_0.id in " );
appendInClause( expectedInClause, MAX_COUNT );
expectedInClause.append( " or p1_0.id in " );
appendInClause( expectedInClause, 4 );
expectedInClause.append( ')' );
assertThat( statementInspector.getSqlQueries().get( 0 ) ).endsWith( expectedInClause.toString() );
}
@Test
public void testInClauseParameterSplittingAfterLimit3(EntityManagerFactoryScope scope) {
final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
statementInspector.clear();
scope.inTransaction( entityManager -> entityManager
.createQuery( "select p from Person p where p.id in :ids" )
.setParameter( "ids", integerRangeList( 0, 33 ) )
.getResultList() );
StringBuilder expectedInClause = new StringBuilder();
expectedInClause.append( "where (p1_0.id in " );
appendInClause( expectedInClause, MAX_COUNT );
expectedInClause.append( " or p1_0.id in " );
appendInClause( expectedInClause, MAX_COUNT );
expectedInClause.append( " or p1_0.id in " );
appendInClause( expectedInClause, 4 );
expectedInClause.append( ')' );
assertThat( statementInspector.getSqlQueries().get( 0 ) ).endsWith( expectedInClause.toString() );
}
@TestForIssue(jiraKey = "HHH-16589")
@Test
public void testInClauseParameterSplittingAfterLimit4(EntityManagerFactoryScope scope) {
final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
statementInspector.clear();
scope.inTransaction( entityManager -> entityManager
.createQuery( "select p from Person p where p.id in :ids" )
.setParameter( "ids", integerRangeList( 0, 39 ) )
.getResultList() );
StringBuilder expectedInClause = new StringBuilder();
expectedInClause.append( "where (p1_0.id in " );
appendInClause( expectedInClause, MAX_COUNT );
expectedInClause.append( " or p1_0.id in " );
appendInClause( expectedInClause, MAX_COUNT );
expectedInClause.append( " or p1_0.id in " );
appendInClause( expectedInClause, MAX_COUNT );
expectedInClause.append( ')' );
assertThat( statementInspector.getSqlQueries().get( 0 ) ).endsWith( expectedInClause.toString() );
}