From 1d1278e58d5262bd039cfac1618bbbe2a3a7e64e Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Wed, 20 Mar 2024 18:25:56 +0100 Subject: [PATCH] HHH-17759 Avoid costly string search and replace --- .../internal/log/DeprecationLogger.java | 7 + .../query/sql/internal/ParameterParser.java | 72 +++++--- .../test/query/sql/ParameterParserTest.java | 160 +++++++++--------- 3 files changed, 136 insertions(+), 103 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/internal/log/DeprecationLogger.java b/hibernate-core/src/main/java/org/hibernate/internal/log/DeprecationLogger.java index e12d47315f..e970223ab8 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/log/DeprecationLogger.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/log/DeprecationLogger.java @@ -283,4 +283,11 @@ public interface DeprecationLogger extends BasicLogger { ) void deprecatedSettingNoReplacement(String settingName); + @LogMessage(level = WARN) + @Message( + id = 90000031, + value = "The native query colon escaping used for the [%s] operator is deprecated and will be removed. Use [%s] instead." + ) + void deprecatedNativeQueryColonEscaping(String oldOperator, String newOperator); + } diff --git a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/ParameterParser.java b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/ParameterParser.java index 0a7f96b312..9b4b953252 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/ParameterParser.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/ParameterParser.java @@ -53,7 +53,6 @@ public class ParameterParser { */ public static void parse(String sqlString, ParameterRecognizer recognizer, boolean nativeJdbcParametersIgnored) throws QueryException { checkIsNotAFunctionCall( sqlString ); - sqlString = preprocessing(sqlString); final int stringLength = sqlString.length(); boolean inSingleQuotes = false; @@ -130,20 +129,52 @@ public class ParameterParser { } // otherwise else { - if ( c == ':' && indx < stringLength - 1 && Character.isJavaIdentifierStart( sqlString.charAt( indx + 1 ) ) - && !(0 < indx && sqlString.charAt( indx - 1 ) == ':')) { - // named parameter - final int right = StringHelper.firstIndexOfChar( sqlString, HQL_SEPARATORS_BITSET, indx + 1 ); - final int chopLocation = right < 0 ? sqlString.length() : right; - final String param = sqlString.substring( indx + 1, chopLocation ); - if ( param.isEmpty() ) { - throw new QueryParameterException( - "Space is not allowed after parameter prefix ':'", - sqlString - ); + if ( c == ':' ) { + if ( indx < stringLength - 1 && Character.isJavaIdentifierStart( sqlString.charAt( indx + 1 ) ) ) { + // named parameter + final int right = StringHelper.firstIndexOfChar( sqlString, HQL_SEPARATORS_BITSET, indx + 1 ); + final int chopLocation = right < 0 ? sqlString.length() : right; + final String param = sqlString.substring( indx + 1, chopLocation ); + if ( param.isEmpty() ) { + throw new QueryParameterException( + "Space is not allowed after parameter prefix ':'", + sqlString + ); + } + recognizer.namedParameter( param, indx ); + indx = chopLocation - 1; + } + else { + // For backwards compatibility, allow some known operators in the escaped form + if ( indx < stringLength - 3 + && sqlString.charAt( indx + 1 ) == ':' + && sqlString.charAt( indx + 2 ) == ':' + && sqlString.charAt( indx + 3 ) == ':' ) { + // Detect the :: operator, escaped as :::: + DeprecationLogger.DEPRECATION_LOGGER.deprecatedNativeQueryColonEscaping( "::::", "::" ); + recognizer.other( ':' ); + recognizer.other( ':' ); + indx += 3; + } + else if ( indx < stringLength - 2 + && sqlString.charAt( indx + 1 ) == ':' + && sqlString.charAt( indx + 2 ) == '=' ) { + // Detect the := operator, escaped as ::= + DeprecationLogger.DEPRECATION_LOGGER.deprecatedNativeQueryColonEscaping( "::=", ":=" ); + recognizer.other( ':' ); + recognizer.other( '=' ); + indx += 2; + } + else { + recognizer.other( ':' ); + // Consume all following colons as they are eagerly to not confuse named parameter detection + while ( indx < stringLength - 1 + && sqlString.charAt( indx + 1 ) == ':' ) { + indx++; + recognizer.other( ':' ); + } + } } - recognizer.namedParameter( param, indx ); - indx = chopLocation - 1; } else if ( c == '?' ) { // could be either a positional or JPA-style ordinal parameter @@ -176,19 +207,6 @@ public class ParameterParser { recognizer.complete(); } - private static String preprocessing(String sqlString) { - final Map preprocessingExchangeMap = Map.of("::=", ":=", "::::", "::"); - for (Map.Entry entry : preprocessingExchangeMap.entrySet()) { - final String preprocessedSqlString = sqlString.replace(entry.getKey(), entry.getValue()); - if (!sqlString.equals(preprocessedSqlString)) { - DeprecationLogger.DEPRECATION_LOGGER.warn( - String.format("An unconventional syntax has been used in the SQL statement. It is recommended to use '%s' instead of '%s'.", entry.getValue(), entry.getKey())); - sqlString = preprocessedSqlString; - } - } - return sqlString; - } - public static void parse(String sqlString, ParameterRecognizer recognizer) throws QueryException { parse( sqlString, recognizer, false ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/sql/ParameterParserTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/sql/ParameterParserTest.java index 554d59f16f..8c4fdb3e3a 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/query/sql/ParameterParserTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/sql/ParameterParserTest.java @@ -13,6 +13,7 @@ import org.hibernate.engine.query.ParameterRecognitionException; import org.hibernate.engine.query.internal.NativeQueryInterpreterStandardImpl; import org.hibernate.query.sql.internal.ParameterParser; import org.hibernate.query.sql.spi.ParameterRecognizer; + import org.hibernate.testing.orm.junit.JiraKey; import org.junit.jupiter.api.Test; @@ -73,7 +74,7 @@ public class ParameterParserTest { recognizer.validate(); - assertTrue(recognizer.getNamedParameters().contains("param")); + assertTrue( recognizer.getNamedParameters().contains( "param" ) ); } @Test @@ -88,7 +89,7 @@ public class ParameterParserTest { recognizer.complete(); recognizer.validate(); - assertTrue( recognizer.getNamedParameters().contains("param")); + assertTrue( recognizer.getNamedParameters().contains( "param" ) ); } @Test @@ -133,94 +134,97 @@ public class ParameterParserTest { recognizer.complete(); recognizer.validate(); - assertTrue(recognizer.getNamedParameters().contains("param")); + assertTrue( recognizer.getNamedParameters().contains( "param" ) ); } - - @Test - @JiraKey( value = "HHH-1237") - public void testParseColonCharacterEscaped() { - final StringBuilder captured = new StringBuilder(); - ParameterRecognizer recognizer = new ParameterRecognizer() { - @Override - public void ordinalParameter(int position) { - fail(); - } - @Override - public void namedParameter(String name, int position) { - fail(); - } + @Test + @JiraKey(value = "HHH-1237") + public void testParseColonCharacterEscaped() { + final StringBuilder captured = new StringBuilder(); + ParameterRecognizer recognizer = new ParameterRecognizer() { + @Override + public void ordinalParameter(int position) { + fail(); + } - @Override - public void jpaPositionalParameter(int name, int position) { - fail(); - } + @Override + public void namedParameter(String name, int position) { + fail(); + } - @Override - public void other(char character) { - captured.append(character); - } + @Override + public void jpaPositionalParameter(int name, int position) { + fail(); + } + + @Override + public void other(char character) { + captured.append( character ); + } @Override public void complete() { } }; - ParameterParser.parse("SELECT @a,(@a::=20) FROM tbl_name", recognizer); + ParameterParser.parse( "SELECT @a,(@a::=20) FROM tbl_name", recognizer ); recognizer.complete(); - assertEquals("SELECT @a,(@a:=20) FROM tbl_name", captured.toString()); - } + assertEquals( "SELECT @a,(@a:=20) FROM tbl_name", captured.toString() ); + } - @Test - @JiraKey( value = "HHH-17759") - public void testParseColonCharacterTypeCasting() { - final StringBuilder captured = new StringBuilder(); - ParameterRecognizer recognizer = new ParameterRecognizer() { - @Override - public void ordinalParameter(int position) { - // don't care - } + @Test + @JiraKey(value = "HHH-17759") + public void testParseColonCharacterTypeCasting() { + final StringBuilder captured = new StringBuilder(); + ParameterRecognizer recognizer = new ParameterRecognizer() { + @Override + public void ordinalParameter(int position) { + // don't care + } - @Override - public void namedParameter(String name, int position) { - // don't care - } + @Override + public void namedParameter(String name, int position) { + // don't care + } - @Override - public void jpaPositionalParameter(int name, int position) { - // don't care - } + @Override + public void jpaPositionalParameter(int name, int position) { + // don't care + } - @Override - public void other(char character) { - captured.append(character); - } + @Override + public void other(char character) { + captured.append( character ); + } - @Override - public void complete() { - } + @Override + public void complete() { + } - }; - String expectedQuery = "SELECT column_name::text FROM table_name"; + }; + String expectedQuery = "SELECT column_name::text FROM table_name"; - ParameterParser.parse("SELECT column_name::text FROM table_name", recognizer); - recognizer.complete(); - assertEquals(expectedQuery, captured.toString()); - - captured.setLength(0); // clear for new test - - ParameterParser.parse("SELECT column_name::::text FROM table_name", recognizer); + ParameterParser.parse( "SELECT column_name::text FROM table_name", recognizer ); recognizer.complete(); - assertEquals(expectedQuery, captured.toString()); - } + assertEquals( expectedQuery, captured.toString() ); - @Test - public void testParseNamedParameter() { - ExtendedParameterRecognizer recognizer = createRecognizer(); - NATIVE_QUERY_INTERPRETER.recognizeParameters("from Stock s where s.stockCode = :stockCode and s.xyz = :pxyz", recognizer); + captured.setLength( 0 ); // clear for new test + + ParameterParser.parse( "SELECT column_name::::text FROM table_name", recognizer ); + recognizer.complete(); + assertEquals( expectedQuery, captured.toString() ); + } + + @Test + public void testParseNamedParameter() { + ExtendedParameterRecognizer recognizer = createRecognizer(); + NATIVE_QUERY_INTERPRETER.recognizeParameters( + "from Stock s where s.stockCode = :stockCode and s.xyz = :pxyz", + recognizer + ); recognizer.complete(); recognizer.validate(); - assertTrue(recognizer.getNamedParameters().contains("stockCode")); + assertTrue(recognizer.getNamedParameters().contains("stockCode")); assertTrue(recognizer.getNamedParameters().contains("pxyz")); assertEquals( 2, recognizer.getNamedParameters().size() ); } @@ -232,15 +236,15 @@ public class ParameterParserTest { recognizer.complete(); recognizer.validate(); - assertEquals( 1, recognizer.getJpaPositionalParameterCount() ); + assertEquals( 1, recognizer.getJpaPositionalParameterCount() ); recognizer = createRecognizer(); - ParameterParser.parse("from Stock s where s.stockCode = ?1 and s.xyz = ?2", recognizer); + ParameterParser.parse( "from Stock s where s.stockCode = ?1 and s.xyz = ?2", recognizer ); recognizer.complete(); recognizer.validate(); - assertEquals( 2, recognizer.getJpaPositionalParameterCount() ); - } + assertEquals( 2, recognizer.getJpaPositionalParameterCount() ); + } @Test public void testJdbcParameterScanningEnabled() { @@ -278,7 +282,7 @@ public class ParameterParserTest { recognizer ); recognizer.validate(); - assertTrue(recognizer.getNamedParameters().contains("id")); + assertTrue( recognizer.getNamedParameters().contains( "id" ) ); assertEquals( 0, recognizer.getOrdinalParameterCount() ); } @@ -289,15 +293,18 @@ public class ParameterParserTest { private interface ExtendedParameterRecognizer extends org.hibernate.query.sql.spi.ParameterRecognizer { void validate(); + int getOrdinalParameterCount(); + int getJpaPositionalParameterCount(); + Set getNamedParameters(); } private final static class TestParameterRecognizer implements ExtendedParameterRecognizer { private int ordinalParameterCount = 0; - private final Set jpaPositionalParameters = new HashSet<>(2); - private final Set namedParameters = new HashSet<>(2); + private final Set jpaPositionalParameters = new HashSet<>( 2 ); + private final Set namedParameters = new HashSet<>( 2 ); @Override public void ordinalParameter(int sourcePosition) { @@ -346,7 +353,8 @@ public class ParameterParserTest { } private ParameterRecognitionException mixedParamStrategy() { - throw new ParameterRecognitionException( "Mixed parameter strategies - use just one of named, positional or JPA-ordinal strategy" ); + throw new ParameterRecognitionException( + "Mixed parameter strategies - use just one of named, positional or JPA-ordinal strategy" ); } } }