diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/AltibaseLimitHandler.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/AltibaseLimitHandler.java index a95be7931045..85b8ab238f7d 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/AltibaseLimitHandler.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/AltibaseLimitHandler.java @@ -5,6 +5,7 @@ package org.hibernate.community.dialect.pagination; import org.hibernate.dialect.pagination.LimitLimitHandler; +import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; /** * Limit handler for {@link org.hibernate.community.dialect.AltibaseDialect}. @@ -19,8 +20,24 @@ protected String limitClause(boolean hasFirstRow) { return hasFirstRow ? " limit 1+?,?" : " limit ?"; } + @Override + protected String limitClause(boolean hasFirstRow, int jdbcParameterCount, ParameterMarkerStrategy parameterMarkerStrategy) { + final String firstParameter = parameterMarkerStrategy.createMarker( jdbcParameterCount + 1, null ); + if ( hasFirstRow ) { + return " limit 1+" + firstParameter + "," + parameterMarkerStrategy.createMarker( jdbcParameterCount + 2, null ); + } + else { + return " limit " + firstParameter; + } + } + @Override protected String offsetOnlyClause() { return " limit 1+?," + Integer.MAX_VALUE; } + + @Override + protected String offsetOnlyClause(int jdbcParameterCount, ParameterMarkerStrategy parameterMarkerStrategy) { + return " limit 1+" + parameterMarkerStrategy.createMarker( jdbcParameterCount + 1, null ) + "," + Integer.MAX_VALUE; + } } diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/FirstLimitHandler.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/FirstLimitHandler.java index 3dd03b74ca14..fd181e374cae 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/FirstLimitHandler.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/FirstLimitHandler.java @@ -6,6 +6,8 @@ import org.hibernate.dialect.pagination.AbstractNoOffsetLimitHandler; import org.hibernate.dialect.pagination.LimitHandler; +import org.hibernate.query.spi.Limit; +import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; /** * A {@link LimitHandler} for older versions of Informix, Ingres, @@ -27,6 +29,11 @@ protected String limitClause() { return " first ?"; } + @Override + protected String limitClause(int jdbcParameterCount, ParameterMarkerStrategy parameterMarkerStrategy) { + return " first " + parameterMarkerStrategy.createMarker( 1, null ) + " rows only"; + } + @Override protected String insert(String first, String sql) { return insertAfterSelect( first, sql ); @@ -36,4 +43,14 @@ protected String insert(String first, String sql) { public boolean bindLimitParametersFirst() { return true; } + + @Override + public boolean processSqlMutatesState() { + return false; + } + + @Override + public int getParameterPositionStart(Limit limit) { + return hasMaxRows( limit ) && supportsVariableLimit() ? 2 : 1; + } } diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/FirstSkipLimitHandler.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/FirstSkipLimitHandler.java index d8d4adf69d5b..db969c3b07b5 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/FirstSkipLimitHandler.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/FirstSkipLimitHandler.java @@ -4,9 +4,13 @@ */ package org.hibernate.community.dialect.pagination; +import org.checkerframework.checker.nullness.qual.Nullable; import org.hibernate.dialect.pagination.AbstractLimitHandler; import org.hibernate.dialect.pagination.LimitHandler; 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; /** * A {@link LimitHandler} for Firebird 2.5 and older which supports the syntax @@ -18,6 +22,15 @@ public class FirstSkipLimitHandler extends AbstractLimitHandler { @Override public String processSql(String sql, Limit limit) { + return processSql( sql, -1, null, limit ); + } + + @Override + public String processSql(String sql, int jdbcParameterCount, @Nullable ParameterMarkerStrategy parameterMarkerStrategy, QueryOptions queryOptions) { + return processSql( sql, jdbcParameterCount, parameterMarkerStrategy, queryOptions.getLimit() ); + } + + private String processSql(String sql, int jdbcParameterCount, @Nullable ParameterMarkerStrategy parameterMarkerStrategy, Limit limit) { boolean hasFirstRow = hasFirstRow( limit ); boolean hasMaxRows = hasMaxRows( limit ); @@ -27,13 +40,26 @@ public String processSql(String sql, Limit limit) { StringBuilder skipFirst = new StringBuilder(); - if ( hasMaxRows ) { - skipFirst.append( " first ?" ); + if ( ParameterMarkerStrategyStandard.isStandardRenderer( parameterMarkerStrategy ) ) { + if ( hasMaxRows ) { + skipFirst.append( " first ?" ); + } + if ( hasFirstRow ) { + skipFirst.append( " skip ?" ); + } } - if ( hasFirstRow ) { - skipFirst.append( " skip ?" ); + else { + String marker = parameterMarkerStrategy.createMarker( jdbcParameterCount + 1, null ); + if ( hasMaxRows ) { + skipFirst.append( " first " ); + skipFirst.append( marker ); + marker = parameterMarkerStrategy.createMarker( jdbcParameterCount + 2, null ); + } + if ( hasFirstRow ) { + skipFirst.append( " skip " ); + skipFirst.append( marker ); + } } - return insertAfterSelect( skipFirst.toString(), sql ); } @@ -57,4 +83,16 @@ public final boolean bindLimitParametersFirst() { return true; } + @Override + public boolean processSqlMutatesState() { + return false; + } + + @Override + public int getParameterPositionStart(Limit limit) { + return hasMaxRows( limit ) + ? hasFirstRow( limit ) ? 3 : 2 + : hasFirstRow( limit ) ? 2 : 1; + } + } diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/LegacyHSQLLimitHandler.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/LegacyHSQLLimitHandler.java index 63ec85a897e5..04a62dcf2511 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/LegacyHSQLLimitHandler.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/LegacyHSQLLimitHandler.java @@ -6,6 +6,8 @@ import org.hibernate.dialect.pagination.AbstractSimpleLimitHandler; import org.hibernate.dialect.pagination.LimitHandler; +import org.hibernate.query.spi.Limit; +import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; /** * A {@link LimitHandler} for HSQL prior to 2.0. @@ -19,6 +21,17 @@ protected String limitClause(boolean hasFirstRow) { return hasFirstRow ? " limit ? ?" : " top ?"; } + @Override + protected String limitClause(boolean hasFirstRow, int jdbcParameterCount, ParameterMarkerStrategy parameterMarkerStrategy) { + final String firstParameter = parameterMarkerStrategy.createMarker( 1, null ); + if ( hasFirstRow ) { + return " limit 1+" + firstParameter + " " + parameterMarkerStrategy.createMarker( 2, null ); + } + else { + return " top " + firstParameter; + } + } + @Override protected String insert(String limitOrTop, String sql) { return insertAfterSelect( limitOrTop, sql ); @@ -28,4 +41,16 @@ protected String insert(String limitOrTop, String sql) { public final boolean bindLimitParametersFirst() { return true; } + + @Override + public boolean processSqlMutatesState() { + return false; + } + + @Override + public int getParameterPositionStart(Limit limit) { + return hasMaxRows( limit ) + ? hasFirstRow( limit ) ? 3 : 2 + : hasFirstRow( limit ) ? 2 : 1; + } } diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/LegacyOracleLimitHandler.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/LegacyOracleLimitHandler.java index 2000e8401f45..00682cef4407 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/LegacyOracleLimitHandler.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/LegacyOracleLimitHandler.java @@ -4,10 +4,14 @@ */ package org.hibernate.community.dialect.pagination; +import org.checkerframework.checker.nullness.qual.Nullable; import org.hibernate.dialect.DatabaseVersion; import org.hibernate.dialect.pagination.AbstractLimitHandler; import org.hibernate.dialect.pagination.LimitHandler; 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 java.util.regex.Matcher; @@ -23,6 +27,15 @@ public LegacyOracleLimitHandler(DatabaseVersion version) { @Override public String processSql(String sql, Limit limit) { + return processSql( sql, -1, limit, null ); + } + + @Override + public String processSql(String sql, int jdbcParameterCount, @Nullable ParameterMarkerStrategy parameterMarkerStrategy, QueryOptions queryOptions) { + return processSql( sql, jdbcParameterCount, queryOptions.getLimit(), parameterMarkerStrategy ); + } + + private String processSql(String sql, int jdbcParameterCount, @Nullable Limit limit, @Nullable ParameterMarkerStrategy parameterMarkerStrategy) { final boolean hasOffset = hasFirstRow( limit ); sql = sql.trim(); @@ -36,17 +49,42 @@ public String processSql(String sql, Limit limit) { } final StringBuilder pagingSelect = new StringBuilder( sql.length() + 100 ); - if ( hasOffset ) { - pagingSelect.append( "select * from (select row_.*,rownum rownum_ from (" ).append( sql ); - if ( version.isBefore( 9 ) ) { - pagingSelect.append( ") row_) where rownum_<=? and rownum_>?" ); + if ( ParameterMarkerStrategyStandard.isStandardRenderer( parameterMarkerStrategy ) ) { + if ( hasOffset ) { + pagingSelect.append( "select * from (select row_.*,rownum rownum_ from (" ).append( sql ); + if ( version.isBefore( 9 ) ) { + pagingSelect.append( ") row_) where rownum_<=? and rownum_>?" ); + } + else { + pagingSelect.append( ") row_ where rownum<=?) where rownum_>?" ); + } } else { - pagingSelect.append( ") row_ where rownum<=?) where rownum_>?" ); + pagingSelect.append( "select * from (" ).append( sql ).append( ") where rownum<=?" ); } } else { - pagingSelect.append( "select * from (" ).append( sql ).append( ") where rownum<=?" ); + final String firstParameter = parameterMarkerStrategy.createMarker( jdbcParameterCount + 1, null ); + if ( hasOffset ) { + final String secondParameter = parameterMarkerStrategy.createMarker( jdbcParameterCount + 2, null ); + pagingSelect.append( "select * from (select row_.*,rownum rownum_ from (" ).append( sql ); + if ( version.isBefore( 9 ) ) { + pagingSelect.append( ") row_) where rownum_<=" ); + pagingSelect.append( firstParameter ); + pagingSelect.append( " and rownum_>" ); + pagingSelect.append( secondParameter ); + } + else { + pagingSelect.append( ") row_ where rownum<=" ); + pagingSelect.append( firstParameter ); + pagingSelect.append( ") where rownum_>" ); + pagingSelect.append( secondParameter ); + } + } + else { + pagingSelect.append( "select * from (" ).append( sql ).append( ") where rownum<=" ); + pagingSelect.append( firstParameter ); + } } if ( forUpdateClause != null ) { @@ -80,4 +118,9 @@ public boolean bindLimitParametersInReverseOrder() { public boolean useMaxForLimit() { return true; } + + @Override + public boolean processSqlMutatesState() { + return false; + } } diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/RowsLimitHandler.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/RowsLimitHandler.java index e666d9184094..ce31a7e4a606 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/RowsLimitHandler.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/RowsLimitHandler.java @@ -8,6 +8,7 @@ import org.hibernate.dialect.pagination.AbstractSimpleLimitHandler; import org.hibernate.dialect.pagination.LimitHandler; +import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; import static java.util.regex.Pattern.CASE_INSENSITIVE; import static java.util.regex.Pattern.compile; @@ -29,11 +30,27 @@ protected String limitClause(boolean hasFirstRow) { return hasFirstRow ? " rows ? to ?" : " rows ?"; } + @Override + protected String limitClause(boolean hasFirstRow, int jdbcParameterCount, ParameterMarkerStrategy parameterMarkerStrategy) { + final String firstParameter = parameterMarkerStrategy.createMarker( jdbcParameterCount + 1, null ); + if ( hasFirstRow ) { + return " rows " + firstParameter + " to " + parameterMarkerStrategy.createMarker( jdbcParameterCount + 2, null ); + } + else { + return " rows " + firstParameter; + } + } + @Override protected String offsetOnlyClause() { return " rows ? to " + Integer.MAX_VALUE; } + @Override + protected String offsetOnlyClause(int jdbcParameterCount, ParameterMarkerStrategy parameterMarkerStrategy) { + return " rows " + parameterMarkerStrategy.createMarker( jdbcParameterCount + 1, null ) + " to " + Integer.MAX_VALUE; + } + @Override public final boolean useMaxForLimit() { return true; @@ -56,4 +73,9 @@ protected Pattern getForUpdatePattern() { public boolean supportsOffset() { return true; } + + @Override + public boolean processSqlMutatesState() { + return false; + } } diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/SQLServer2005LimitHandler.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/SQLServer2005LimitHandler.java index e48e94e60204..73c0fcdea1dd 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/SQLServer2005LimitHandler.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/pagination/SQLServer2005LimitHandler.java @@ -11,10 +11,14 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.checkerframework.checker.nullness.qual.Nullable; import org.hibernate.dialect.pagination.AbstractLimitHandler; import org.hibernate.dialect.pagination.LimitHandler; import org.hibernate.internal.util.StringHelper; 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 static java.util.regex.Pattern.CASE_INSENSITIVE; import static java.util.regex.Pattern.compile; @@ -80,6 +84,15 @@ public int convertToFirstRowValue(int zeroBasedFirstResult) { */ @Override public String processSql(String sql, Limit limit) { + return processSql( sql, -1, null, limit ); + } + + @Override + public String processSql(String sql, int jdbcParameterCount, @Nullable ParameterMarkerStrategy parameterMarkerStrategy, QueryOptions queryOptions) { + return processSql( sql, jdbcParameterCount, parameterMarkerStrategy, queryOptions.getLimit() ); + } + + private String processSql(String sql, int jdbcParameterCount, @Nullable ParameterMarkerStrategy parameterMarkerStrategy, @Nullable Limit limit) { sql = sql.trim(); if ( sql.endsWith(";") ) { sql = sql.substring( 0, sql.length()-1 ); @@ -96,7 +109,13 @@ public String processSql(String sql, Limit limit) { final StringBuilder result = new StringBuilder( sql ); if ( !hasFirstRow || hasOrderBy ) { - result.insert( afterSelectOffset, " top(?)" ); + if ( ParameterMarkerStrategyStandard.isStandardRenderer( parameterMarkerStrategy ) ) { + result.insert( afterSelectOffset, " top(?)" ); + } + else { + final String parameterMarker = parameterMarkerStrategy.createMarker( 1, null ); + result.insert( afterSelectOffset, " top(" + parameterMarker + ")" ); + } topAdded = true; } @@ -109,7 +128,16 @@ public String processSql(String sql, Limit limit) { result.insert( selectOffset, ( hasCommonTables ? "," : "with" ) + " query_ as (select row_.*,row_number() over (order by current_timestamp) as rownumber_ from (" ) .append( ") row_) select " ).append( aliases ) - .append( " from query_ where rownumber_>=? and rownumber_=" ); + if ( ParameterMarkerStrategyStandard.isStandardRenderer( parameterMarkerStrategy ) ) { + result.append( "? and rownumber_ Assertions.assertEquals( 69, s.createSelectionQuery( "select count from Constrained", int.class).getSingleResult())); } + private String withLimit(String sql, Limit limit) { + return new DerbyDialect().getLimitHandler().processSql( sql, -1, null, new LimitQueryOptions( limit ) ); + } + private Limit toRowSelection(int firstRow, int maxRows) { Limit selection = new Limit(); selection.setFirstRow( firstRow ); diff --git a/hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/DerbyLegacyDialectTestCase.java b/hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/DerbyLegacyDialectTestCase.java index 6eb127580196..a224bfb40bc9 100644 --- a/hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/DerbyLegacyDialectTestCase.java +++ b/hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/DerbyLegacyDialectTestCase.java @@ -5,6 +5,7 @@ package org.hibernate.community.dialect; import org.hibernate.dialect.DatabaseVersion; +import org.hibernate.orm.test.dialect.LimitQueryOptions; import org.hibernate.query.spi.Limit; import static org.junit.Assert.assertEquals; @@ -27,7 +28,7 @@ public void testInsertLimitClause() { final String input = "select * from tablename t where t.cat = 5"; final String expected = "select * from tablename t where t.cat = 5 fetch first " + limit + " rows only"; - final String actual = new DerbyLegacyDialect( DatabaseVersion.make( 10, 5 ) ).getLimitHandler().processSql( input, toRowSelection( 0, limit ) ); + final String actual = withLimit( input, toRowSelection( 0, limit ) ); assertEquals( expected, actual ); } @@ -39,7 +40,7 @@ public void testInsertLimitWithOffsetClause() { final String input = "select * from tablename t where t.cat = 5"; final String expected = "select * from tablename t where t.cat = 5 offset " + offset + " rows fetch next " + limit + " rows only"; - final String actual = new DerbyLegacyDialect( DatabaseVersion.make( 10, 5 ) ).getLimitHandler().processSql( input, toRowSelection( offset, limit ) ); + final String actual = withLimit( input, toRowSelection( offset, limit ) ); assertEquals( expected, actual ); } @@ -52,7 +53,7 @@ public void testInsertLimitWithForUpdateClause() { final String expected = "select c11 as col1, c12 as col2, c13 as col13 from t1 offset " + offset + " rows fetch next " + limit + " rows only for update of c11, c13"; - final String actual = new DerbyLegacyDialect( DatabaseVersion.make( 10, 5 ) ).getLimitHandler().processSql( input, toRowSelection( offset, limit ) ); + final String actual = withLimit( input, toRowSelection( offset, limit ) ); assertEquals( expected, actual ); } @@ -65,7 +66,7 @@ public void testInsertLimitWithWithClause() { final String expected = "select c11 as col1, c12 as col2, c13 as col13 from t1 where flight_id between 'AA1111' and 'AA1112' offset " + offset + " rows fetch next " + limit + " rows only with rr"; - final String actual = new DerbyLegacyDialect( DatabaseVersion.make( 10, 5 ) ).getLimitHandler().processSql( input, toRowSelection( offset, limit ) ); + final String actual = withLimit( input, toRowSelection( offset, limit ) ); assertEquals( expected, actual ); } @@ -78,10 +79,14 @@ public void testInsertLimitWithForUpdateAndWithClauses() { final String expected = "select c11 as col1, c12 as col2, c13 as col13 from t1 where flight_id between 'AA1111' and 'AA1112' offset " + offset + " rows fetch next " + limit + " rows only for update of c11,c13 with rr"; - final String actual = new DerbyLegacyDialect( DatabaseVersion.make( 10, 5 ) ).getLimitHandler().processSql( input, toRowSelection( offset, limit ) ); + final String actual = withLimit( input, toRowSelection( offset, limit ) ); assertEquals( expected, actual ); } + private String withLimit(String sql, Limit limit) { + return new DerbyLegacyDialect( DatabaseVersion.make( 10, 5 ) ).getLimitHandler().processSql( sql, -1, null, new LimitQueryOptions( limit ) ); + } + private Limit toRowSelection(int firstRow, int maxRows) { Limit selection = new Limit(); selection.setFirstRow( firstRow ); diff --git a/hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/FirebirdDialectTest.java b/hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/FirebirdDialectTest.java index f8439b52d2da..36f9e19d93b0 100644 --- a/hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/FirebirdDialectTest.java +++ b/hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/FirebirdDialectTest.java @@ -5,6 +5,7 @@ package org.hibernate.community.dialect; import org.hibernate.dialect.DatabaseVersion; +import org.hibernate.orm.test.dialect.LimitQueryOptions; import org.hibernate.query.spi.Limit; import org.hibernate.testing.orm.junit.JiraKey; @@ -29,7 +30,7 @@ class FirebirdDialectTest { void insertOffsetLimitClause(int major, int minor, int offset, int limit, String expectedSql) { String input = "select * from tablename t where t.cat = 5"; FirebirdDialect dialect = new FirebirdDialect( DatabaseVersion.make( major, minor ) ); - String actual = dialect.getLimitHandler().processSql( input, new Limit( offset, limit ) ); + String actual = dialect.getLimitHandler().processSql( input, -1, null, new LimitQueryOptions( new Limit( offset, limit ) ) ); assertEquals( expectedSql, actual ); } } diff --git a/hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/SQLServer2005DialectTestCase.java b/hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/SQLServer2005DialectTestCase.java index 329cbcacfba5..6144b87416ce 100644 --- a/hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/SQLServer2005DialectTestCase.java +++ b/hibernate-community-dialects/src/test/java/org/hibernate/community/dialect/SQLServer2005DialectTestCase.java @@ -9,6 +9,7 @@ import org.hibernate.LockMode; import org.hibernate.LockOptions; import org.hibernate.dialect.DatabaseVersion; +import org.hibernate.orm.test.dialect.LimitQueryOptions; import org.hibernate.query.spi.Limit; import org.hibernate.testing.orm.junit.JiraKey; @@ -48,7 +49,7 @@ public void testGetLimitString() { "with query_ as (select row_.*,row_number() over (order by current_timestamp) as rownumber_ from (" + "select distinct top(?) f1 as f53245 from table849752 order by f234, f67 desc) row_)" + " select f53245 from query_ where rownumber_>=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_0 then 'ADDED' else 'UNMODIFIED' end from table2 t2 WHERE (t2.c1 in (?))) as col_1_0 from table1 t1 WHERE 1=1 ORDER BY t1.c1 ASC) row_) " + "select col_0_0,col_1_0 from query_ where rownumber_>=? and rownumber_=? and rownumber_0 then 'ADDED' else 'UNMODIFIED' end from table2 t2 WHERE (t2.c1 in (?))) as col_1_0 from table1 t1 WHERE 1=1 ORDER BY t1.c1 ASC", - dialect.getLimitHandler().processSql( query, toRowSelection( 0, 5 ) ) + withLimit( query, toRowSelection( 0, 5 ) ) ); } @@ -418,14 +419,14 @@ public void testGetLimitStringUsingCTEQueryNoOffset() { final String query1 = "WITH a (c1, c2) AS (SELECT c1, c2 FROM t) SELECT c1, c2 FROM a"; assertEquals( "WITH a (c1, c2) AS (SELECT c1, c2 FROM t) SELECT top(?) c1, c2 FROM a", - dialect.getLimitHandler().processSql( query1, selection ) + withLimit( query1, selection ) ); // test top-based CTE with single CTE query_ definition and various tab, newline spaces final String query2 = " \n\tWITH a (c1\n\t,c2)\t\nAS (SELECT\n\tc1,c2 FROM t)\t\nSELECT c1, c2 FROM a"; assertEquals( "WITH a (c1\n\t,c2)\t\nAS (SELECT\n\tc1,c2 FROM t)\t\nSELECT top(?) c1, c2 FROM a", - dialect.getLimitHandler().processSql( query2, selection ) + withLimit( query2, selection ) ); // test top-based CTE with multiple CTE query_ definitions with no odd formatting @@ -434,7 +435,7 @@ public void testGetLimitStringUsingCTEQueryNoOffset() { assertEquals( "WITH a (c1, c2) AS (SELECT c1, c2 FROM t1), b (b1, b2) AS (SELECT b1, b2 FROM t2) " + "SELECT top(?) c1, c2, b1, b2 FROM t1, t2 WHERE t1.c1 = t2.b1", - dialect.getLimitHandler().processSql( query3, selection ) + withLimit( query3, selection ) ); // test top-based CTE with multiple CTE query_ definitions and various tab, newline spaces @@ -443,7 +444,7 @@ public void testGetLimitStringUsingCTEQueryNoOffset() { assertEquals( "WITH a (c1, c2) AS\n\r (SELECT c1, c2 FROM t1)\n\r, b (b1, b2)\tAS\t(SELECT b1, b2 FROM t2)" + " SELECT top(?) c1, c2, b1, b2 FROM t1, t2 WHERE t1.c1 = t2.b1", - dialect.getLimitHandler().processSql( query4, selection ) + withLimit( query4, selection ) ); } @@ -459,7 +460,7 @@ public void testGetLimitStringUsingCTEQueryWithOffset() { "(order by current_timestamp) as rownumber_ from (SELECT c1 as col0_, c2 as col1_ " + "FROM a) row_) select col0_,col1_ from query_ where rownumber_>=? " + "and rownumber_=" + "? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_=? and rownumber_0 then 'ADDED' else 'UNMODIFIED' end from table2 t2 WHERE (t2.c1 in (?))) as col_1_0 from table1 t1 WHERE 1=1 ORDER BY t1.c1 ASC) row_) " + "select col_0_0,col_1_0 from query_ where rownumber_>=? and rownumber_=? and rownumber_0 then 'ADDED' else 'UNMODIFIED' end from table2 t2 WHERE (t2.c1 in (?))) as col_1_0 from table1 t1 WHERE 1=1 ORDER BY t1.c1 ASC", - dialect.getLimitHandler().processSql( query, toRowSelection( 0, 5 ) ) + withLimit( query, toRowSelection( 0, 5 ) ) ); } @@ -419,14 +420,14 @@ public void testGetLimitStringUsingCTEQueryNoOffset() { final String query1 = "WITH a (c1, c2) AS (SELECT c1, c2 FROM t) SELECT c1, c2 FROM a"; assertEquals( "WITH a (c1, c2) AS (SELECT c1, c2 FROM t) SELECT top(?) c1, c2 FROM a", - dialect.getLimitHandler().processSql( query1, selection ) + withLimit( query1, selection ) ); // test top-based CTE with single CTE query_ definition and various tab, newline spaces final String query2 = " \n\tWITH a (c1\n\t,c2)\t\nAS (SELECT\n\tc1,c2 FROM t)\t\nSELECT c1, c2 FROM a"; assertEquals( "WITH a (c1\n\t,c2)\t\nAS (SELECT\n\tc1,c2 FROM t)\t\nSELECT top(?) c1, c2 FROM a", - dialect.getLimitHandler().processSql( query2, selection ) + withLimit( query2, selection ) ); // test top-based CTE with multiple CTE query_ definitions with no odd formatting @@ -435,7 +436,7 @@ public void testGetLimitStringUsingCTEQueryNoOffset() { assertEquals( "WITH a (c1, c2) AS (SELECT c1, c2 FROM t1), b (b1, b2) AS (SELECT b1, b2 FROM t2) " + "SELECT top(?) c1, c2, b1, b2 FROM t1, t2 WHERE t1.c1 = t2.b1", - dialect.getLimitHandler().processSql( query3, selection ) + withLimit( query3, selection ) ); // test top-based CTE with multiple CTE query_ definitions and various tab, newline spaces @@ -444,7 +445,7 @@ public void testGetLimitStringUsingCTEQueryNoOffset() { assertEquals( "WITH a (c1, c2) AS\n\r (SELECT c1, c2 FROM t1)\n\r, b (b1, b2)\tAS\t(SELECT b1, b2 FROM t2)" + " SELECT top(?) c1, c2, b1, b2 FROM t1, t2 WHERE t1.c1 = t2.b1", - dialect.getLimitHandler().processSql( query4, selection ) + withLimit( query4, selection ) ); } @@ -460,7 +461,7 @@ public void testGetLimitStringUsingCTEQueryWithOffset() { "(order by current_timestamp) as rownumber_ from (SELECT c1 as col0_, c2 as col1_ " + "FROM a) row_) select col0_,col1_ from query_ where rownumber_>=? " + "and rownumber_=" + "? and rownumber_=? and rownumber_=? and rownumber_?" ); - } - else if ( hasFirstRow ) { - pagingSelect = new StringBuilder( sql.length() + forUpdateClauseLength + 98 ); - pagingSelect.append( "select * from (" ); - pagingSelect.append( sql ); - pagingSelect.append( ") row_ where rownum>?" ); + if ( ParameterMarkerStrategyStandard.isStandardRenderer( parameterMarkerStrategy ) ) { + if ( hasFirstRow && hasMaxRows ) { + pagingSelect = new StringBuilder( sql.length() + forUpdateClauseLength + 98 ); + pagingSelect.append( "select * from (select row_.*,rownum rownum_ from (" ); + pagingSelect.append( sql ); + pagingSelect.append( ") row_ where rownum<=?) where rownum_>?" ); + } + else if ( hasFirstRow ) { + pagingSelect = new StringBuilder( sql.length() + forUpdateClauseLength + 98 ); + pagingSelect.append( "select * from (" ); + pagingSelect.append( sql ); + pagingSelect.append( ") row_ where rownum>?" ); + } + else { + pagingSelect = new StringBuilder( sql.length() + forUpdateClauseLength + 37 ); + pagingSelect.append( "select * from (" ); + pagingSelect.append( sql ); + pagingSelect.append( ") where rownum<=?" ); + } } else { - pagingSelect = new StringBuilder( sql.length() + forUpdateClauseLength + 37 ); - pagingSelect.append( "select * from (" ); - pagingSelect.append( sql ); - pagingSelect.append( ") where rownum<=?" ); + final String firstParameter = parameterMarkerStrategy.createMarker( jdbcParameterCount + 1, null ); + if ( hasFirstRow && hasMaxRows ) { + final String secondParameter = parameterMarkerStrategy.createMarker( jdbcParameterCount + 2, null ); + pagingSelect = new StringBuilder( sql.length() + forUpdateClauseLength + 98 ); + pagingSelect.append( "select * from (select row_.*,rownum rownum_ from (" ); + pagingSelect.append( sql ); + pagingSelect.append( ") row_ where rownum<=" ); + pagingSelect.append( firstParameter ); + pagingSelect.append( ") where rownum_>" ); + pagingSelect.append( secondParameter ); + } + else if ( hasFirstRow ) { + pagingSelect = new StringBuilder( sql.length() + forUpdateClauseLength + 98 ); + pagingSelect.append( "select * from (" ); + pagingSelect.append( sql ); + pagingSelect.append( ") row_ where rownum>" ); + pagingSelect.append( firstParameter ); + } + else { + pagingSelect = new StringBuilder( sql.length() + forUpdateClauseLength + 37 ); + pagingSelect.append( "select * from (" ); + pagingSelect.append( sql ); + pagingSelect.append( ") where rownum<=" ); + pagingSelect.append( firstParameter ); + } } if ( isForUpdate ) { diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/TopLimitHandler.java b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/TopLimitHandler.java index 2a3974e321a3..21c575ff5f56 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/pagination/TopLimitHandler.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/pagination/TopLimitHandler.java @@ -4,6 +4,9 @@ */ package org.hibernate.dialect.pagination; +import org.hibernate.query.spi.Limit; +import org.hibernate.sql.ast.spi.ParameterMarkerStrategy; + /** * A {@link LimitHandler} for Transact SQL and similar * databases which support the syntax {@code SELECT TOP n}. @@ -25,6 +28,11 @@ protected String limitClause() { return " top ? "; } + @Override + protected String limitClause(int jdbcParameterCount, ParameterMarkerStrategy parameterMarkerStrategy) { + return " top " + parameterMarkerStrategy.createMarker( 1, null ) + " rows only"; + } + @Override protected String insert(String limitClause, String sql) { return insertAfterDistinct( limitClause, sql ); @@ -35,4 +43,13 @@ public boolean bindLimitParametersFirst() { return true; } + @Override + public boolean processSqlMutatesState() { + return false; + } + + @Override + public int getParameterPositionStart(Limit limit) { + return hasMaxRows( limit ) && supportsVariableLimit() ? 2 : 1; + } } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/FilterJdbcParameter.java b/hibernate-core/src/main/java/org/hibernate/internal/FilterJdbcParameter.java index 7717f7a1b0d1..9f265eedf95a 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/FilterJdbcParameter.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/FilterJdbcParameter.java @@ -7,6 +7,7 @@ import java.sql.PreparedStatement; import java.sql.SQLException; +import org.checkerframework.checker.nullness.qual.Nullable; import org.hibernate.metamodel.mapping.JdbcMapping; import org.hibernate.metamodel.mapping.JdbcMappingContainer; import org.hibernate.sql.ast.SqlAstWalker; @@ -48,6 +49,11 @@ public JdbcMappingContainer getExpressionType() { return jdbcMapping; } + @Override + public @Nullable Integer getParameterId() { + return null; + } + @Override public void accept(SqlAstWalker sqlTreeWalker) { throw new IllegalStateException( ); diff --git a/hibernate-core/src/main/java/org/hibernate/query/internal/QueryInterpretationCacheDisabledImpl.java b/hibernate-core/src/main/java/org/hibernate/query/internal/QueryInterpretationCacheDisabledImpl.java index 5de05004e0ab..961d56ba3485 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/internal/QueryInterpretationCacheDisabledImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/internal/QueryInterpretationCacheDisabledImpl.java @@ -60,6 +60,15 @@ public SelectQueryPlan resolveSelectQueryPlan(Key key, Supplier SelectQueryPlan resolveSelectQueryPlan(K key, Function> creator) { + final StatisticsImplementor statistics = getStatistics(); + if ( statistics.isStatisticsEnabled() ) { + statistics.queryPlanCacheMiss( key.getQueryString() ); + } + return creator.apply( key ); + } + @Override public NonSelectQueryPlan getNonSelectQueryPlan(Key key) { return null; diff --git a/hibernate-core/src/main/java/org/hibernate/query/internal/QueryInterpretationCacheStandardImpl.java b/hibernate-core/src/main/java/org/hibernate/query/internal/QueryInterpretationCacheStandardImpl.java index ae8c3d182be6..2ea8121b460a 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/internal/QueryInterpretationCacheStandardImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/internal/QueryInterpretationCacheStandardImpl.java @@ -76,6 +76,13 @@ private StatisticsImplementor getStatistics() { public SelectQueryPlan resolveSelectQueryPlan( Key key, Supplier> creator) { + return resolveSelectQueryPlan( key, k -> creator.get() ); + } + + @Override + public SelectQueryPlan resolveSelectQueryPlan( + K key, + Function> creator) { log.tracef( "Resolving cached query plan for [%s]", key ); final StatisticsImplementor statistics = getStatistics(); final boolean stats = statistics.isStatisticsEnabled(); @@ -89,7 +96,7 @@ public SelectQueryPlan resolveSelectQueryPlan( return cached; } - final SelectQueryPlan plan = creator.get(); + final SelectQueryPlan plan = creator.apply( key ); queryPlanCache.put( key.prepareForStore(), plan ); if ( stats ) { statistics.queryPlanCacheMiss( key.getQueryString() ); diff --git a/hibernate-core/src/main/java/org/hibernate/query/results/ResultSetMapping.java b/hibernate-core/src/main/java/org/hibernate/query/results/ResultSetMapping.java index 2ef874bfc632..3564169f3288 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/results/ResultSetMapping.java +++ b/hibernate-core/src/main/java/org/hibernate/query/results/ResultSetMapping.java @@ -98,4 +98,7 @@ static ResultSetMapping resolveResultSetMapping(String name, boolean isDynamic, return sessionFactory.getJdbcValuesMappingProducerProvider() .buildResultSetMapping( name, isDynamic, sessionFactory ); } + + @Override + ResultSetMapping cacheKeyInstance(); } diff --git a/hibernate-core/src/main/java/org/hibernate/query/results/internal/ResultSetMappingImpl.java b/hibernate-core/src/main/java/org/hibernate/query/results/internal/ResultSetMappingImpl.java index 98aff703491b..4b92de3d23b7 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/results/internal/ResultSetMappingImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/results/internal/ResultSetMappingImpl.java @@ -20,7 +20,6 @@ import org.hibernate.sql.results.graph.basic.BasicResult; import org.hibernate.sql.results.graph.entity.EntityResult; import org.hibernate.sql.results.jdbc.spi.JdbcValuesMapping; -import org.hibernate.sql.results.jdbc.spi.JdbcValuesMappingProducer; import org.hibernate.sql.results.jdbc.spi.JdbcValuesMetadata; import org.hibernate.type.BasicType; @@ -329,7 +328,7 @@ public NamedResultSetMappingMemento toMemento(String name) { } @Override - public JdbcValuesMappingProducer cacheKeyInstance() { + public ResultSetMapping cacheKeyInstance() { return new ResultSetMappingImpl( this ); } diff --git a/hibernate-core/src/main/java/org/hibernate/query/spi/QueryInterpretationCache.java b/hibernate-core/src/main/java/org/hibernate/query/spi/QueryInterpretationCache.java index 178bda50bb8f..ca3482e37f8f 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/spi/QueryInterpretationCache.java +++ b/hibernate-core/src/main/java/org/hibernate/query/spi/QueryInterpretationCache.java @@ -45,6 +45,9 @@ static Key createInterpretationsKey(InterpretationsKeySource keySource) { void cacheHqlInterpretation(Object cacheKey, HqlInterpretation hqlInterpretation); SelectQueryPlan resolveSelectQueryPlan(Key key, Supplier> creator); + default SelectQueryPlan resolveSelectQueryPlan(K key, Function> creator) { + return resolveSelectQueryPlan( key, () -> creator.apply( key ) ); + } NonSelectQueryPlan getNonSelectQueryPlan(Key key); void cacheNonSelectQueryPlan(Key key, NonSelectQueryPlan plan); 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 1ce0bbdd8bab..51f5a729fe6c 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 @@ -24,6 +24,8 @@ import org.hibernate.CacheMode; import org.hibernate.FlushMode; import org.hibernate.Locking; +import org.hibernate.dialect.pagination.LimitHandler; +import org.hibernate.engine.jdbc.spi.JdbcServices; import org.hibernate.internal.CoreLogging; import org.hibernate.internal.CoreMessageLogger; import org.hibernate.jpa.spi.NativeQueryArrayTransformer; @@ -78,6 +80,7 @@ import org.hibernate.query.results.internal.implicit.ImplicitResultClassBuilder; import org.hibernate.query.spi.AbstractQuery; import org.hibernate.query.spi.DomainQueryExecutionContext; +import org.hibernate.query.spi.Limit; import org.hibernate.query.spi.MutableQueryOptions; import org.hibernate.query.spi.NonSelectQueryPlan; import org.hibernate.query.spi.ParameterMetadataImplementor; @@ -97,6 +100,8 @@ 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.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; @@ -793,15 +798,33 @@ else if ( !isResultTypeAlwaysAllowed( resultType ) mapping = resultSetMapping; } checkResultType( resultType, mapping ); - return isCacheableQuery() - ? getInterpretationCache() - .resolveSelectQueryPlan( selectInterpretationsKey( mapping ), () -> createQueryPlan( mapping ) ) - : createQueryPlan( mapping ); + int parameterStartPosition = 1; + final JdbcServices jdbcServices = getSessionFactory().getJdbcServices(); + if ( !ParameterMarkerStrategyStandard.isStandardRenderer( jdbcServices.getParameterMarkerStrategy() ) + && hasLimit( getQueryOptions().getLimit() ) ) { + final LimitHandler limitHandler = jdbcServices.getDialect().getLimitHandler(); + if ( limitHandler.processSqlMutatesState() ) { + limitHandler.processSql( sqlString, -1, null, getQueryOptions() ); + } + // A non-standard parameter marker strategy is in use, and the limit handler wants to bind parameters + // before the main parameters. This requires recording the start position in the cache key + // because the generated SQL depends on this information + parameterStartPosition = limitHandler.getParameterPositionStart( getQueryOptions().getLimit() ); + } + if ( isCacheableQuery() ) { + return getInterpretationCache().resolveSelectQueryPlan( + selectInterpretationsKey( mapping, parameterStartPosition ), + key -> createQueryPlan( key.getResultSetMapping(), key.getStartPosition() ) + ); + } + else { + return createQueryPlan( mapping, parameterStartPosition ); + } } - private NativeSelectQueryPlan createQueryPlan(ResultSetMapping resultSetMapping) { + private NativeSelectQueryPlan createQueryPlan(ResultSetMapping resultSetMapping, int parameterStartPosition) { final NativeSelectQueryDefinition queryDefinition = new NativeSelectQueryDefinition<>() { - final String sqlString = expandParameterLists(); + final String sqlString = expandParameterLists( parameterStartPosition ); @Override public String getSqlString() { @@ -837,7 +860,7 @@ public Set getAffectedTableNames() { protected NativeSelectQueryPlan createCountQueryPlan() { final NativeSelectQueryDefinition queryDefinition = new NativeSelectQueryDefinition<>() { final BasicType longType = getTypeConfiguration().getBasicTypeForJavaType(Long.class); - final String sqlString = expandParameterLists(); + final String sqlString = expandParameterLists( 1 ); @Override public String getSqlString() { @@ -873,7 +896,7 @@ private NativeQueryInterpreter getNativeQueryInterpreter() { return getSessionFactory().getQueryEngine().getNativeQueryInterpreter(); } - protected String expandParameterLists() { + protected String expandParameterLists(int parameterStartPosition) { if ( parameterOccurrences == null || parameterOccurrences.isEmpty() ) { return sqlString; } @@ -883,11 +906,15 @@ protected String expandParameterLists() { final Dialect dialect = factory.getJdbcServices().getDialect(); final boolean paddingEnabled = factory.getSessionFactoryOptions().inClauseParameterPaddingEnabled(); final int inExprLimit = dialect.getInExpressionCountLimit(); + final ParameterMarkerStrategy parameterMarkerStrategy = factory.getJdbcServices().getParameterMarkerStrategy(); + final boolean needsMarker = !ParameterMarkerStrategyStandard.isStandardRenderer( parameterMarkerStrategy ); - StringBuilder sql = null; + StringBuilder sql = !needsMarker ? null + : new StringBuilder( sqlString.length() + parameterOccurrences.size() * 10 ).append( sqlString ); // Handle parameter lists int offset = 0; + int parameterPosition = parameterStartPosition; for ( ParameterOccurrence occurrence : parameterOccurrences ) { final QueryParameterImplementor queryParameter = occurrence.parameter(); final QueryParameterBinding binding = parameterBindings.getBinding( queryParameter ); @@ -908,15 +935,38 @@ protected String expandParameterLists() { } final int bindValueMaxCount = determineBindValueMaxCount( paddingEnabled, inExprLimit, bindValueCount ); - final String expansionListAsString = - expandList( bindValueMaxCount, isEnclosedInParens ); + final String expansionListAsString = expandList( + bindValueMaxCount, + isEnclosedInParens, + parameterPosition, + parameterMarkerStrategy, + needsMarker + ); final int start = sourcePosition + offset; final int end = start + 1; sql.replace( start, end, expansionListAsString ); offset += expansionListAsString.length() - 1; + parameterPosition += bindValueMaxCount; + } + else if ( needsMarker ) { + final int start = sourcePosition + offset; + final int end = start + 1; + final String parameterMarker = parameterMarkerStrategy.createMarker( parameterPosition, null ); + sql.replace( start, end, parameterMarker ); + offset += parameterMarker.length() - 1; + parameterPosition++; } } } + else if ( needsMarker ) { + final int sourcePosition = occurrence.sourcePosition(); + final int start = sourcePosition + offset; + final int end = start + 1; + final String parameterMarker = parameterMarkerStrategy.createMarker( parameterPosition, null ); + sql.replace( start, end, parameterMarker ); + offset += parameterMarker.length() - 1; + parameterPosition++; + } } return sql == null ? sqlString : sql.toString(); } @@ -936,11 +986,26 @@ private static void logTooManyExpressions( } } - private static String expandList(int bindValueMaxCount, boolean isEnclosedInParens) { + private static String expandList(int bindValueMaxCount, boolean isEnclosedInParens, int parameterPosition, ParameterMarkerStrategy parameterMarkerStrategy, boolean needsMarker) { // HHH-8901 if ( bindValueMaxCount == 0 ) { return isEnclosedInParens ? "null" : "(null)"; } + else if ( needsMarker ) { + final StringBuilder sb = new StringBuilder( bindValueMaxCount * 4 ); + if ( !isEnclosedInParens ) { + sb.append( '(' ); + } + for ( int i = 0; i < bindValueMaxCount; i++ ) { + sb.append( parameterMarkerStrategy.createMarker( parameterPosition + i, null ) ); + sb.append( ',' ); + } + sb.setLength( sb.length() - 1 ); + if ( !isEnclosedInParens ) { + sb.append( ')' ); + } + return sb.toString(); + } else { // Shift 1 bit instead of multiplication by 2 final char[] chars; @@ -1008,11 +1073,12 @@ public static int determineBindValueMaxCount(boolean paddingEnabled, int inExprL return bindValueMaxCount; } - private SelectInterpretationsKey selectInterpretationsKey(ResultSetMapping resultSetMapping) { + private SelectInterpretationsKey selectInterpretationsKey(ResultSetMapping resultSetMapping, int parameterStartPosition) { return new SelectInterpretationsKey( getQueryString(), resultSetMapping, - getSynchronizedQuerySpaces() + getSynchronizedQuerySpaces(), + parameterStartPosition ); } @@ -1028,6 +1094,10 @@ private boolean isCacheableQuery() { return !parameterBindings.hasAnyMultiValuedBindings(); } + private boolean hasLimit(Limit limit) { + return limit != null && !limit.isEmpty(); + } + @Override protected ScrollableResultsImplementor doScroll(ScrollMode scrollMode) { return resolveSelectQueryPlan().performScroll( scrollMode, this ); @@ -1054,7 +1124,7 @@ private NonSelectQueryPlan resolveNonSelectQueryPlan() { } if ( queryPlan == null ) { - final String sqlString = expandParameterLists(); + final String sqlString = expandParameterLists( 1 ); queryPlan = new NativeNonSelectQueryPlanImpl( sqlString, querySpaces, parameterOccurrences ); if ( cacheKey != null ) { getInterpretationCache().cacheNonSelectQueryPlan( cacheKey, queryPlan ); diff --git a/hibernate-core/src/main/java/org/hibernate/query/sql/spi/SelectInterpretationsKey.java b/hibernate-core/src/main/java/org/hibernate/query/sql/spi/SelectInterpretationsKey.java index da7c3864f1ec..e742f26893a6 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sql/spi/SelectInterpretationsKey.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sql/spi/SelectInterpretationsKey.java @@ -10,6 +10,7 @@ import org.hibernate.query.ResultListTransformer; import org.hibernate.query.TupleTransformer; +import org.hibernate.query.results.ResultSetMapping; import org.hibernate.query.spi.QueryInterpretationCache; import org.hibernate.sql.results.jdbc.spi.JdbcValuesMappingProducer; @@ -18,8 +19,9 @@ */ public class SelectInterpretationsKey implements QueryInterpretationCache.Key { private final String sql; - private final JdbcValuesMappingProducer jdbcValuesMappingProducer; + private final ResultSetMapping resultSetMapping; private final Collection querySpaces; + private final int parameterStartPosition; private final int hash; @Deprecated(forRemoval = true) @@ -32,24 +34,36 @@ public SelectInterpretationsKey( this( sql, jdbcValuesMappingProducer, querySpaces ); } + @Deprecated(forRemoval = true) public SelectInterpretationsKey( String sql, JdbcValuesMappingProducer jdbcValuesMappingProducer, Collection querySpaces) { + this( sql, (ResultSetMapping) jdbcValuesMappingProducer, querySpaces, 1 ); + } + + public SelectInterpretationsKey( + String sql, + ResultSetMapping jdbcValuesMappingProducer, + Collection querySpaces, + int parameterStartPosition) { this.sql = sql; - this.jdbcValuesMappingProducer = jdbcValuesMappingProducer; + this.resultSetMapping = jdbcValuesMappingProducer; this.querySpaces = querySpaces; + this.parameterStartPosition = parameterStartPosition; this.hash = generateHashCode(); } private SelectInterpretationsKey( String sql, - JdbcValuesMappingProducer jdbcValuesMappingProducer, + ResultSetMapping resultSetMapping, Collection querySpaces, + int parameterStartPosition, int hash) { this.sql = sql; - this.jdbcValuesMappingProducer = jdbcValuesMappingProducer; + this.resultSetMapping = resultSetMapping; this.querySpaces = querySpaces; + this.parameterStartPosition = parameterStartPosition; this.hash = hash; } @@ -58,12 +72,21 @@ public String getQueryString() { return sql; } + public ResultSetMapping getResultSetMapping() { + return resultSetMapping; + } + + public int getStartPosition() { + return parameterStartPosition; + } + @Override public QueryInterpretationCache.Key prepareForStore() { return new SelectInterpretationsKey( sql, - jdbcValuesMappingProducer.cacheKeyInstance(), + resultSetMapping.cacheKeyInstance(), new HashSet<>( querySpaces ), + parameterStartPosition, hash ); } @@ -86,7 +109,8 @@ public boolean equals(Object o) { return false; } return sql.equals( that.sql ) - && Objects.equals( jdbcValuesMappingProducer, that.jdbcValuesMappingProducer ) - && Objects.equals( querySpaces, that.querySpaces ); + && Objects.equals( resultSetMapping, that.resultSetMapping ) + && Objects.equals( querySpaces, that.querySpaces ) + && parameterStartPosition == that.parameterStartPosition; } } diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java index c92a6172b5c7..7213d5d2fe44 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java @@ -26,6 +26,7 @@ import org.hibernate.id.CompositeNestedGeneratedValueGenerator; import org.hibernate.id.OptimizableGenerator; import org.hibernate.id.enhanced.Optimizer; +import org.hibernate.internal.util.NullnessUtil; import org.hibernate.internal.util.collections.Stack; import org.hibernate.internal.util.collections.StandardStack; import org.hibernate.loader.MultipleBagFetchException; @@ -6197,14 +6198,17 @@ private void resolveSqmParameter( MappingModelExpressible valueMapping, BiConsumer jdbcParameterConsumer) { sqmParameterMappingModelTypes.put( expression, valueMapping ); + final List> jdbcParams = jdbcParamsBySqmParam.get( expression ); + final int parameterId = jdbcParams == null ? jdbcParamsBySqmParam.size() + : NullnessUtil.castNonNull( jdbcParams.get( 0 ).get( 0 ).getParameterId() ); final Bindable bindable = bindable( valueMapping ); if ( bindable instanceof SelectableMappings selectableMappings ) { selectableMappings.forEachSelectable( - (index, selectableMapping) -> jdbcParameterConsumer.accept( index, new SqlTypedMappingJdbcParameter( selectableMapping ) ) + (index, selectableMapping) -> jdbcParameterConsumer.accept( index, new SqlTypedMappingJdbcParameter( selectableMapping, parameterId ) ) ); } else if ( bindable instanceof SelectableMapping selectableMapping ) { - jdbcParameterConsumer.accept( 0, new SqlTypedMappingJdbcParameter( selectableMapping ) ); + jdbcParameterConsumer.accept( 0, new SqlTypedMappingJdbcParameter( selectableMapping, parameterId ) ); } else { final SqlTypedMapping sqlTypedMapping = sqlTypedMapping( expression, bindable ); @@ -6216,12 +6220,12 @@ else if ( bindable instanceof SelectableMapping selectableMapping ) { bindable.forEachJdbcType( (index, jdbcMapping) -> jdbcParameterConsumer.accept( index, - new JdbcParameterImpl( jdbcMapping ) + new JdbcParameterImpl( jdbcMapping, parameterId ) ) ); } else { - jdbcParameterConsumer.accept( 0, new SqlTypedMappingJdbcParameter( sqlTypedMapping ) ); + jdbcParameterConsumer.accept( 0, new SqlTypedMappingJdbcParameter( sqlTypedMapping, parameterId ) ); } } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java index e31ebdff8b00..46b3a75d00fd 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java @@ -70,8 +70,8 @@ import org.hibernate.sql.ast.SqlAstNodeRenderingMode; import org.hibernate.sql.ast.SqlAstTranslator; import org.hibernate.sql.ast.SqlTreeCreationException; -import org.hibernate.sql.ast.internal.ParameterMarkerStrategyStandard; import org.hibernate.sql.ast.internal.TableGroupHelper; +import org.hibernate.sql.ast.internal.ParameterMarkerStrategyStandard; import org.hibernate.sql.ast.tree.AbstractUpdateOrDeleteStatement; import org.hibernate.sql.ast.tree.MutationStatement; import org.hibernate.sql.ast.tree.SqlAstNode; @@ -135,7 +135,6 @@ import org.hibernate.sql.exec.internal.AbstractJdbcParameter; import org.hibernate.sql.exec.internal.JdbcOperationQueryInsertImpl; import org.hibernate.sql.exec.internal.JdbcParameterBindingImpl; -import org.hibernate.sql.exec.internal.JdbcParametersImpl; import org.hibernate.sql.exec.internal.SqlTypedMappingJdbcParameter; import org.hibernate.sql.exec.spi.ExecutionContext; import org.hibernate.sql.exec.spi.JdbcLockStrategy; @@ -177,6 +176,7 @@ import java.sql.SQLException; import java.time.Period; import java.util.ArrayList; +import java.util.Arrays; import java.util.BitSet; import java.util.Collection; import java.util.Collections; @@ -260,7 +260,7 @@ public abstract class AbstractSqlAstTranslator implemen private final StringBuilder sqlBuffer = new StringBuilder(); private final List parameterBinders = new ArrayList<>(); - private final JdbcParametersImpl jdbcParameters = new JdbcParametersImpl(); + private int[] parameterIdToBinderIndex; private JdbcParameterBindings jdbcParameterBindings; private Map appliedParameterBindings = Collections.emptyMap(); private SqlAstNodeRenderingMode parameterRenderingMode = SqlAstNodeRenderingMode.DEFAULT; @@ -4607,11 +4607,10 @@ protected void renderFetchPlusOffsetExpressionAsSingleParameter( appendSql( fetchCount.intValue() + offsetCount.intValue() + offset ); } else { - appendSql( PARAM_MARKER ); final JdbcParameter offsetParameter = (JdbcParameter) offsetClauseExpression; final int offsetValue = offset + fetchCount.intValue(); - jdbcParameters.addParameter( offsetParameter ); - parameterBinders.add( + final int parameterPosition = addParameterBinder( + offsetParameter, (statement, startPosition, jdbcParameterBindings, executionContext) -> { final JdbcParameterBinding binding = jdbcParameterBindings.getBinding( offsetParameter ); if ( binding == null ) { @@ -4627,44 +4626,29 @@ protected void renderFetchPlusOffsetExpressionAsSingleParameter( ); } ); + renderParameterAsParameter( parameterPosition, offsetParameter ); } } else { - appendSql( PARAM_MARKER ); final JdbcParameter offsetParameter = (JdbcParameter) offsetClauseExpression; final JdbcParameter fetchParameter = (JdbcParameter) fetchClauseExpression; - final OffsetReceivingParameterBinder fetchBinder = new OffsetReceivingParameterBinder( + final FetchPlusOffsetParameterBinder fetchBinder = new FetchPlusOffsetParameterBinder( offsetParameter, fetchParameter, offset ); - // We don't register and bind the special OffsetJdbcParameter as that comes from the query options - // And in this case, we only want to bind a single JDBC parameter - if ( !( offsetParameter instanceof OffsetJdbcParameter ) ) { - jdbcParameters.addParameter( offsetParameter ); - parameterBinders.add( - (statement, startPosition, jdbcParameterBindings, executionContext) -> { - final JdbcParameterBinding binding = jdbcParameterBindings.getBinding( offsetParameter ); - if ( binding == null ) { - throw new ExecutionException( "JDBC parameter value not bound - " + offsetParameter ); - } - fetchBinder.dynamicOffset = (Number) binding.getBindValue(); - } - ); - } - jdbcParameters.addParameter( fetchParameter ); - parameterBinders.add( fetchBinder ); + final int parameterPosition = addParameterBinder( fetchParameter, fetchBinder ); + renderParameterAsParameter( parameterPosition, fetchParameter ); } } - private static class OffsetReceivingParameterBinder implements JdbcParameterBinder { + private static class FetchPlusOffsetParameterBinder implements JdbcParameterBinder { private final JdbcParameter offsetParameter; private final JdbcParameter fetchParameter; private final int staticOffset; - private Number dynamicOffset; - public OffsetReceivingParameterBinder( + public FetchPlusOffsetParameterBinder( JdbcParameter offsetParameter, JdbcParameter fetchParameter, int staticOffset) { @@ -4695,8 +4679,11 @@ public void bindParameterValue( offsetValue = executionContext.getQueryOptions().getEffectiveLimit().getFirstRow(); } else { - offsetValue = dynamicOffset.intValue() + staticOffset; - dynamicOffset = null; + final JdbcParameterBinding binding = jdbcParameterBindings.getBinding( offsetParameter ); + if ( binding == null ) { + throw new ExecutionException( "JDBC parameter value not bound - " + offsetParameter ); + } + offsetValue = ((Number) binding.getBindValue()).intValue() + staticOffset; } //noinspection unchecked fetchParameter.getExpressionType().getSingleJdbcMapping().getJdbcValueBinder().bind( @@ -5670,15 +5657,15 @@ protected void renderLiteral(Literal literal, boolean castParameter) { final JdbcLiteralFormatter literalFormatter = literal.getJdbcMapping().getJdbcLiteralFormatter(); // If we encounter a plain literal in the select clause which has no literal formatter, we must render it as parameter if ( literalFormatter == null ) { - parameterBinders.add( literal ); + final int parameterPosition = addParameterBinderOnly( literal ); final JdbcType jdbcType = literal.getJdbcMapping().getJdbcType(); - final String marker = parameterMarkerStrategy.createMarker( parameterBinders.size(), jdbcType ); - final LiteralAsParameter jdbcParameter = new LiteralAsParameter<>( literal, marker ); + final String marker = parameterMarkerStrategy.createMarker( parameterPosition, jdbcType ); + if ( castParameter ) { - renderCasted( jdbcParameter ); + renderCasted( new LiteralAsParameter<>( literal, marker ) ); } else { - jdbcParameter.renderToSql( this, this, sessionFactory ); + jdbcType.appendWriteExpression( marker, this, dialect ); } } else { @@ -7002,15 +6989,8 @@ public void visitParameter(JdbcParameter jdbcParameter) { } protected void visitParameterAsParameter(JdbcParameter jdbcParameter) { - renderParameterAsParameter( jdbcParameter ); - parameterBinders.add( jdbcParameter.getParameterBinder() ); - jdbcParameters.addParameter( jdbcParameter ); - } - - protected final void renderParameterAsParameter(JdbcParameter jdbcParameter) { - final JdbcType jdbcType = jdbcParameter.getExpressionType().getJdbcMapping( 0 ).getJdbcType(); - assert jdbcType != null; - renderParameterAsParameter( parameterBinders.size() + 1, jdbcParameter ); + final int parameterPosition = addParameterBinder( jdbcParameter ); + renderParameterAsParameter( parameterPosition, jdbcParameter ); } protected void renderWrappedParameter(JdbcParameter jdbcParameter) { @@ -7037,6 +7017,53 @@ protected void renderParameterAsParameter(int position, JdbcParameter jdbcParame jdbcType.appendWriteExpression( parameterMarker, this, dialect ); } + protected final int addParameterBinder(JdbcParameter parameter) { + return addParameterBinder( parameter, parameter.getParameterBinder() ); + } + + protected final int addParameterBinder(JdbcParameter parameter, JdbcParameterBinder parameterBinder) { + final Integer parameterId = parameter.getParameterId(); + if ( ParameterMarkerStrategyStandard.isStandardRenderer( parameterMarkerStrategy ) + // Filter parameters are unique and they are not tracked via parameterInfo + || parameter instanceof FilterJdbcParameter + || parameterId == null ) { + return addParameterBinderOnly( parameterBinder ); + } + else { + parameterIdToBinderIndex = ensureCapacity( parameterIdToBinderIndex, parameterId + 1 ); + int binderIndex = parameterIdToBinderIndex[parameterId]; + if ( binderIndex == -1 ) { + parameterIdToBinderIndex[parameterId] = binderIndex = addParameterBinderOnly( parameterBinder ); + } + return binderIndex; + } + } + + private static int[] ensureCapacity(int[] array, int minCapacity) { + int oldCapacity; + if ( array == null ) { + oldCapacity = 0; + array = new int[minCapacity]; + } + else { + oldCapacity = array.length; + if ( minCapacity > oldCapacity ) { + int newCapacity = oldCapacity + (oldCapacity >> 1); + newCapacity = Math.max( Math.max( newCapacity, minCapacity ), 10 ); + array = Arrays.copyOf( array, newCapacity ); + } + } + for ( int i = oldCapacity; i < array.length; i++ ) { + array[i] = -1; + } + return array; + } + + private int addParameterBinderOnly(JdbcParameterBinder parameterBinder) { + parameterBinders.add( parameterBinder ); + return parameterBinders.size(); + } + @Override public void render(SqlAstNode sqlAstNode, SqlAstNodeRenderingMode renderingMode) { final SqlAstNodeRenderingMode original = this.parameterRenderingMode; @@ -8495,7 +8522,7 @@ public void visitCustomTableInsert(TableInsertCustomSql tableInsert) { assert sqlBuffer.toString().isEmpty(); sqlBuffer.append( tableInsert.getCustomSql() ); - tableInsert.forEachParameter( this::applyParameter ); + tableInsert.forEachParameter( this::addParameterBinder ); } @Override @@ -8604,7 +8631,7 @@ public void visitCustomTableUpdate(TableUpdateCustomSql tableUpdate) { assert sqlBuffer.toString().isEmpty(); sqlBuffer.append( tableUpdate.getCustomSql() ); - tableUpdate.forEachParameter( this::applyParameter ); + tableUpdate.forEachParameter( this::addParameterBinder ); } @Override @@ -8668,13 +8695,7 @@ public void visitCustomTableDelete(TableDeleteCustomSql tableDelete) { assert sqlBuffer.toString().isEmpty(); sqlBuffer.append( tableDelete.getCustomSql() ); - tableDelete.forEachParameter( this::applyParameter ); - } - - protected void applyParameter(ColumnValueParameter parameter) { - assert parameter != null; - parameterBinders.add( parameter.getParameterBinder() ); - jdbcParameters.addParameter( parameter ); + tableDelete.forEachParameter( this::addParameterBinder ); } @Override @@ -8711,10 +8732,6 @@ public void visitColumnWriteFragment(ColumnWriteFragment columnWriteFragment) { protected void simpleColumnWriteFragmentRendering(ColumnWriteFragment columnWriteFragment) { appendSql( columnWriteFragment.getFragment() ); - - for ( ColumnValueParameter parameter : columnWriteFragment.getParameters() ) { - parameterBinders.add( parameter.getParameterBinder() ); - jdbcParameters.addParameter( parameter ); - } + columnWriteFragment.getParameters().forEach( this::addParameterBinder ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/expression/JdbcParameter.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/expression/JdbcParameter.java index 1871a5c65299..bff7a487658d 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/expression/JdbcParameter.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/expression/JdbcParameter.java @@ -4,6 +4,7 @@ */ package org.hibernate.sql.ast.tree.expression; +import org.checkerframework.checker.nullness.qual.Nullable; import org.hibernate.sql.exec.spi.JdbcParameterBinder; /** @@ -11,4 +12,5 @@ */ public interface JdbcParameter extends Expression { JdbcParameterBinder getParameterBinder(); + @Nullable Integer getParameterId(); } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/AbstractJdbcParameter.java b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/AbstractJdbcParameter.java index 2d3b2165b9a3..68a1c5bd35c3 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/AbstractJdbcParameter.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/AbstractJdbcParameter.java @@ -7,6 +7,7 @@ import java.sql.PreparedStatement; import java.sql.SQLException; +import org.checkerframework.checker.nullness.qual.Nullable; import org.hibernate.cache.MutableCacheKeyBuilder; import org.hibernate.engine.internal.CacheHelper; import org.hibernate.engine.spi.SharedSessionContractImplementor; @@ -36,9 +37,15 @@ public abstract class AbstractJdbcParameter implements JdbcParameter, JdbcParameterBinder, MappingModelExpressible, SqlExpressible, BasicValuedMapping { private final JdbcMapping jdbcMapping; + private final @Nullable Integer parameterId; public AbstractJdbcParameter(JdbcMapping jdbcMapping) { + this( jdbcMapping, null ); + } + + public AbstractJdbcParameter(JdbcMapping jdbcMapping, @Nullable Integer parameterId) { this.jdbcMapping = jdbcMapping; + this.parameterId = parameterId; } @Override @@ -56,6 +63,11 @@ public MappingType getMappedType() { return jdbcMapping; } + @Override + public @Nullable Integer getParameterId() { + return parameterId; + } + @Override public void accept(SqlAstWalker sqlTreeWalker) { sqlTreeWalker.visitParameter( this ); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/JdbcParameterImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/JdbcParameterImpl.java index 2190fef2bf9c..12004d9777a6 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/JdbcParameterImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/JdbcParameterImpl.java @@ -14,4 +14,8 @@ public class JdbcParameterImpl extends AbstractJdbcParameter { public JdbcParameterImpl(JdbcMapping jdbcMapping) { super( jdbcMapping ); } + + public JdbcParameterImpl(JdbcMapping jdbcMapping, Integer parameterId) { + super( jdbcMapping, parameterId ); + } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/SqlTypedMappingJdbcParameter.java b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/SqlTypedMappingJdbcParameter.java index 0e23f7d83d9c..5e80bc9e9c8b 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/SqlTypedMappingJdbcParameter.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/exec/internal/SqlTypedMappingJdbcParameter.java @@ -4,6 +4,7 @@ */ package org.hibernate.sql.exec.internal; +import org.checkerframework.checker.nullness.qual.Nullable; import org.hibernate.metamodel.mapping.SqlTypedMapping; /** @@ -18,6 +19,11 @@ public SqlTypedMappingJdbcParameter(SqlTypedMapping sqlTypedMapping) { this.sqlTypedMapping = sqlTypedMapping; } + public SqlTypedMappingJdbcParameter(SqlTypedMapping sqlTypedMapping, @Nullable Integer parameterId) { + super( sqlTypedMapping.getJdbcMapping(), parameterId ); + this.sqlTypedMapping = sqlTypedMapping; + } + public SqlTypedMapping getSqlTypedMapping() { return sqlTypedMapping; } 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 a36c4b2e5c48..f2d3c9e8292a 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 @@ -91,9 +91,14 @@ public DeferredResultSetAccess( final String sql = jdbcSelect.getSqlString(); limit = queryOptions.getLimit(); - final boolean hasLimit = hasLimit( jdbcSelect ); - limitHandler = hasLimit ? NoopLimitHandler.NO_LIMIT : dialect.getLimitHandler(); - final String sqlWithLimit = hasLimit ? sql : limitHandler.processSql( sql, limit, queryOptions ); + final boolean needsLimitHandler = needsLimitHandler( jdbcSelect ); + limitHandler = needsLimitHandler ? dialect.getLimitHandler() : NoopLimitHandler.NO_LIMIT; + final String sqlWithLimit = !needsLimitHandler ? sql : limitHandler.processSql( + sql, + jdbcParameterBindings.getBindings().size(), + jdbcServices.getParameterMarkerStrategy(), + queryOptions + ); final LockOptions lockOptions = queryOptions.getLockOptions(); final JdbcLockStrategy jdbcLockStrategy = jdbcSelect.getLockStrategy(); @@ -120,8 +125,8 @@ public DeferredResultSetAccess( } } - private boolean hasLimit(JdbcOperationQuerySelect jdbcSelect) { - return limit == null || limit.isEmpty() || jdbcSelect.usesLimitParameters(); + private boolean needsLimitHandler(JdbcOperationQuerySelect jdbcSelect) { + return limit != null && !limit.isEmpty() && !jdbcSelect.usesLimitParameters(); } private static boolean hasLocking(JdbcLockStrategy jdbcLockStrategy, LockOptions lockOptions) { diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/AbstractLimitHandlerTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/AbstractLimitHandlerTest.java index 39f4f7fda54d..c8fb684e1f4e 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/AbstractLimitHandlerTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/AbstractLimitHandlerTest.java @@ -7,7 +7,6 @@ import org.hibernate.dialect.pagination.LimitHandler; import org.hibernate.dialect.pagination.OffsetFetchLimitHandler; import org.hibernate.query.spi.Limit; -import org.hibernate.query.spi.QueryOptions; import org.junit.jupiter.api.Test; import static org.hibernate.dialect.pagination.AbstractLimitHandler.hasFirstRow; @@ -42,7 +41,8 @@ public void testSqlWithSemicolonInsideQuotedStringAndEndsWithSemicolon() { } protected void assertGenerateExpectedSql(String expected, String sql) { - assertEquals(expected, getLimitHandler().processSql(sql, getLimit(), QueryOptions.NONE)); + assertEquals( expected, getLimitHandler().processSql( sql, 0, null, + new LimitQueryOptions( AbstractLimitHandlerTest.this.getLimit() ) ) ); } protected abstract LimitHandler getLimitHandler(); @@ -68,4 +68,5 @@ else if (hasFirstRow(limit)) { } return " limit ?"; } + } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/DB2DialectTestCase.java b/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/DB2DialectTestCase.java index b3bb82f6f225..688f39bea409 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/DB2DialectTestCase.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/DB2DialectTestCase.java @@ -83,7 +83,7 @@ public void testIntegerOverflowForMaxResults() { Limit rowSelection = new Limit(); rowSelection.setFirstRow(1); rowSelection.setMaxRows(Integer.MAX_VALUE); - String sql = dialect.getLimitHandler().processSql( "select a.id from tbl_a a order by a.id", rowSelection ); + String sql = dialect.getLimitHandler().processSql( "select a.id from tbl_a a order by a.id", -1, null, new LimitQueryOptions( rowSelection ) ); assertTrue( "Integer overflow for max rows in: " + sql, sql.contains("fetch next ? rows only") diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/LimitQueryOptions.java b/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/LimitQueryOptions.java new file mode 100644 index 000000000000..f029e1c1dc0f --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/dialect/LimitQueryOptions.java @@ -0,0 +1,22 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.dialect; + +import org.hibernate.query.spi.Limit; +import org.hibernate.query.spi.QueryOptionsAdapter; + +public class LimitQueryOptions extends QueryOptionsAdapter { + + private final Limit limit; + + public LimitQueryOptions(Limit limit) { + this.limit = limit; + } + + @Override + public Limit getLimit() { + return 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 new file mode 100644 index 000000000000..3ac69011e9de --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/NativeParameterMarkerStrategyTests.java @@ -0,0 +1,178 @@ +/* + * 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.SupportsNonStandardNativeParameterRendering.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 { + + public static final DialectParameterMarkerStrategy INSTANCE = new DialectParameterMarkerStrategy(); + + @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", Book.class ) + .setParameter( "title", parameterValue ); + } else { + nativeQuery = session.createNativeQuery( "select * from books b where b.title = " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Book.class ) + .setParameter( 1, parameterValue ); + }; + nativeQuery.list(); + assertNativeQueryContainsMarkers( 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", Book.class ) + .setParameterList( "titles", parameterValue ); + } else { + nativeQuery = session.createNativeQuery( "select * from books b where b.title in " + ( style == ParameterStyle.ORDINAL ? "?1" : "?" ), Book.class ) + .setParameterList( 1, parameterValue ); + }; + nativeQuery.list(); + assertNativeQueryContainsMarkers( 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", Book.class ) + .setParameter( "author", parameterValue ); + } else { + 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( 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", Book.class ) + .setParameterList( "titles", parameterValue ); + } else { + 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( parameterValue.size() + 2 ); + } ); + } + + private void assertNativeQueryContainsMarkers(int expectedMarkerNum) { + + final var strategy = DialectParameterMarkerStrategy.INSTANCE; + + 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 + int id; + 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 3258f8b8ee1b..69abd19105b2 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 @@ -153,6 +153,24 @@ public void testNativeQuery(SessionFactoryScope scope) { } ); } + @Test + @Jira( "https://hibernate.atlassian.net/browse/HHH-19632" ) + public void testQueryParamReuse(SessionFactoryScope scope) { + final String queryString = "select e from EntityOfBasics e where e.id = :id and e.id = :id"; + + final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector(); + statementInspector.clear(); + + scope.inTransaction( (session) -> { + session.createSelectionQuery( queryString, EntityOfBasics.class ).setParameter( "id", 1 ).list(); + } ); + + assertThat( statementInspector.getSqlQueries() ).hasSize( 1 ); + final String sql = statementInspector.getSqlQueries().get( 0 ); + assertThat( sql ).contains( "?1" ); + assertThat( sql ).doesNotContain( "?2" ); + } + @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 d0dc5ec754f8..7bb583f3801f 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 @@ -88,6 +88,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; @@ -1164,6 +1165,13 @@ public boolean apply(Dialect dialect) { } } + public static class SupportsNonStandardNativeParameterRendering implements DialectFeatureCheck { + @Override + public boolean apply(Dialect dialect) { + return !ParameterMarkerStrategyStandard.isStandardRenderer( dialect.getNativeParameterMarkerStrategy() ); + } + } + private static SqmFunctionRegistry getSqmFunctionRegistry(Dialect dialect) { SqmFunctionRegistry sqmFunctionRegistry = FUNCTION_REGISTRIES.get( dialect );