Skip to content

HHH-16283 - Integrate ParameterMarkerStrategy into NativeQuery #8798

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

HHH-16283 - Integrate ParameterMarkerStrategy into NativeQuery #8798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0bdbeb1
HHH-16283 - Integrate ParameterMarkerStrategy into NativeQuery
Aug 19, 2024
668ff21
Merge branch 'main' into HHH-16283
NathanQingyangXu Jun 22, 2025
ecad65d
HHH-16283 minor improvements
NathanQingyangXu Jun 23, 2025
5870503
HHH-16283 fix static analysis issue
NathanQingyangXu Jun 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 62 additions & 42 deletions hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@
import org.hibernate.query.sql.spi.ParameterInterpretation;
import org.hibernate.query.sql.spi.ParameterOccurrence;
import org.hibernate.query.sql.spi.SelectInterpretationsKey;
import org.hibernate.service.ServiceRegistry;
import org.hibernate.service.spi.ServiceRegistryImplementor;
import org.hibernate.sql.ast.internal.ParameterMarkerStrategyStandard;
import org.hibernate.sql.ast.spi.ParameterMarkerStrategy;
import org.hibernate.sql.exec.internal.CallbackImpl;
import org.hibernate.sql.exec.spi.Callback;
import org.hibernate.sql.results.graph.Fetchable;
Expand Down Expand Up @@ -486,13 +490,20 @@ private ParameterInterpretation resolveParameterInterpretation(

private static ParameterInterpretationImpl parameterInterpretation(
String sqlString, SharedSessionContractImplementor session) {
final ParameterRecognizerImpl parameterRecognizer = new ParameterRecognizerImpl();
session.getFactory().getServiceRegistry()
final ServiceRegistryImplementor serviceRegistry = session.getFactory().getServiceRegistry();
final ParameterMarkerStrategy parameterMarkerStrategy = getNullSafeParameterMarkerStrategy( serviceRegistry );
final ParameterRecognizerImpl parameterRecognizer = new ParameterRecognizerImpl( parameterMarkerStrategy );
serviceRegistry
.requireService( NativeQueryInterpreter.class )
.recognizeParameters( sqlString, parameterRecognizer );
return new ParameterInterpretationImpl( parameterRecognizer );
}

private static ParameterMarkerStrategy getNullSafeParameterMarkerStrategy(ServiceRegistry serviceRegistry) {
final ParameterMarkerStrategy parameterMarkerStrategy = serviceRegistry.getService( ParameterMarkerStrategy.class );
return parameterMarkerStrategy == null ? ParameterMarkerStrategyStandard.INSTANCE : parameterMarkerStrategy;
}

protected void applyOptions(NamedNativeQueryMemento<?> memento) {
super.applyOptions( memento );

Expand Down Expand Up @@ -882,37 +893,55 @@ protected String expandParameterLists() {

StringBuilder sql = null;

final ParameterMarkerStrategy parameterMarkerStrategy = getNullSafeParameterMarkerStrategy( factory.getServiceRegistry() );

// Handle parameter lists
int offset = 0;
for ( ParameterOccurrence occurrence : parameterOccurrences ) {
int expandedParameterPosition = 1;
for ( int originalParameterPosition = 1; originalParameterPosition <= parameterOccurrences.size(); originalParameterPosition++ ) {
final ParameterOccurrence occurrence = parameterOccurrences.get( originalParameterPosition - 1 );
final QueryParameterImplementor<?> queryParameter = occurrence.parameter();
final QueryParameterBinding<?> binding = parameterBindings.getBinding( queryParameter );
String occurenceReplacement = null;
int expandedParameterPositionIncrement = 1;
if ( binding.isMultiValued() ) {
final int bindValueCount = binding.getBindValues().size();
logTooManyExpressions( inExprLimit, bindValueCount, dialect, queryParameter );
final int sourcePosition = occurrence.sourcePosition();
if ( sourcePosition >= 0 ) {
if ( occurrence.sourcePosition() >= 0 ) {
// check if placeholder is already immediately enclosed in parentheses
// (ignoring whitespace)
final boolean isEnclosedInParens = isEnclosedInParens( sourcePosition );
final boolean isEnclosedInParens = isEnclosedInParens( occurrence );
// short-circuit for performance when only 1 value and the
// placeholder is already enclosed in parentheses...
if ( bindValueCount != 1 || !isEnclosedInParens ) {
if ( sql == null ) {
sql = new StringBuilder( sqlString.length() + 20 );
sql.append( sqlString );
}
if ( bindValueCount != 1 || !isEnclosedInParens || expandedParameterPosition != originalParameterPosition) {
final int bindValueMaxCount =
determineBindValueMaxCount( paddingEnabled, inExprLimit, bindValueCount );
final String expansionListAsString =
expandList( bindValueMaxCount, isEnclosedInParens );
final int start = sourcePosition + offset;
final int end = start + 1;
sql.replace( start, end, expansionListAsString );
offset += expansionListAsString.length() - 1;
occurenceReplacement =
expandList( bindValueMaxCount, isEnclosedInParens, parameterMarkerStrategy, expandedParameterPosition );
expandedParameterPositionIncrement = bindValueCount;
}
}
}
else if ( expandedParameterPosition != originalParameterPosition ) {
final String oldParameterMarker = parameterMarkerStrategy.createMarker( originalParameterPosition,
null );
final String newParameterMarker = parameterMarkerStrategy.createMarker( expandedParameterPosition,
null );
if ( !oldParameterMarker.equals( newParameterMarker ) ) {
occurenceReplacement = newParameterMarker;
}
}
if (occurenceReplacement != null) {
final int start = occurrence.sourcePosition() + offset;
final int end = start + occurrence.length();
if ( sql == null ) {
sql = new StringBuilder( sqlString.length() + 20 );
sql.append( sqlString );
}
sql.replace( start, end, occurenceReplacement );
offset += occurenceReplacement.length() - occurrence.length();
}
expandedParameterPosition += expandedParameterPositionIncrement;
}
return sql == null ? sqlString : sql.toString();
}
Expand All @@ -932,49 +961,40 @@ private static void logTooManyExpressions(
}
}

private static String expandList(int bindValueMaxCount, boolean isEnclosedInParens) {
private static String expandList(int bindValueMaxCount, boolean isEnclosedInParens, ParameterMarkerStrategy parameterMarkerStrategy, int parameterStartPosition) {
// HHH-8901
if ( bindValueMaxCount == 0 ) {
return isEnclosedInParens ? "null" : "(null)";
}
else {
// Shift 1 bit instead of multiplication by 2
final char[] chars;
if ( isEnclosedInParens ) {
chars = new char[(bindValueMaxCount << 1) - 1];
chars[0] = '?';
for ( int i = 1; i < bindValueMaxCount; i++ ) {
final int index = i << 1;
chars[index - 1] = ',';
chars[index] = '?';
}
final String firstParameterMarker = parameterMarkerStrategy.createMarker( parameterStartPosition, null );
final int estimatedLength = bindValueMaxCount * ( firstParameterMarker.length() + 1 ) - 1 + ( isEnclosedInParens ? 0 : 2 );
final StringBuilder stringBuilder = new StringBuilder( estimatedLength );
if ( ! isEnclosedInParens ) {
stringBuilder.append( '(' );
}
else {
chars = new char[(bindValueMaxCount << 1) + 1];
chars[0] = '(';
chars[1] = '?';
for ( int i = 1; i < bindValueMaxCount; i++ ) {
final int index = i << 1;
chars[index] = ',';
chars[index + 1] = '?';
}
chars[chars.length - 1] = ')';
stringBuilder.append( firstParameterMarker );
for ( int i = 1; i < bindValueMaxCount; i++ ) {
stringBuilder.append( ',' ).append( parameterMarkerStrategy.createMarker( parameterStartPosition + i, null ) );
}
if ( ! isEnclosedInParens ) {
stringBuilder.append( ')' );
}
return new String( chars );
return stringBuilder.toString();
}
}

private boolean isEnclosedInParens(int sourcePosition) {
private boolean isEnclosedInParens(ParameterOccurrence occurrence) {
boolean isEnclosedInParens = true;
for ( int i = sourcePosition - 1; i >= 0; i-- ) {
for ( int i = occurrence.sourcePosition() - 1; i >= 0; i-- ) {
final char ch = sqlString.charAt( i );
if ( !isWhitespace( ch ) ) {
isEnclosedInParens = ch == '(';
break;
}
}
if ( isEnclosedInParens ) {
for ( int i = sourcePosition + 1; i < sqlString.length(); i++ ) {
for ( int i = occurrence.sourcePosition() + occurrence.length(); i < sqlString.length(); i++ ) {
final char ch = sqlString.charAt( i );
if ( !isWhitespace( ch ) ) {
isEnclosedInParens = ch == ')';
Expand Down
44 changes: 23 additions & 21 deletions hibernate-core/src/main/java/org/hibernate/query/sql/internal/ParameterRecognizerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.hibernate.query.spi.QueryParameterImplementor;
import org.hibernate.query.sql.spi.ParameterOccurrence;
import org.hibernate.query.sql.spi.ParameterRecognizer;
import org.hibernate.sql.ast.spi.ParameterMarkerStrategy;

/**
* @author Steve Ebersole
Expand All @@ -36,11 +37,17 @@ private enum ParameterStyle {

private int ordinalParameterImplicitPosition;

private int nextParameterMarkerPosition;
private final ParameterMarkerStrategy parameterMarkerStrategy;

private List<ParameterOccurrence> parameterList;
private final StringBuilder sqlStringBuffer = new StringBuilder();

public ParameterRecognizerImpl() {
public ParameterRecognizerImpl(ParameterMarkerStrategy parameterMarkerStrategy) {
assert parameterMarkerStrategy != null;
ordinalParameterImplicitPosition = 1;
nextParameterMarkerPosition = 1;
this.parameterMarkerStrategy = parameterMarkerStrategy;
}

@Override
Expand All @@ -57,13 +64,13 @@ public void complete() {
if ( first ) {
throw new ParameterLabelException(
"Ordinal parameter labels start from '?" + position + "'"
+ " (ordinal parameters must be labelled from '?1')"
+ " (ordinal parameters must be labelled from '?1')"
);
}
else {
throw new ParameterLabelException(
"Gap between '?" + previous + "' and '?" + position + "' in ordinal parameter labels"
+ " (ordinal parameters must be labelled sequentially)"
+ " (ordinal parameters must be labelled sequentially)"
);
}
}
Expand Down Expand Up @@ -117,12 +124,7 @@ else if ( parameterStyle != ParameterStyle.JDBC ) {
positionalQueryParameters.put( implicitPosition, parameter );
}

if ( parameterList == null ) {
parameterList = new ArrayList<>();
}

parameterList.add( new ParameterOccurrence( parameter, sqlStringBuffer.length() ) );
sqlStringBuffer.append( "?" );
recognizeParameter( parameter );
}

@Override
Expand All @@ -148,12 +150,7 @@ else if ( parameterStyle != ParameterStyle.NAMED ) {
namedQueryParameters.put( name, parameter );
}

if ( parameterList == null ) {
parameterList = new ArrayList<>();
}

parameterList.add( new ParameterOccurrence( parameter, sqlStringBuffer.length() ) );
sqlStringBuffer.append( "?" );
recognizeParameter( parameter );
}

@Override
Expand Down Expand Up @@ -183,16 +180,21 @@ else if ( parameterStyle != ParameterStyle.NAMED ) {
positionalQueryParameters.put( position, parameter );
}

if ( parameterList == null ) {
parameterList = new ArrayList<>();
}

parameterList.add( new ParameterOccurrence( parameter, sqlStringBuffer.length() ) );
sqlStringBuffer.append( "?" );
recognizeParameter( parameter );
}

@Override
public void other(char character) {
sqlStringBuffer.append( character );
}

private void recognizeParameter(QueryParameterImplementor<?> parameter) {
final String marker = parameterMarkerStrategy.createMarker( nextParameterMarkerPosition++, null );
final int markerLength = marker.length();
if ( parameterList == null ) {
parameterList = new ArrayList<>();
}
sqlStringBuffer.append( marker );
parameterList.add( new ParameterOccurrence( parameter, sqlStringBuffer.length() - markerLength, markerLength ) );
}
}
2 changes: 1 addition & 1 deletion hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
/**
* @author Christian Beikov
*/
public record ParameterOccurrence(QueryParameterImplementor<?> parameter, int sourcePosition) {
public record ParameterOccurrence(QueryParameterImplementor<?> parameter, int sourcePosition, int length) {
}
14 changes: 7 additions & 7 deletions ...rnate-core/src/test/java/org/hibernate/orm/test/sql/ast/ParameterMarkerStrategyTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
*/
package org.hibernate.orm.test.sql.ast;

import java.util.List;

import org.hibernate.annotations.Filter;
import org.hibernate.annotations.FilterDef;
Expand All @@ -16,7 +15,6 @@
import org.hibernate.testing.jdbc.SQLStatementInspector;
import org.hibernate.testing.orm.domain.gambit.EntityOfBasics;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.FailureExpected;
import org.hibernate.testing.orm.junit.Jira;
import org.hibernate.testing.orm.junit.RequiresDialect;
import org.hibernate.testing.orm.junit.ServiceRegistry;
Expand All @@ -31,6 +29,8 @@
import jakarta.persistence.Table;
import jakarta.persistence.Version;

import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;
import static org.hibernate.internal.util.StringHelper.*;

Expand Down Expand Up @@ -125,14 +125,13 @@ public void testMutations(SessionFactoryScope scope) {
}

@Test
@FailureExpected
@Jira( "https://hibernate.atlassian.net/browse/HHH-16283" )
public void testNativeQuery(SessionFactoryScope scope) {
final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();

statementInspector.clear();
scope.inTransaction( (session) -> {
session.createNativeQuery( "select count(1) from filtered_entity e where e.region = :region" )
session.createNativeQuery( "select count(1) from filtered_entity e where e.region = :region", Integer.class )
.setParameter( "region", "ABC" )
.uniqueResult();

Expand All @@ -143,13 +142,14 @@ public void testNativeQuery(SessionFactoryScope scope) {

statementInspector.clear();
scope.inTransaction( (session) -> {
session.createNativeQuery( "select count(1) from filtered_entity e where e.region in (:region)" )
session.createNativeQuery( "select count(1) from filtered_entity e where e.region in (:region) and e.name = :name", Integer.class )
.setParameterList( "region", List.of( "ABC", "DEF" ) )
.setParameter( "name", "It" )
.uniqueResult();

assertThat( statementInspector.getSqlQueries() ).hasSize( 1 );
assertThat( count( statementInspector.getSqlQueries().get( 0 ), "?" ) ).isEqualTo( 1 );
assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?1" );
assertThat( count( statementInspector.getSqlQueries().get( 0 ), "?" ) ).isEqualTo( 3 );
assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?1" ).contains( "?2" ).contains( "?3" );
} );
}

Expand Down