From 3c5d2b504deb708b02d57d4105fa2e28e1dea8ab Mon Sep 17 00:00:00 2001 From: Davide D'Alto Date: Thu, 21 Aug 2025 17:58:30 +0200 Subject: [PATCH 1/2] [#2464] Skip query parameter processing We used it to replace parameter markers for queries because Vert.x doesn't support the JDBC syntax and our own implementation of `ParameterMarkerStrategy` was not enough in the past. Now all tests pass even without the processing the query. --- .../sql/internal/ReactiveNativeNonSelectQueryPlan.java | 6 +----- .../internal/ReactiveDeferredResultSetAccess.java | 9 +-------- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sql/internal/ReactiveNativeNonSelectQueryPlan.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sql/internal/ReactiveNativeNonSelectQueryPlan.java index be5541c5d..470218e17 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sql/internal/ReactiveNativeNonSelectQueryPlan.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sql/internal/ReactiveNativeNonSelectQueryPlan.java @@ -5,7 +5,6 @@ */ package org.hibernate.reactive.query.sql.internal; - import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -20,7 +19,6 @@ import org.hibernate.query.sql.spi.ParameterOccurrence; import org.hibernate.query.sqm.internal.SqmJdbcExecutionContextAdapter; import org.hibernate.reactive.engine.spi.ReactiveSharedSessionContractImplementor; -import org.hibernate.reactive.pool.impl.Parameters; import org.hibernate.reactive.query.sql.spi.ReactiveNonSelectQueryPlan; import org.hibernate.reactive.sql.exec.internal.StandardReactiveJdbcMutationExecutor; import org.hibernate.sql.exec.internal.JdbcParameterBindingsImpl; @@ -71,10 +69,8 @@ public CompletionStage executeReactiveUpdate(DomainQueryExecutionContex } final SQLQueryParser parser = new SQLQueryParser( sql, null, session.getFactory() ); - - Parameters parameters = Parameters.instance( session.getDialect() ); final JdbcOperationQueryMutation jdbcMutation = new JdbcOperationQueryMutationNative( - parameters.process( parser.process() ), + parser.process(), jdbcParameterBinders, affectedTableNames ); diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/sql/results/internal/ReactiveDeferredResultSetAccess.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/sql/results/internal/ReactiveDeferredResultSetAccess.java index 8de288c0e..0d81a2f8b 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/sql/results/internal/ReactiveDeferredResultSetAccess.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/sql/results/internal/ReactiveDeferredResultSetAccess.java @@ -14,7 +14,6 @@ import org.hibernate.HibernateException; import org.hibernate.JDBCException; import org.hibernate.LockOptions; -import org.hibernate.dialect.Dialect; import org.hibernate.engine.jdbc.spi.JdbcServices; import org.hibernate.engine.jdbc.spi.SqlStatementLogger; import org.hibernate.engine.spi.SessionEventListenerManager; @@ -24,7 +23,6 @@ import org.hibernate.reactive.logging.impl.Log; import org.hibernate.reactive.logging.impl.LoggerFactory; import org.hibernate.reactive.pool.ReactiveConnection; -import org.hibernate.reactive.pool.impl.Parameters; import org.hibernate.reactive.session.ReactiveConnectionSupplier; import org.hibernate.reactive.session.ReactiveSession; import org.hibernate.reactive.util.impl.CompletionStages; @@ -172,11 +170,6 @@ private CompletionStage executeQuery() { return completedFuture( logicalConnection ) .thenCompose( lg -> { LOG.tracef( "Executing query to retrieve ResultSet : %s", getFinalSql() ); - - Dialect dialect = executionContext.getSession().getJdbcServices().getDialect(); - // This must happen at the very last minute in order to process parameters - // added in org.hibernate.dialect.pagination.OffsetFetchLimitHandler.processSql - final String sql = Parameters.instance( dialect ).process( getFinalSql() ); Object[] parameters = PreparedStatementAdaptor.bind( super::bindParameters ); final SessionEventListenerManager eventListenerManager = executionContext @@ -186,7 +179,7 @@ private CompletionStage executeQuery() { eventListenerManager.jdbcExecuteStatementStart(); return connection() - .selectJdbc( sql, parameters ) + .selectJdbc( getFinalSql(), parameters ) .thenCompose( this::validateResultSet ) .whenComplete( (resultSet, throwable) -> { // FIXME: I don't know if this event makes sense for Vert.x From a3999bfdcfcd86354d0f5c354cdeed07efe82324 Mon Sep 17 00:00:00 2001 From: Davide D'Alto Date: Thu, 21 Aug 2025 18:01:27 +0200 Subject: [PATCH 2/2] [#2464] Remove unused classes and related tests --- .../reactive/pool/impl/OracleParameters.java | 15 -- .../reactive/pool/impl/Parameters.java | 153 ------------------ .../pool/impl/PostgresParameters.java | 15 -- .../pool/impl/SQLServerParameters.java | 15 -- .../reactive/ParametersProcessorTest.java | 67 -------- 5 files changed, 265 deletions(-) delete mode 100644 hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/OracleParameters.java delete mode 100644 hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/Parameters.java delete mode 100644 hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/PostgresParameters.java delete mode 100644 hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/SQLServerParameters.java delete mode 100644 hibernate-reactive-core/src/test/java/org/hibernate/reactive/ParametersProcessorTest.java diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/OracleParameters.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/OracleParameters.java deleted file mode 100644 index 2f4dd34c1..000000000 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/OracleParameters.java +++ /dev/null @@ -1,15 +0,0 @@ -/* Hibernate, Relational Persistence for Idiomatic Java - * - * SPDX-License-Identifier: Apache-2.0 - * Copyright: Red Hat Inc. and Hibernate Authors - */ -package org.hibernate.reactive.pool.impl; - -public class OracleParameters extends Parameters { - - public static final OracleParameters INSTANCE = new OracleParameters(); - - private OracleParameters() { - super( ":" ); - } -} diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/Parameters.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/Parameters.java deleted file mode 100644 index d30826846..000000000 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/Parameters.java +++ /dev/null @@ -1,153 +0,0 @@ -/* Hibernate, Relational Persistence for Idiomatic Java - * - * SPDX-License-Identifier: Apache-2.0 - * Copyright: Red Hat Inc. and Hibernate Authors - */ -package org.hibernate.reactive.pool.impl; - -import java.util.function.IntConsumer; - -import org.hibernate.dialect.CockroachDialect; -import org.hibernate.dialect.Dialect; -import org.hibernate.dialect.PostgreSQLDialect; -import org.hibernate.dialect.SQLServerDialect; - -/** - * Some databases have a different parameter syntax, which - * the Vert.x {@link io.vertx.sqlclient.SqlClient} does not abstract. - * This class converts JDBC/ODBC-style {@code ?} parameters generated - * by Hibernate ORM to the native format. - */ -public abstract class Parameters { - - private final String paramPrefix; - - private static final Parameters NO_PARSING = new Parameters( null ) { - @Override - public String process(String sql) { - return sql; - } - - @Override - public String process(String sql, int parameterCount) { - return sql; - } - }; - - protected Parameters(String paramPrefix) { - this.paramPrefix = paramPrefix; - } - - public static Parameters instance(Dialect dialect) { - if ( dialect instanceof PostgreSQLDialect || dialect instanceof CockroachDialect ) { - return PostgresParameters.INSTANCE; - } - if ( dialect instanceof SQLServerDialect ) { - return SQLServerParameters.INSTANCE; - } - return NO_PARSING; - } - - public static boolean isProcessingNotRequired(String sql) { - return sql == null - // There aren't any parameters - || sql.indexOf( '?' ) == -1; - } - - public String process(String sql) { - if ( isProcessingNotRequired( sql ) ) { - return sql; - } - return new Parser( sql, paramPrefix ).result(); - } - - /** - * Replace all JDBC-style {@code ?} parameters with Postgres-style - * {@code $n} parameters in the given SQL string. - */ - public String process(String sql, int parameterCount) { - if ( isProcessingNotRequired( sql ) ) { - return sql; - } - return new Parser( sql, parameterCount, paramPrefix ).result(); - } - - private static class Parser { - - private boolean inString; - private boolean inQuoted; - private boolean inSqlComment; - private boolean inCComment; - private boolean escaped; - private int count = 0; - private StringBuilder result; - private int previous; - - private Parser(String sql, String paramPrefix) { - this( sql, 10, paramPrefix ); - } - - private Parser(String sql, int parameterCount, final String paramPrefix) { - result = new StringBuilder( sql.length() + parameterCount ); - // We aren't using lambdas or method reference because of a bug in the JVM: - // https://bugs.openjdk.java.net/browse/JDK-8161588 - // Please, don't change this unless you've tested it with Quarkus - sql.codePoints().forEach( new IntConsumer() { - @Override - public void accept(int codePoint) { - if ( escaped ) { - escaped = false; - } - else { - switch ( codePoint ) { - case '\\': - escaped = true; - return; - case '"': - if ( !inString && !inSqlComment && !inCComment ) { - inQuoted = !inQuoted; - } - break; - case '\'': - if ( !inQuoted && !inSqlComment && !inCComment ) { - inString = !inString; - } - break; - case '-': - if ( !inQuoted && !inString && !inCComment && previous == '-' ) { - inSqlComment = true; - } - break; - case '\n': - inSqlComment = false; - break; - case '*': - if ( !inQuoted && !inString && !inSqlComment && previous == '/' ) { - inCComment = true; - } - break; - case '/': - if ( previous == '*' ) { - inCComment = false; - } - break; - //TODO: $$-quoted strings - case '?': - if ( !inQuoted && !inString ) { - result.append( paramPrefix ).append( ++count ); - previous = '?'; - return; - } - } - } - previous = codePoint; - result.appendCodePoint( codePoint ); - } - } ); - } - - public String result() { - return result.toString(); - } - } -} diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/PostgresParameters.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/PostgresParameters.java deleted file mode 100644 index b53e37839..000000000 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/PostgresParameters.java +++ /dev/null @@ -1,15 +0,0 @@ -/* Hibernate, Relational Persistence for Idiomatic Java - * - * SPDX-License-Identifier: Apache-2.0 - * Copyright: Red Hat Inc. and Hibernate Authors - */ -package org.hibernate.reactive.pool.impl; - -public class PostgresParameters extends Parameters { - - public static final PostgresParameters INSTANCE = new PostgresParameters(); - - private PostgresParameters() { - super( "$" ); - } -} diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/SQLServerParameters.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/SQLServerParameters.java deleted file mode 100644 index 232c001c1..000000000 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/pool/impl/SQLServerParameters.java +++ /dev/null @@ -1,15 +0,0 @@ -/* Hibernate, Relational Persistence for Idiomatic Java - * - * SPDX-License-Identifier: Apache-2.0 - * Copyright: Red Hat Inc. and Hibernate Authors - */ -package org.hibernate.reactive.pool.impl; - -public class SQLServerParameters extends Parameters { - - public static final SQLServerParameters INSTANCE = new SQLServerParameters(); - - private SQLServerParameters() { - super( "@P" ); - } -} diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/ParametersProcessorTest.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/ParametersProcessorTest.java deleted file mode 100644 index ed4b3323c..000000000 --- a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/ParametersProcessorTest.java +++ /dev/null @@ -1,67 +0,0 @@ -/* Hibernate, Relational Persistence for Idiomatic Java - * - * SPDX-License-Identifier: Apache-2.0 - * Copyright: Red Hat Inc. and Hibernate Authors - */ -package org.hibernate.reactive; - -import java.util.stream.Stream; - -import org.hibernate.reactive.pool.impl.OracleParameters; -import org.hibernate.reactive.pool.impl.PostgresParameters; -import org.hibernate.reactive.pool.impl.SQLServerParameters; - -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.params.provider.Arguments.arguments; - -/** - * Test the {@link org.hibernate.reactive.pool.impl.Parameters} processor for each database - */ -public class ParametersProcessorTest { - - /** - * Each test will replace this placeholder with the correct parameter prefix for the selected database - */ - private static final String PARAM_PREFIX = "__paramPrefix__"; - - - /** - * Return the strings to process and the expected result for each one - */ - static Stream expectations() { - return Stream.of( - arguments( "/* One comment */ \\?", "/* One comment */ ?" ), - arguments( "/* One comment */ ?", "/* One comment */ " + PARAM_PREFIX + "1" ), - arguments( "'Sql text ?'", "'Sql text ?'" ), - arguments( "\\?", "?" ), - arguments( "???", PARAM_PREFIX + "1" + PARAM_PREFIX + "2" + PARAM_PREFIX + "3" ), - arguments( "\\?|?", "?|" + PARAM_PREFIX + "1" ), - arguments( " ? ", " " + PARAM_PREFIX + "1 " ) - ); - } - - @ParameterizedTest - @MethodSource("expectations") - public void testPostgreSQLProcessing(String unprocessed, String expected) { - assertThat( PostgresParameters.INSTANCE.process( unprocessed ) ) - .isEqualTo( expected.replaceAll( PARAM_PREFIX, "\\$" ) ); - } - - @ParameterizedTest - @MethodSource("expectations") - public void testSqlServerProcessing(String unprocessed, String expected) { - assertThat( SQLServerParameters.INSTANCE.process( unprocessed ) ) - .isEqualTo( expected.replaceAll( PARAM_PREFIX, "@P" ) ); - } - - @ParameterizedTest - @MethodSource("expectations") - public void testOracleProcessing(String unprocessed, String expected) { - assertThat( OracleParameters.INSTANCE.process( unprocessed ) ) - .isEqualTo( expected.replaceAll( PARAM_PREFIX, ":" ) ); - } -}