From 0bdbeb1677e028b188f01caa16ecd380d0b352a1 Mon Sep 17 00:00:00 2001 From: "nathan.xu" Date: Mon, 19 Aug 2024 18:53:27 -0400 Subject: [PATCH 01/10] HHH-16283 - Integrate ParameterMarkerStrategy into NativeQuery --- .../query/sql/internal/NativeQueryImpl.java | 100 ++++++++++-------- .../sql/internal/ParameterRecognizerImpl.java | 43 ++++---- .../query/sql/spi/ParameterOccurrence.java | 8 +- .../sql/ast/spi/ParameterMarkerStrategy.java | 2 +- .../sql/ast/ParameterMarkerStrategyTests.java | 6 +- 5 files changed, 89 insertions(+), 70 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java index 691252daa6ad..c3498cee8de6 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java @@ -88,6 +88,7 @@ import org.hibernate.query.sql.spi.ParameterInterpretation; import org.hibernate.query.sql.spi.ParameterOccurrence; import org.hibernate.query.sql.spi.SelectInterpretationsKey; +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.jdbc.spi.JdbcValuesMappingProducer; @@ -368,7 +369,8 @@ private ParameterInterpretation resolveParameterInterpretation( return interpretationCache.resolveNativeQueryParameters( sqlString, s -> { - final ParameterRecognizerImpl parameterRecognizer = new ParameterRecognizerImpl(); + final ParameterMarkerStrategy parameterMarkerStrategy = sessionFactory.getServiceRegistry().getService( ParameterMarkerStrategy.class ); + final ParameterRecognizerImpl parameterRecognizer = new ParameterRecognizerImpl( parameterMarkerStrategy ); session.getFactory().getServiceRegistry() .requireService( NativeQueryInterpreter.class ) @@ -734,23 +736,36 @@ protected String expandParameterLists() { // Some DBs limit number of IN expressions. For now, warn... final SessionFactoryImplementor sessionFactory = getSessionFactory(); final Dialect dialect = sessionFactory.getJdbcServices().getDialect(); + + final ParameterMarkerStrategy parameterMarkerStrategy = sessionFactory.getServiceRegistry().getService( ParameterMarkerStrategy.class ); + final boolean paddingEnabled = sessionFactory.getSessionFactoryOptions().inClauseParameterPaddingEnabled(); final int inExprLimit = dialect.getInExpressionCountLimit(); StringBuilder sb = null; + StringBuilder occurrenceExpansionSB = null; // Handle parameter lists - int offset = 0; - for ( ParameterOccurrence occurrence : parameterOccurrences ) { + int sourceOffset = 0; + int expandedParamPosition = 1; + for ( int originalParamPosition = 1; originalParamPosition <= parameterOccurrences.size(); originalParamPosition++ ) { + final ParameterOccurrence occurrence = parameterOccurrences.get( originalParamPosition - 1 ); final QueryParameterImplementor queryParameter = occurrence.getParameter(); final QueryParameterBinding binding = parameterBindings.getBinding( queryParameter ); if ( !binding.isMultiValued() ) { + if ( originalParamPosition != expandedParamPosition ) { + if ( sb == null ) { + sb = new StringBuilder( sqlString ); + } + sourceOffset = getNewSourceOffsetAfterReplacement( sb, sourceOffset, occurrence, parameterMarkerStrategy.createMarker( expandedParamPosition, null ) ); + } + expandedParamPosition++; continue; } final Collection bindValues = binding.getBindValues(); - int bindValueCount = bindValues.size(); - int bindValueMaxCount = determineBindValueMaxCount( paddingEnabled, inExprLimit, bindValueCount ); + final int bindValueCount = bindValues.size(); + final int bindValueMaxCount = determineBindValueMaxCount( paddingEnabled, inExprLimit, bindValueCount ); if ( inExprLimit > 0 && bindValueCount > inExprLimit ) { log.tooManyInExpressions( @@ -765,6 +780,7 @@ protected String expandParameterLists() { final int sourcePosition = occurrence.getSourcePosition(); if ( sourcePosition < 0 ) { + expandedParamPosition++; continue; } @@ -779,7 +795,7 @@ protected String expandParameterLists() { } } if ( isEnclosedInParens ) { - for ( int i = sourcePosition + 1; i < sqlString.length(); i++ ) { + for ( int i = sourcePosition + occurrence.getLength(); i < sqlString.length(); i++ ) { final char ch = sqlString.charAt( i ); if ( !Character.isWhitespace( ch ) ) { isEnclosedInParens = ch == ')'; @@ -788,62 +804,62 @@ protected String expandParameterLists() { } } - if ( bindValueCount == 1 && isEnclosedInParens ) { + if ( bindValueCount == 1 && isEnclosedInParens && expandedParamPosition == originalParamPosition ) { // short-circuit for performance when only 1 value and the // placeholder is already enclosed in parentheses... + expandedParamPosition++; continue; } if ( sb == null ) { - sb = new StringBuilder( sqlString.length() + 20 ); - sb.append( sqlString ); + sb = new StringBuilder( sqlString ); + } + + if ( occurrenceExpansionSB == null ) { + occurrenceExpansionSB = new StringBuilder(); + } + else { + occurrenceExpansionSB.setLength( 0 ); + } + + if ( !isEnclosedInParens ) { + occurrenceExpansionSB.append( '(' ); } - final String expansionListAsString; // HHH-8901 if ( bindValueMaxCount == 0 ) { - if ( isEnclosedInParens ) { - expansionListAsString = "null"; - } - else { - expansionListAsString = "(null)"; - } + occurrenceExpansionSB.append( "null" ); } else { - // Shift 1 bit instead of multiplication by 2 - 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] = '?'; + for ( int i = 0; i < bindValueMaxCount; i++ ) { + final String marker = parameterMarkerStrategy.createMarker( + expandedParamPosition + i, + null + ); + occurrenceExpansionSB.append( marker ); + if ( i + 1 < bindValueMaxCount ) { + occurrenceExpansionSB.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] = ')'; - } - - expansionListAsString = new String(chars); } + if ( !isEnclosedInParens ) { + occurrenceExpansionSB.append( ')' ); + } + + sourceOffset = getNewSourceOffsetAfterReplacement( sb, sourceOffset, occurrence, occurrenceExpansionSB.toString() ); - final int start = sourcePosition + offset; - final int end = start + 1; - sb.replace( start, end, expansionListAsString ); - offset += expansionListAsString.length() - 1; + expandedParamPosition += bindValueMaxCount; } return sb == null ? sqlString : sb.toString(); } + private int getNewSourceOffsetAfterReplacement(StringBuilder sb, int sourceOffset, ParameterOccurrence occurrence, String replacement) { + final int start = occurrence.getSourcePosition() + sourceOffset; + final int end = start + occurrence.getLength(); + sb.replace( start, end, replacement ); + return sourceOffset + ( replacement.length() - occurrence.getLength() ); + } + public static int determineBindValueMaxCount(boolean paddingEnabled, int inExprLimit, int bindValueCount) { int bindValueMaxCount = bindValueCount; diff --git a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/ParameterRecognizerImpl.java b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/ParameterRecognizerImpl.java index 9fcc306483c0..c9d2b6e5ae2b 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/ParameterRecognizerImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/ParameterRecognizerImpl.java @@ -18,6 +18,8 @@ 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.internal.ParameterMarkerStrategyStandard; +import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; /** * @author Steve Ebersole @@ -34,13 +36,15 @@ private enum ParameterStyle { private Map> namedQueryParameters; private Map> positionalQueryParameters; - private int ordinalParameterImplicitPosition; + private int parameterImplicitPosition; + private final ParameterMarkerStrategy parameterMarkerStrategy; private List parameterList; private final StringBuilder sqlStringBuffer = new StringBuilder(); - public ParameterRecognizerImpl() { - ordinalParameterImplicitPosition = 1; + public ParameterRecognizerImpl(ParameterMarkerStrategy parameterMarkerStrategy) { + this.parameterMarkerStrategy = parameterMarkerStrategy == null ? ParameterMarkerStrategyStandard.INSTANCE : parameterMarkerStrategy; + parameterImplicitPosition = 1; } @Override @@ -101,7 +105,7 @@ else if ( parameterStyle != ParameterStyle.JDBC ) { throw new ParameterRecognitionException( "Cannot mix parameter styles between JDBC-style, ordinal and named in the same query" ); } - int implicitPosition = ordinalParameterImplicitPosition++; + int implicitPosition = parameterImplicitPosition++; QueryParameterImplementor parameter = null; @@ -117,12 +121,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, implicitPosition ); } @Override @@ -148,12 +147,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, parameterImplicitPosition++ ); } @Override @@ -183,16 +177,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, parameterImplicitPosition++ ); } @Override public void other(char character) { sqlStringBuffer.append( character ); } + + private void recognizeParameter(QueryParameterImplementor parameter, int position) { + final String marker = parameterMarkerStrategy.createMarker( position, null ); + final int markerLength = marker.length(); + if ( parameterList == null ) { + parameterList = new ArrayList<>(); + } + sqlStringBuffer.append( marker ); + parameterList.add( new ParameterOccurrence( parameter, sqlStringBuffer.length() - markerLength, markerLength ) ); + } } diff --git a/hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java b/hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java index 951db3d84597..30cb75cca784 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java @@ -13,10 +13,12 @@ public final class ParameterOccurrence { private final QueryParameterImplementor parameter; private final int sourcePosition; + private final int length; - public ParameterOccurrence(QueryParameterImplementor parameter, int sourcePosition) { + public ParameterOccurrence(QueryParameterImplementor parameter, int sourcePosition, int length) { this.parameter = parameter; this.sourcePosition = sourcePosition; + this.length = length; } public QueryParameterImplementor getParameter() { @@ -26,4 +28,8 @@ public QueryParameterImplementor getParameter() { public int getSourcePosition() { return sourcePosition; } + + public int getLength() { + return length; + } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/ParameterMarkerStrategy.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/ParameterMarkerStrategy.java index a23f0d71d3ae..a4b3914f6f17 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/ParameterMarkerStrategy.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/ParameterMarkerStrategy.java @@ -8,7 +8,7 @@ import org.hibernate.type.descriptor.jdbc.JdbcType; /** - * Strategy for generating parameter markers used in {@linkplain java.sql.PreparedStatement preparable} SQL strings. + * Strategy for generating parameter markers used in {@linkplain java.sql.PreparedStatement preparable} and native SQL strings. *

* Generally Hibernate will use the JDBC standard marker - {@code ?}. Many JDBC drivers support the * use of the "native" marker syntax of the underlying database - e.g. {@code $n}, {@code ?n}, ... diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/ParameterMarkerStrategyTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/ParameterMarkerStrategyTests.java index 14e932dc31bf..47f4888feff5 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/ParameterMarkerStrategyTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/ParameterMarkerStrategyTests.java @@ -16,7 +16,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; @@ -125,7 +124,6 @@ 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(); @@ -148,8 +146,8 @@ public void testNativeQuery(SessionFactoryScope scope) { .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( 2 ); + assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?1", "?2" ); } ); } From ecad65dcd1c20bf728b4511ea708689809e476e8 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Sun, 22 Jun 2025 22:29:21 -0400 Subject: [PATCH 02/10] HHH-16283 minor improvements --- .../sql/internal/ParameterRecognizerImpl.java | 27 ++++++++++--------- .../sql/ast/spi/ParameterMarkerStrategy.java | 2 +- .../sql/ast/ParameterMarkerStrategyTests.java | 12 +++++---- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/ParameterRecognizerImpl.java b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/ParameterRecognizerImpl.java index b01a503d7943..901c03f06a32 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/ParameterRecognizerImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/ParameterRecognizerImpl.java @@ -18,7 +18,6 @@ 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.internal.ParameterMarkerStrategyStandard; import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; /** @@ -36,15 +35,19 @@ private enum ParameterStyle { private Map> namedQueryParameters; private Map> positionalQueryParameters; - private int parameterImplicitPosition; + private int ordinalParameterImplicitPosition; + + private int nextParameterMarkerPosition; private final ParameterMarkerStrategy parameterMarkerStrategy; private List parameterList; private final StringBuilder sqlStringBuffer = new StringBuilder(); public ParameterRecognizerImpl(ParameterMarkerStrategy parameterMarkerStrategy) { - this.parameterMarkerStrategy = parameterMarkerStrategy == null ? ParameterMarkerStrategyStandard.INSTANCE : parameterMarkerStrategy; - parameterImplicitPosition = 1; + assert parameterMarkerStrategy != null; + ordinalParameterImplicitPosition = 1; + nextParameterMarkerPosition = 1; + this.parameterMarkerStrategy = parameterMarkerStrategy; } @Override @@ -61,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)" ); } } @@ -105,7 +108,7 @@ else if ( parameterStyle != ParameterStyle.JDBC ) { throw new ParameterRecognitionException( "Cannot mix parameter styles between JDBC-style, ordinal and named in the same query" ); } - int implicitPosition = parameterImplicitPosition++; + int implicitPosition = ordinalParameterImplicitPosition++; QueryParameterImplementor parameter = null; @@ -121,7 +124,7 @@ else if ( parameterStyle != ParameterStyle.JDBC ) { positionalQueryParameters.put( implicitPosition, parameter ); } - recognizeParameter( parameter, implicitPosition ); + recognizeParameter( parameter ); } @Override @@ -147,7 +150,7 @@ else if ( parameterStyle != ParameterStyle.NAMED ) { namedQueryParameters.put( name, parameter ); } - recognizeParameter( parameter, parameterImplicitPosition++ ); + recognizeParameter( parameter ); } @Override @@ -177,7 +180,7 @@ else if ( parameterStyle != ParameterStyle.NAMED ) { positionalQueryParameters.put( position, parameter ); } - recognizeParameter( parameter, parameterImplicitPosition++ ); + recognizeParameter( parameter ); } @Override @@ -185,8 +188,8 @@ public void other(char character) { sqlStringBuffer.append( character ); } - private void recognizeParameter(QueryParameterImplementor parameter, int position) { - final String marker = parameterMarkerStrategy.createMarker( position, null ); + private void recognizeParameter(QueryParameterImplementor parameter) { + final String marker = parameterMarkerStrategy.createMarker( nextParameterMarkerPosition++, null ); final int markerLength = marker.length(); if ( parameterList == null ) { parameterList = new ArrayList<>(); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/ParameterMarkerStrategy.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/ParameterMarkerStrategy.java index e17e72961763..428d0d7f7321 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/ParameterMarkerStrategy.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/ParameterMarkerStrategy.java @@ -8,7 +8,7 @@ import org.hibernate.type.descriptor.jdbc.JdbcType; /** - * Strategy for generating parameter markers used in {@linkplain java.sql.PreparedStatement preparable} and native SQL strings. + * Strategy for generating parameter markers used in {@linkplain java.sql.PreparedStatement preparable} SQL strings. *

* Generally Hibernate will use the JDBC standard marker - {@code ?}. Many JDBC drivers support the * use of the "native" marker syntax of the underlying database - e.g. {@code $n}, {@code ?n}, ... diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/ParameterMarkerStrategyTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/ParameterMarkerStrategyTests.java index c27c1cde8042..a05673dc0d14 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/ParameterMarkerStrategyTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/ParameterMarkerStrategyTests.java @@ -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; @@ -30,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.*; @@ -130,7 +131,7 @@ public void testNativeQuery(SessionFactoryScope scope) { 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(); @@ -141,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( 2 ); - assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?1", "?2" ); + assertThat( count( statementInspector.getSqlQueries().get( 0 ), "?" ) ).isEqualTo( 3 ); + assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?1" ).contains( "?2" ).contains( "?3" ); } ); } From 58705035d14783fcb09d3e6609d54bdf86c4614e Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Mon, 23 Jun 2025 08:43:37 -0400 Subject: [PATCH 03/10] HHH-16283 fix static analysis issue --- .../hibernate/query/sql/internal/NativeQueryImpl.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java index 40c6034e1a1b..05058956f9da 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java @@ -491,7 +491,7 @@ private ParameterInterpretation resolveParameterInterpretation( private static ParameterInterpretationImpl parameterInterpretation( String sqlString, SharedSessionContractImplementor session) { final ServiceRegistryImplementor serviceRegistry = session.getFactory().getServiceRegistry(); - final ParameterMarkerStrategy parameterMarkerStrategy = getNullSafeParameterMarkerStrategy(serviceRegistry); + final ParameterMarkerStrategy parameterMarkerStrategy = getNullSafeParameterMarkerStrategy( serviceRegistry ); final ParameterRecognizerImpl parameterRecognizer = new ParameterRecognizerImpl( parameterMarkerStrategy ); serviceRegistry .requireService( NativeQueryInterpreter.class ) @@ -921,7 +921,8 @@ protected String expandParameterLists() { expandedParameterPositionIncrement = bindValueCount; } } - } else if (expandedParameterPosition != originalParameterPosition) { + } + else if ( expandedParameterPosition != originalParameterPosition ) { final String oldParameterMarker = parameterMarkerStrategy.createMarker( originalParameterPosition, null ); final String newParameterMarker = parameterMarkerStrategy.createMarker( expandedParameterPosition, @@ -937,7 +938,7 @@ protected String expandParameterLists() { sql = new StringBuilder( sqlString.length() + 20 ); sql.append( sqlString ); } - sql.replace( start, end, occurenceReplacement); + sql.replace( start, end, occurenceReplacement ); offset += occurenceReplacement.length() - occurrence.length(); } expandedParameterPosition += expandedParameterPositionIncrement; @@ -967,7 +968,7 @@ private static String expandList(int bindValueMaxCount, boolean isEnclosedInPare } else { final String firstParameterMarker = parameterMarkerStrategy.createMarker( parameterStartPosition, null ); - final int estimatedLength = bindValueMaxCount * ( firstParameterMarker.length() + 1 ) - 1 + ( isEnclosedInParens ? 2 : 0 ); + final int estimatedLength = bindValueMaxCount * ( firstParameterMarker.length() + 1 ) - 1 + ( isEnclosedInParens ? 0 : 2 ); final StringBuilder stringBuilder = new StringBuilder( estimatedLength ); if ( ! isEnclosedInParens ) { stringBuilder.append( '(' ); From e557f4956faea5ffd3067c4c1e1dec4d6c7a6449 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Tue, 1 Jul 2025 08:07:16 -0400 Subject: [PATCH 04/10] add LimitHandler processing --- .../dialect/pagination/LimitHandler.java | 8 + .../pagination/OffsetFetchLimitHandler.java | 13 +- .../internal/DeferredResultSetAccess.java | 12 +- .../NativeParameterMarkerStrategyTests.java | 176 ++++++++++++++++++ .../sql/ast/ParameterMarkerStrategyTests.java | 31 --- .../orm/junit/DialectFeatureChecks.java | 7 + 6 files changed, 213 insertions(+), 34 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/NativeParameterMarkerStrategyTests.java diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java index 936c5f49bdb5..9d5dae20fbc3 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java @@ -7,8 +7,10 @@ import java.sql.PreparedStatement; import java.sql.SQLException; +import org.checkerframework.checker.nullness.qual.Nullable; import org.hibernate.query.spi.Limit; import org.hibernate.query.spi.QueryOptions; +import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; /** * Contract defining dialect-specific limit and offset handling. @@ -41,10 +43,16 @@ public interface LimitHandler { String processSql(String sql, Limit limit); + @Deprecated // Never called directly by Hibernate ORM default String processSql(String sql, Limit limit, QueryOptions queryOptions) { return processSql( sql, limit ); } + // This is the one called directly by Hibernate ORM + default String processSql(String sql, int jdbcParameterBindingsCnt, @Nullable ParameterMarkerStrategy parameterMarkerStrategy, Limit limit, QueryOptions queryOptions) { + return processSql( sql, limit ); + } + int bindLimitParametersAtStartOfQuery(Limit limit, PreparedStatement statement, int index) throws SQLException; int bindLimitParametersAtEndOfQuery(Limit limit, PreparedStatement statement, int index) throws SQLException; diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/OffsetFetchLimitHandler.java b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/OffsetFetchLimitHandler.java index 0697d1c0cac5..577de737e7ab 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/OffsetFetchLimitHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/OffsetFetchLimitHandler.java @@ -5,6 +5,10 @@ package org.hibernate.dialect.pagination; import org.hibernate.query.spi.Limit; +import org.hibernate.query.spi.QueryOptions; +import org.hibernate.sql.ast.internal.ParameterMarkerStrategyStandard; +import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; +import org.hibernate.type.descriptor.jdbc.IntegerJdbcType; /** * A {@link LimitHandler} for databases which support the @@ -25,7 +29,12 @@ public OffsetFetchLimitHandler(boolean variableLimit) { @Override public String processSql(String sql, Limit limit) { + return processSql( sql, 0, ParameterMarkerStrategyStandard.INSTANCE, limit, QueryOptions.NONE ); + } + + @Override + public String processSql(String sql, int jdbcParameterBindingsCnt, ParameterMarkerStrategy parameterMarkerStrategy, Limit limit, QueryOptions queryOptions) { boolean hasFirstRow = hasFirstRow(limit); boolean hasMaxRows = hasMaxRows(limit); @@ -40,7 +49,7 @@ public String processSql(String sql, Limit limit) { if ( hasFirstRow ) { offsetFetch.append( " offset " ); if ( supportsVariableLimit() ) { - offsetFetch.append( "?" ); + offsetFetch.append( parameterMarkerStrategy.createMarker( ++jdbcParameterBindingsCnt, IntegerJdbcType.INSTANCE ) ); } else { offsetFetch.append( limit.getFirstRow() ); @@ -58,7 +67,7 @@ public String processSql(String sql, Limit limit) { offsetFetch.append( " fetch first " ); } if ( supportsVariableLimit() ) { - offsetFetch.append( "?" ); + offsetFetch.append( parameterMarkerStrategy.createMarker( ++jdbcParameterBindingsCnt, IntegerJdbcType.INSTANCE ) ); } else { offsetFetch.append( getMaxOrLimit( limit ) ); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java b/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java index 9f8dfb979251..2386bc4b087b 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java @@ -27,6 +27,7 @@ import org.hibernate.query.spi.QueryOptions; import org.hibernate.resource.jdbc.spi.JdbcSessionContext; import org.hibernate.resource.jdbc.spi.LogicalConnectionImplementor; +import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; import org.hibernate.sql.exec.spi.ExecutionContext; import org.hibernate.sql.exec.spi.JdbcLockStrategy; import org.hibernate.sql.exec.spi.JdbcOperationQuerySelect; @@ -91,7 +92,16 @@ public DeferredResultSetAccess( limit = queryOptions.getLimit(); final boolean hasLimit = isHasLimit( jdbcSelect ); limitHandler = hasLimit ? NoopLimitHandler.NO_LIMIT : dialect.getLimitHandler(); - final String sqlWithLimit = hasLimit ? sql : limitHandler.processSql( sql, limit, queryOptions ); + + final String sqlWithLimit; + if ( hasLimit ) { + sqlWithLimit = sql; + } + else { + final int jdbcBindingsCnt = jdbcParameterBindings.getBindings().size(); + final ParameterMarkerStrategy parameterMarkerStrategy = dialect.getNativeParameterMarkerStrategy(); + sqlWithLimit = limitHandler.processSql( sql, jdbcBindingsCnt, parameterMarkerStrategy, limit, queryOptions ); + } final LockOptions lockOptions = queryOptions.getLockOptions(); final JdbcLockStrategy jdbcLockStrategy = jdbcSelect.getLockStrategy(); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/NativeParameterMarkerStrategyTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/NativeParameterMarkerStrategyTests.java new file mode 100644 index 000000000000..44d1f76bc6de --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/NativeParameterMarkerStrategyTests.java @@ -0,0 +1,176 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.sql.ast; + +import java.util.List; + +import org.hibernate.query.NativeQuery; +import org.hibernate.query.sql.spi.NativeQueryImplementor; +import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; +import org.hibernate.type.descriptor.jdbc.IntegerJdbcType; +import org.hibernate.type.descriptor.jdbc.JdbcType; + +import org.hibernate.testing.jdbc.SQLStatementInspector; +import org.hibernate.testing.orm.junit.DialectContext; +import org.hibernate.testing.orm.junit.DialectFeatureChecks; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.Jira; +import org.hibernate.testing.orm.junit.RequiresDialectFeature; +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.SessionFactoryScopeAware; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Table; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Nathan Xu + */ +@ServiceRegistry( services = @ServiceRegistry.Service( + role = ParameterMarkerStrategy.class, + impl = NativeParameterMarkerStrategyTests.DialectParameterMarkerStrategy.class +) ) +@DomainModel( annotatedClasses = NativeParameterMarkerStrategyTests.Book.class ) +@SessionFactory( useCollectingStatementInspector = true ) +@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsNativeParameterMarker.class ) +@Jira( "https://hibernate.atlassian.net/browse/HHH-16283" ) +class NativeParameterMarkerStrategyTests implements SessionFactoryScopeAware { + + private enum ParameterStyle { + JDBC, + ORDINAL, + NAMED + } + + public static class DialectParameterMarkerStrategy implements ParameterMarkerStrategy { + @Override + public String createMarker(int position, JdbcType jdbcType) { + return DialectContext.getDialect().getNativeParameterMarkerStrategy().createMarker( position, jdbcType ); + } + } + + private SessionFactoryScope scope; + private SQLStatementInspector statementInspector; + + @Override + public void injectSessionFactoryScope(SessionFactoryScope scope) { + this.scope = scope; + } + + @BeforeEach + void setUp() { + statementInspector = scope.getCollectingStatementInspector(); + statementInspector.clear(); + } + + @ParameterizedTest + @EnumSource(ParameterStyle.class) + void testHappyPath(ParameterStyle style) { + scope.inTransaction( (session) -> { + final NativeQueryImplementor nativeQuery; + final var parameterValue = "War and Peace"; + if ( style == ParameterStyle.NAMED ) { + nativeQuery = session.createNativeQuery( "select * from books b where b.title = :title", Integer.class ) + .setParameter( "title", parameterValue ); + } else { + nativeQuery = session.createNativeQuery( "select * from books b where b.title = " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Integer.class ) + .setParameter( 1, parameterValue ); + }; + nativeQuery.uniqueResult(); + assertNativeQueryContainsMarkers( statementInspector, 1 ); + } ); + } + + @ParameterizedTest + @EnumSource(ParameterStyle.class) + void testParameterExpansion(ParameterStyle style) { + final var parameterValue = List.of( "Moby-Dick", "Don Quixote", "In Search of Lost Time" ); + + scope.inTransaction( (session) -> { + final NativeQuery nativeQuery; + if ( style == ParameterStyle.NAMED ) { + nativeQuery = session.createNativeQuery( "select * from books b where b.title in :titles", Integer.class ) + .setParameterList( "titles", parameterValue ); + } else { + nativeQuery = session.createNativeQuery( "select * from books b where b.title in " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Integer.class ) + .setParameterList( 1, parameterValue ); + }; + nativeQuery.list(); + assertNativeQueryContainsMarkers( statementInspector, parameterValue.size() ); + } ); + } + + @ParameterizedTest + @EnumSource(ParameterStyle.class) + void testLimitHandler(ParameterStyle style) { + scope.inTransaction( (session) -> { + final NativeQueryImplementor nativeQuery; + final var parameterValue = "Herman Melville"; + if ( style == ParameterStyle.NAMED ) { + nativeQuery = session.createNativeQuery( "select * from books b where b.author = :author", Integer.class ) + .setParameter( "author", parameterValue ); + } else { + nativeQuery = session.createNativeQuery( "select * from books b where b.author = " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Integer.class ) + .setParameter( 1, parameterValue ); + }; + nativeQuery.setFirstResult( 2 ).setMaxResults( 1 ).list(); + + assertNativeQueryContainsMarkers( statementInspector, 3 ); + } ); + } + + @ParameterizedTest + @EnumSource(ParameterStyle.class) + void test_parameterExpansionAndLimitHandler(ParameterStyle style) { + final var parameterValue = List.of( "Moby-Dick", "Don Quixote", "In Search of Lost Time" ); + + scope.inTransaction( (session) -> { + final NativeQueryImplementor nativeQuery; + if ( style == ParameterStyle.NAMED ) { + nativeQuery = session.createNativeQuery( "select * from books b where b.title in :titles", Integer.class ) + .setParameterList( "titles", parameterValue ); + } else { + nativeQuery = session.createNativeQuery( "select * from books b where b.title in " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Integer.class ) + .setParameterList( 1, parameterValue ); + }; + nativeQuery.setFirstResult( 1 ).setMaxResults( 3 ).list(); + + assertNativeQueryContainsMarkers( statementInspector, parameterValue.size() + 2 ); + } ); + } + + private void assertNativeQueryContainsMarkers(SQLStatementInspector statementInspector, int expectedMarkerNum) { + + final var strategy = new DialectParameterMarkerStrategy(); + + final var expectedMarkers = new String[expectedMarkerNum]; + for (int i = 1; i <= expectedMarkerNum; i++) { + expectedMarkers[i - 1] = strategy.createMarker( i, IntegerJdbcType.INSTANCE ); + } + + final var unexpectedMarker = strategy.createMarker( expectedMarkerNum + 1, IntegerJdbcType.INSTANCE ); + + assertThat( statementInspector.getSqlQueries() ) + .singleElement() + .satisfies( query -> assertThat( query ).contains( expectedMarkers ).doesNotContain( unexpectedMarker ) ); + } + + @Entity + @Table(name = "books") + static class Book { + @Id + String isbn; + String title; + String author; + } + +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/ParameterMarkerStrategyTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/ParameterMarkerStrategyTests.java index a05673dc0d14..f278e2ff3cad 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/ParameterMarkerStrategyTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/ParameterMarkerStrategyTests.java @@ -29,8 +29,6 @@ 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.*; @@ -124,35 +122,6 @@ public void testMutations(SessionFactoryScope scope) { } ); } - @Test - @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", Integer.class ) - .setParameter( "region", "ABC" ) - .uniqueResult(); - - assertThat( statementInspector.getSqlQueries() ).hasSize( 1 ); - assertThat( count( statementInspector.getSqlQueries().get( 0 ), "?" ) ).isEqualTo( 1 ); - assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?1" ); - } ); - - statementInspector.clear(); - scope.inTransaction( (session) -> { - 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( 3 ); - assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?1" ).contains( "?2" ).contains( "?3" ); - } ); - } - @AfterEach public void cleanUpTestData(SessionFactoryScope scope) { scope.getSessionFactory().getSchemaManager().truncate(); diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/DialectFeatureChecks.java b/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/DialectFeatureChecks.java index b27b78efe6d3..97dc4cc55c86 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/DialectFeatureChecks.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/DialectFeatureChecks.java @@ -1035,6 +1035,13 @@ public boolean apply(Dialect dialect) { } } + public static class SupportsNativeParameterMarker implements DialectFeatureCheck { + @Override + public boolean apply(Dialect dialect) { + return dialect.getNativeParameterMarkerStrategy() != null; + } + } + private static final HashMap FUNCTION_REGISTRIES = new HashMap<>(); public static boolean definesFunction(Dialect dialect, String functionName) { From c3bb671a0829ef5cd0ab3758ee2873fd6a5e298c Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Tue, 1 Jul 2025 08:09:49 -0400 Subject: [PATCH 05/10] Update hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java Co-authored-by: Christian Beikov --- .../java/org/hibernate/query/sql/spi/ParameterOccurrence.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java b/hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java index 886dd3003b89..4dce13b77071 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java @@ -10,4 +10,8 @@ * @author Christian Beikov */ public record ParameterOccurrence(QueryParameterImplementor parameter, int sourcePosition, int length) { + @Deprecated(forRemoval = true) + public ParameterOccurrence(QueryParameterImplementor parameter, int sourcePosition) { + this( parameter, sourcePosition, 1 ); + } } From 223528a85f75a57f8d8141ee299e414de3849fd2 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Tue, 1 Jul 2025 08:30:39 -0400 Subject: [PATCH 06/10] fix potential NPE when ParameterMakerStrategy parameter is null in OffsetFetchLimitHandler#processSql --- .../dialect/pagination/OffsetFetchLimitHandler.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/OffsetFetchLimitHandler.java b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/OffsetFetchLimitHandler.java index 577de737e7ab..2fc251a90636 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/OffsetFetchLimitHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/OffsetFetchLimitHandler.java @@ -4,6 +4,7 @@ */ package org.hibernate.dialect.pagination; +import org.checkerframework.checker.nullness.qual.Nullable; import org.hibernate.query.spi.Limit; import org.hibernate.query.spi.QueryOptions; import org.hibernate.sql.ast.internal.ParameterMarkerStrategyStandard; @@ -34,7 +35,7 @@ public String processSql(String sql, Limit limit) { } @Override - public String processSql(String sql, int jdbcParameterBindingsCnt, ParameterMarkerStrategy parameterMarkerStrategy, Limit limit, QueryOptions queryOptions) { + public String processSql(String sql, int jdbcParameterBindingsCnt, @Nullable ParameterMarkerStrategy parameterMarkerStrategy, Limit limit, QueryOptions queryOptions) { boolean hasFirstRow = hasFirstRow(limit); boolean hasMaxRows = hasMaxRows(limit); @@ -46,6 +47,9 @@ public String processSql(String sql, int jdbcParameterBindingsCnt, ParameterMark begin(sql, offsetFetch, hasFirstRow, hasMaxRows); + if (parameterMarkerStrategy == null) { + parameterMarkerStrategy = ParameterMarkerStrategyStandard.INSTANCE; + } if ( hasFirstRow ) { offsetFetch.append( " offset " ); if ( supportsVariableLimit() ) { From f26b32cfeca5ae89e2d03dbbe533441d0a7c4f25 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Tue, 1 Jul 2025 10:00:53 -0400 Subject: [PATCH 07/10] Update hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java Co-authored-by: Christian Beikov --- .../java/org/hibernate/dialect/pagination/LimitHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java index 9d5dae20fbc3..0d48a4b4da77 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java @@ -49,7 +49,7 @@ default String processSql(String sql, Limit limit, QueryOptions queryOptions) { } // This is the one called directly by Hibernate ORM - default String processSql(String sql, int jdbcParameterBindingsCnt, @Nullable ParameterMarkerStrategy parameterMarkerStrategy, Limit limit, QueryOptions queryOptions) { + default String processSql(String sql, int jdbcParameterBindingsCnt, ParameterMarkerStrategy parameterMarkerStrategy, Limit limit, QueryOptions queryOptions) { return processSql( sql, limit ); } From 896bd0210af17871f1e9268016370bb3b48d9e96 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Tue, 1 Jul 2025 10:05:45 -0400 Subject: [PATCH 08/10] followup code changes to remove @Nullable for ParameterMarkerStrategy in LimitHandler#processSql --- .../org/hibernate/dialect/pagination/LimitHandler.java | 1 - .../dialect/pagination/OffsetFetchLimitHandler.java | 6 +----- .../org/hibernate/query/sql/spi/ParameterOccurrence.java | 8 ++++---- .../results/jdbc/internal/DeferredResultSetAccess.java | 5 ++++- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java index 0d48a4b4da77..e040eb2ac337 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java @@ -7,7 +7,6 @@ import java.sql.PreparedStatement; import java.sql.SQLException; -import org.checkerframework.checker.nullness.qual.Nullable; import org.hibernate.query.spi.Limit; import org.hibernate.query.spi.QueryOptions; import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/OffsetFetchLimitHandler.java b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/OffsetFetchLimitHandler.java index 2fc251a90636..577de737e7ab 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/OffsetFetchLimitHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/OffsetFetchLimitHandler.java @@ -4,7 +4,6 @@ */ package org.hibernate.dialect.pagination; -import org.checkerframework.checker.nullness.qual.Nullable; import org.hibernate.query.spi.Limit; import org.hibernate.query.spi.QueryOptions; import org.hibernate.sql.ast.internal.ParameterMarkerStrategyStandard; @@ -35,7 +34,7 @@ public String processSql(String sql, Limit limit) { } @Override - public String processSql(String sql, int jdbcParameterBindingsCnt, @Nullable ParameterMarkerStrategy parameterMarkerStrategy, Limit limit, QueryOptions queryOptions) { + public String processSql(String sql, int jdbcParameterBindingsCnt, ParameterMarkerStrategy parameterMarkerStrategy, Limit limit, QueryOptions queryOptions) { boolean hasFirstRow = hasFirstRow(limit); boolean hasMaxRows = hasMaxRows(limit); @@ -47,9 +46,6 @@ public String processSql(String sql, int jdbcParameterBindingsCnt, @Nullable Par begin(sql, offsetFetch, hasFirstRow, hasMaxRows); - if (parameterMarkerStrategy == null) { - parameterMarkerStrategy = ParameterMarkerStrategyStandard.INSTANCE; - } if ( hasFirstRow ) { offsetFetch.append( " offset " ); if ( supportsVariableLimit() ) { diff --git a/hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java b/hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java index 4dce13b77071..e2518588aee5 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sql/spi/ParameterOccurrence.java @@ -10,8 +10,8 @@ * @author Christian Beikov */ public record ParameterOccurrence(QueryParameterImplementor parameter, int sourcePosition, int length) { - @Deprecated(forRemoval = true) - public ParameterOccurrence(QueryParameterImplementor parameter, int sourcePosition) { - this( parameter, sourcePosition, 1 ); - } + @Deprecated(forRemoval = true) + public ParameterOccurrence(QueryParameterImplementor parameter, int sourcePosition) { + this( parameter, sourcePosition, 1 ); + } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java b/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java index 2386bc4b087b..7f1a391449c6 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java @@ -7,6 +7,7 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; +import java.util.Objects; import org.hibernate.LockMode; import org.hibernate.LockOptions; @@ -27,6 +28,7 @@ import org.hibernate.query.spi.QueryOptions; import org.hibernate.resource.jdbc.spi.JdbcSessionContext; import org.hibernate.resource.jdbc.spi.LogicalConnectionImplementor; +import org.hibernate.sql.ast.internal.ParameterMarkerStrategyStandard; import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; import org.hibernate.sql.exec.spi.ExecutionContext; import org.hibernate.sql.exec.spi.JdbcLockStrategy; @@ -99,7 +101,8 @@ public DeferredResultSetAccess( } else { final int jdbcBindingsCnt = jdbcParameterBindings.getBindings().size(); - final ParameterMarkerStrategy parameterMarkerStrategy = dialect.getNativeParameterMarkerStrategy(); + final ParameterMarkerStrategy parameterMarkerStrategy = + Objects.requireNonNullElse(dialect.getNativeParameterMarkerStrategy(), ParameterMarkerStrategyStandard.INSTANCE); sqlWithLimit = limitHandler.processSql( sql, jdbcBindingsCnt, parameterMarkerStrategy, limit, queryOptions ); } From 9121fb25af88afda9e0aa54e4df92902aa29f088 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Tue, 1 Jul 2025 10:50:10 -0400 Subject: [PATCH 09/10] fix broken testing case --- .../sql/results/jdbc/internal/DeferredResultSetAccess.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java b/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java index 7f1a391449c6..12395381f69e 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/jdbc/internal/DeferredResultSetAccess.java @@ -7,7 +7,6 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; -import java.util.Objects; import org.hibernate.LockMode; import org.hibernate.LockOptions; @@ -28,7 +27,6 @@ import org.hibernate.query.spi.QueryOptions; import org.hibernate.resource.jdbc.spi.JdbcSessionContext; import org.hibernate.resource.jdbc.spi.LogicalConnectionImplementor; -import org.hibernate.sql.ast.internal.ParameterMarkerStrategyStandard; import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; import org.hibernate.sql.exec.spi.ExecutionContext; import org.hibernate.sql.exec.spi.JdbcLockStrategy; @@ -102,7 +100,7 @@ public DeferredResultSetAccess( else { final int jdbcBindingsCnt = jdbcParameterBindings.getBindings().size(); final ParameterMarkerStrategy parameterMarkerStrategy = - Objects.requireNonNullElse(dialect.getNativeParameterMarkerStrategy(), ParameterMarkerStrategyStandard.INSTANCE); + executionContext.getSession().getFactory().getServiceRegistry().requireService( ParameterMarkerStrategy.class ); sqlWithLimit = limitHandler.processSql( sql, jdbcBindingsCnt, parameterMarkerStrategy, limit, queryOptions ); } From d0cb598f0e28bad7f092983cd4468d7fbee37a2f Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Tue, 1 Jul 2025 12:08:32 -0400 Subject: [PATCH 10/10] some further minor improvements --- .../dialect/pagination/LimitHandler.java | 5 ++ .../NativeParameterMarkerStrategyTests.java | 48 ++++++++++--------- .../orm/junit/DialectFeatureChecks.java | 5 +- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java index e040eb2ac337..06e0153656a9 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/LimitHandler.java @@ -47,6 +47,11 @@ default String processSql(String sql, Limit limit, QueryOptions queryOptions) { return processSql( sql, limit ); } + /** + * Currently all Dialects with non-standard native parameter renderers invariably rely on {@link OffsetFetchLimitHandler}, + * so the method below is currently only overridden in it. However, if other {@link LimitHandler}s are involved in the future + * (e.g. {@link Limit} related parameters need to be inserted after select), code refactoring is required (HHH-18624). + */ // This is the one called directly by Hibernate ORM default String processSql(String sql, int jdbcParameterBindingsCnt, ParameterMarkerStrategy parameterMarkerStrategy, Limit limit, QueryOptions queryOptions) { return processSql( sql, limit ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/NativeParameterMarkerStrategyTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/NativeParameterMarkerStrategyTests.java index 44d1f76bc6de..3ac69011e9de 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/NativeParameterMarkerStrategyTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/NativeParameterMarkerStrategyTests.java @@ -41,7 +41,7 @@ ) ) @DomainModel( annotatedClasses = NativeParameterMarkerStrategyTests.Book.class ) @SessionFactory( useCollectingStatementInspector = true ) -@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsNativeParameterMarker.class ) +@RequiresDialectFeature( feature = DialectFeatureChecks.SupportsNonStandardNativeParameterRendering.class ) @Jira( "https://hibernate.atlassian.net/browse/HHH-16283" ) class NativeParameterMarkerStrategyTests implements SessionFactoryScopeAware { @@ -52,6 +52,9 @@ private enum ParameterStyle { } public static class DialectParameterMarkerStrategy implements ParameterMarkerStrategy { + + public static final DialectParameterMarkerStrategy INSTANCE = new DialectParameterMarkerStrategy(); + @Override public String createMarker(int position, JdbcType jdbcType) { return DialectContext.getDialect().getNativeParameterMarkerStrategy().createMarker( position, jdbcType ); @@ -76,17 +79,17 @@ void setUp() { @EnumSource(ParameterStyle.class) void testHappyPath(ParameterStyle style) { scope.inTransaction( (session) -> { - final NativeQueryImplementor nativeQuery; + final NativeQueryImplementor nativeQuery; final var parameterValue = "War and Peace"; if ( style == ParameterStyle.NAMED ) { - nativeQuery = session.createNativeQuery( "select * from books b where b.title = :title", Integer.class ) + nativeQuery = session.createNativeQuery( "select * from books b where b.title = :title", Book.class ) .setParameter( "title", parameterValue ); } else { - nativeQuery = session.createNativeQuery( "select * from books b where b.title = " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Integer.class ) + nativeQuery = session.createNativeQuery( "select * from books b where b.title = " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Book.class ) .setParameter( 1, parameterValue ); }; - nativeQuery.uniqueResult(); - assertNativeQueryContainsMarkers( statementInspector, 1 ); + nativeQuery.list(); + assertNativeQueryContainsMarkers( 1 ); } ); } @@ -96,16 +99,16 @@ void testParameterExpansion(ParameterStyle style) { final var parameterValue = List.of( "Moby-Dick", "Don Quixote", "In Search of Lost Time" ); scope.inTransaction( (session) -> { - final NativeQuery nativeQuery; + final NativeQuery nativeQuery; if ( style == ParameterStyle.NAMED ) { - nativeQuery = session.createNativeQuery( "select * from books b where b.title in :titles", Integer.class ) + nativeQuery = session.createNativeQuery( "select * from books b where b.title in :titles", Book.class ) .setParameterList( "titles", parameterValue ); } else { - nativeQuery = session.createNativeQuery( "select * from books b where b.title in " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Integer.class ) + nativeQuery = session.createNativeQuery( "select * from books b where b.title in " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Book.class ) .setParameterList( 1, parameterValue ); }; nativeQuery.list(); - assertNativeQueryContainsMarkers( statementInspector, parameterValue.size() ); + assertNativeQueryContainsMarkers( parameterValue.size() ); } ); } @@ -113,18 +116,18 @@ void testParameterExpansion(ParameterStyle style) { @EnumSource(ParameterStyle.class) void testLimitHandler(ParameterStyle style) { scope.inTransaction( (session) -> { - final NativeQueryImplementor nativeQuery; + final NativeQueryImplementor nativeQuery; final var parameterValue = "Herman Melville"; if ( style == ParameterStyle.NAMED ) { - nativeQuery = session.createNativeQuery( "select * from books b where b.author = :author", Integer.class ) + nativeQuery = session.createNativeQuery( "select * from books b where b.author = :author", Book.class ) .setParameter( "author", parameterValue ); } else { - nativeQuery = session.createNativeQuery( "select * from books b where b.author = " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Integer.class ) + nativeQuery = session.createNativeQuery( "select * from books b where b.author = " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Book.class ) .setParameter( 1, parameterValue ); }; nativeQuery.setFirstResult( 2 ).setMaxResults( 1 ).list(); - assertNativeQueryContainsMarkers( statementInspector, 3 ); + assertNativeQueryContainsMarkers( 3 ); } ); } @@ -134,26 +137,26 @@ void test_parameterExpansionAndLimitHandler(ParameterStyle style) { final var parameterValue = List.of( "Moby-Dick", "Don Quixote", "In Search of Lost Time" ); scope.inTransaction( (session) -> { - final NativeQueryImplementor nativeQuery; + final NativeQueryImplementor nativeQuery; if ( style == ParameterStyle.NAMED ) { - nativeQuery = session.createNativeQuery( "select * from books b where b.title in :titles", Integer.class ) + nativeQuery = session.createNativeQuery( "select * from books b where b.title in :titles", Book.class ) .setParameterList( "titles", parameterValue ); } else { - nativeQuery = session.createNativeQuery( "select * from books b where b.title in " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Integer.class ) + nativeQuery = session.createNativeQuery( "select * from books b where b.title in " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Book.class ) .setParameterList( 1, parameterValue ); }; nativeQuery.setFirstResult( 1 ).setMaxResults( 3 ).list(); - assertNativeQueryContainsMarkers( statementInspector, parameterValue.size() + 2 ); + assertNativeQueryContainsMarkers( parameterValue.size() + 2 ); } ); } - private void assertNativeQueryContainsMarkers(SQLStatementInspector statementInspector, int expectedMarkerNum) { + private void assertNativeQueryContainsMarkers(int expectedMarkerNum) { - final var strategy = new DialectParameterMarkerStrategy(); + final var strategy = DialectParameterMarkerStrategy.INSTANCE; final var expectedMarkers = new String[expectedMarkerNum]; - for (int i = 1; i <= expectedMarkerNum; i++) { + for ( int i = 1; i <= expectedMarkerNum; i++ ) { expectedMarkers[i - 1] = strategy.createMarker( i, IntegerJdbcType.INSTANCE ); } @@ -168,9 +171,8 @@ private void assertNativeQueryContainsMarkers(SQLStatementInspector statementIns @Table(name = "books") static class Book { @Id - String isbn; + int id; String title; String author; } - } diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/DialectFeatureChecks.java b/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/DialectFeatureChecks.java index 97dc4cc55c86..e98def7f1f6c 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/DialectFeatureChecks.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/orm/junit/DialectFeatureChecks.java @@ -81,6 +81,7 @@ import org.hibernate.query.sqm.function.SqmFunctionDescriptor; import org.hibernate.query.sqm.function.SqmFunctionRegistry; import org.hibernate.service.ServiceRegistry; +import org.hibernate.sql.ast.internal.ParameterMarkerStrategyStandard; import org.hibernate.sql.ast.spi.StringBuilderSqlAppender; import org.hibernate.testing.boot.BootstrapContextImpl; import org.hibernate.type.SqlTypes; @@ -1035,10 +1036,10 @@ public boolean apply(Dialect dialect) { } } - public static class SupportsNativeParameterMarker implements DialectFeatureCheck { + public static class SupportsNonStandardNativeParameterRendering implements DialectFeatureCheck { @Override public boolean apply(Dialect dialect) { - return dialect.getNativeParameterMarkerStrategy() != null; + return !ParameterMarkerStrategyStandard.isStandardRenderer( dialect.getNativeParameterMarkerStrategy() ); } }