From 521872ea6a381e1819c93e363744a892a7ae904d Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Sun, 10 Nov 2024 18:39:01 +0100 Subject: [PATCH] Fix some issues with Oracle on Hibernate 6+ --- .../blazebit/persistence/spi/DbmsDialect.java | 8 +- .../impl/AbstractCommonQueryBuilder.java | 43 ++++------ .../CriteriaBuilderConfigurationImpl.java | 3 +- .../impl/PaginatedCriteriaBuilderImpl.java | 14 +++- .../impl/dialect/DefaultDbmsDialect.java | 2 +- .../impl/dialect/OracleDbmsDialect.java | 6 +- .../nullsubquery/NullSubqueryFunction.java | 12 ++- .../impl/query/CustomQuerySpecification.java | 14 +--- .../testsuite/DelegatingDbmsDialect.java | 4 +- .../testsuite/DateExtractTest.java | 4 + .../testsuite/SizeTransformationTest.java | 5 ++ .../base/HibernateExtendedQuerySupport.java | 78 +++++++++++++++++-- 12 files changed, 132 insertions(+), 61 deletions(-) diff --git a/core/api/src/main/java/com/blazebit/persistence/spi/DbmsDialect.java b/core/api/src/main/java/com/blazebit/persistence/spi/DbmsDialect.java index 9c0007722a..63aa0019bc 100644 --- a/core/api/src/main/java/com/blazebit/persistence/spi/DbmsDialect.java +++ b/core/api/src/main/java/com/blazebit/persistence/spi/DbmsDialect.java @@ -39,11 +39,13 @@ public interface DbmsDialect { public boolean supportsNonRecursiveWithClause(); /** - * Returns true if the dbms supports the with clause head for aliasing, false otherwise. + * Returns true if the dbms supports pagination in the with clause directly or needs to alias selection items. + * This is only needed for Hibernate ORM which uses Oracles {@code fetch first} syntax, + * since that is implemented through a query rewrite that needs aliases. * - * @return Whether the with clause head is supported by the dbms + * @return Whether pagination is supported in the with clause head by the dbms */ - public boolean supportsWithClauseHead(); + public boolean supportsPaginationInWithClause(); /** * Returns the SQL representation for the normal or recursive with clause. diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractCommonQueryBuilder.java b/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractCommonQueryBuilder.java index 011338a78d..5344107781 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractCommonQueryBuilder.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractCommonQueryBuilder.java @@ -3452,39 +3452,28 @@ protected List getCteNodes(boolean isSubquery) { String cteName = cteInfo.cteType.getName(); final List columnNames = cteInfo.columnNames; String head; - String[] aliases; - - if (mainQuery.dbmsDialect.supportsWithClauseHead()) { - sb.setLength(0); - sb.append(cteName); - sb.append('('); - for (int i = 0; i < columnNames.size(); i++) { - String column = columnNames.get(i); - if (i != 0) { - sb.append(", "); - } + sb.setLength(0); + sb.append(cteName); + sb.append('('); - sb.append(column); + for (int i = 0; i < columnNames.size(); i++) { + String column = columnNames.get(i); + if (i != 0) { + sb.append(", "); } - sb.append(')'); - head = sb.toString(); - aliases = null; - } else { - sb.setLength(0); - sb.append(cteName); - List list = new ArrayList<>(columnNames.size()); + sb.append(column); + } - for (int i = 0; i < columnNames.size(); i++) { - String[] columns = mainQuery.metamodel.getManagedType(ExtendedManagedType.class, cteInfo.cteType.getJavaType()).getAttribute(columnNames.get(i)).getColumnNames(); - for (String column : columns) { - list.add(column); - } - } + sb.append(')'); + head = sb.toString(); - head = sb.toString(); - aliases = list.toArray(new String[list.size()]); + String[] aliases; + if (mainQuery.dbmsDialect.supportsPaginationInWithClause() || !cteInfo.nonRecursiveCriteriaBuilder.hasLimit()) { + aliases = null; + } else { + aliases = columnNames.toArray(new String[0]); } String nonRecursiveWithClauseSuffix = null; diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/CriteriaBuilderConfigurationImpl.java b/core/impl/src/main/java/com/blazebit/persistence/impl/CriteriaBuilderConfigurationImpl.java index 5ceacd0414..bce4cad3dd 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/CriteriaBuilderConfigurationImpl.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/CriteriaBuilderConfigurationImpl.java @@ -1671,7 +1671,8 @@ private void loadInternalFunctions() { // null subquery function jpqlFunctionGroup = new JpqlFunctionGroup(NullSubqueryFunction.FUNCTION_NAME, false); - jpqlFunctionGroup.add(null, new NullSubqueryFunction()); + jpqlFunctionGroup.add(null, new NullSubqueryFunction(null)); + jpqlFunctionGroup.add("oracle", new NullSubqueryFunction(" from dual")); registerFunction(jpqlFunctionGroup); // subquery diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedCriteriaBuilderImpl.java b/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedCriteriaBuilderImpl.java index ae71820cd8..714c6e1655 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedCriteriaBuilderImpl.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedCriteriaBuilderImpl.java @@ -999,9 +999,15 @@ private Map.Entry, ObjectBuilder> getObjectQuery(boolean normal return new AbstractMap.SimpleEntry, ObjectBuilder>(query, objectBuilder); } - private TypedQuery getIdQuery(String idQueryString, boolean normalQueryMode, Set keyRestrictedLeftJoins, List entityFunctions) { + private TypedQuery getIdQuery(String idQueryString, boolean normalQueryMode, Set keyRestrictedLeftJoins, List entityFunctions) { + Class resultType; + if (needsNewIdList || withInlineCountQuery) { + resultType = Object[].class; + } else { + resultType = Object.class; + } if (normalQueryMode && isEmpty(keyRestrictedLeftJoins, ID_QUERY_CLAUSE_EXCLUSIONS)) { - TypedQuery idQuery = em.createQuery(idQueryString, Object[].class); + TypedQuery idQuery = em.createQuery(idQueryString, resultType); if (isCacheable()) { mainQuery.jpaProvider.setCacheable(idQuery); } @@ -1014,7 +1020,7 @@ private TypedQuery getIdQuery(String idQueryString, boolean normalQuer return parameterManager.getCriteriaNameMapping() == null ? idQuery : new TypedQueryWrapper<>(idQuery, parameterManager.getCriteriaNameMapping()); } - TypedQuery baseQuery = em.createQuery(idQueryString, Object[].class); + TypedQuery baseQuery = em.createQuery(idQueryString, resultType); Set parameterListNames = parameterManager.getParameterListNames(baseQuery); List keyRestrictedLeftJoinAliases = getKeyRestrictedLeftJoinAliases(baseQuery, keyRestrictedLeftJoins, ID_QUERY_CLAUSE_EXCLUSIONS); @@ -1026,7 +1032,7 @@ private TypedQuery getIdQuery(String idQueryString, boolean normalQuer mainQuery.cteManager.isRecursive(), ctes, shouldRenderCteNodes, mainQuery.getQueryConfiguration().isQueryPlanCacheEnabled(), null ); - CustomSQLTypedQuery idQuery = new CustomSQLTypedQuery( + CustomSQLTypedQuery idQuery = new CustomSQLTypedQuery<>( querySpecification, baseQuery, parameterManager.getCriteriaNameMapping(), diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/DefaultDbmsDialect.java b/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/DefaultDbmsDialect.java index 1e5142f932..fc4d7365df 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/DefaultDbmsDialect.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/DefaultDbmsDialect.java @@ -87,7 +87,7 @@ public boolean supportsNonRecursiveWithClause() { } @Override - public boolean supportsWithClauseHead() { + public boolean supportsPaginationInWithClause() { return supportsWithClause(); } diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/OracleDbmsDialect.java b/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/OracleDbmsDialect.java index aa73946a0b..a8700a2553 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/OracleDbmsDialect.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/dialect/OracleDbmsDialect.java @@ -107,9 +107,9 @@ public String getWithClause(boolean recursive) { } @Override - public boolean supportsWithClauseHead() { - // NOTE: For 10g return false - return true; + public boolean supportsPaginationInWithClause() { + // Actually, only need it for Hibernate ORM + return false; } @Override diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/function/nullsubquery/NullSubqueryFunction.java b/core/impl/src/main/java/com/blazebit/persistence/impl/function/nullsubquery/NullSubqueryFunction.java index 88aee72b34..d988ddc076 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/function/nullsubquery/NullSubqueryFunction.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/function/nullsubquery/NullSubqueryFunction.java @@ -17,6 +17,12 @@ public class NullSubqueryFunction implements JpqlFunction { public static final String FUNCTION_NAME = "null_subquery"; + private final String fromDual; + + public NullSubqueryFunction(String fromDual) { + this.fromDual = fromDual; + } + @Override public boolean hasArguments() { return true; @@ -34,6 +40,10 @@ public Class getReturnType(Class firstArgumentType) { @Override public void render(FunctionRenderContext context) { - context.addChunk("(select null)"); + context.addChunk("(select null"); + if (fromDual != null) { + context.addChunk(fromDual); + } + context.addChunk(")"); } } diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/query/CustomQuerySpecification.java b/core/impl/src/main/java/com/blazebit/persistence/impl/query/CustomQuerySpecification.java index 38572910ff..986be201fc 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/query/CustomQuerySpecification.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/query/CustomQuerySpecification.java @@ -239,17 +239,9 @@ public String extract(StringBuilder sb, int index, int currentPosition) { newSqlSb.append(','); } - String originalAlias = SqlUtils.extractAlias(sb); - int aliasPosition = sb.length() - originalAlias.length() - 1; - // Replace the original alias with the new one - if (aliasPosition != -1 && sb.charAt(aliasPosition) == ' ') { - newSqlSb.append(sb, 0, aliasPosition + 1); - } else { - // Append the new alias - newSqlSb.append(sb); - newSqlSb.append(" as "); - } - + // Append the new alias + newSqlSb.append(sb); + newSqlSb.append(" as "); newSqlSb.append(newAliases[index]); return Integer.toString(currentPosition); diff --git a/core/testsuite/src/main/java/com/blazebit/persistence/testsuite/DelegatingDbmsDialect.java b/core/testsuite/src/main/java/com/blazebit/persistence/testsuite/DelegatingDbmsDialect.java index ff4018262f..fb7b2c6dea 100644 --- a/core/testsuite/src/main/java/com/blazebit/persistence/testsuite/DelegatingDbmsDialect.java +++ b/core/testsuite/src/main/java/com/blazebit/persistence/testsuite/DelegatingDbmsDialect.java @@ -52,8 +52,8 @@ public boolean supportsNonRecursiveWithClause() { } @Override - public boolean supportsWithClauseHead() { - return delegate.supportsWithClauseHead(); + public boolean supportsPaginationInWithClause() { + return delegate.supportsPaginationInWithClause(); } @Override diff --git a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/DateExtractTest.java b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/DateExtractTest.java index 84171f1d6c..2459180965 100644 --- a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/DateExtractTest.java +++ b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/DateExtractTest.java @@ -6,6 +6,7 @@ package com.blazebit.persistence.testsuite; import com.blazebit.persistence.CriteriaBuilder; +import com.blazebit.persistence.testsuite.base.jpa.category.NoOracle; import com.blazebit.persistence.testsuite.entity.Document; import com.blazebit.persistence.testsuite.entity.Person; import com.blazebit.persistence.testsuite.entity.Version; @@ -14,6 +15,7 @@ import org.junit.AfterClass; import org.junit.Before; import org.junit.Test; +import org.junit.experimental.categories.Category; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -490,6 +492,8 @@ public void testDateExtractMicrosecond() { } @Test + // NOTE: Hibernate ORM has a bug: https://hibernate.atlassian.net/browse/HHH-18837 + @Category({ NoOracle.class }) public void testDateExtractEpoch() { // Set the client timezone TimeZone.setDefault(clientTimeZone); diff --git a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/SizeTransformationTest.java b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/SizeTransformationTest.java index 466bc072c0..4251d40eb5 100644 --- a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/SizeTransformationTest.java +++ b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/SizeTransformationTest.java @@ -15,6 +15,7 @@ import com.blazebit.persistence.CriteriaBuilder; import com.blazebit.persistence.impl.function.subquery.SubqueryFunction; import com.blazebit.persistence.testsuite.base.jpa.category.NoMSSQL; +import com.blazebit.persistence.testsuite.base.jpa.category.NoOracle; import com.blazebit.persistence.testsuite.entity.Document; import com.blazebit.persistence.testsuite.entity.Person; import com.blazebit.persistence.testsuite.entity.Version; @@ -98,6 +99,8 @@ public void testSizeToCountTransformationWithList1() { } @Test + // NOTE: Hibernate ORM bug: https://hibernate.atlassian.net/browse/HHH-18838 + @Category({ NoOracle.class }) public void testSizeToCountTransformationWithList2() { CriteriaBuilder cb = cbf.create(em, Tuple.class).from(Document.class, "d") .select("SIZE(d.people)") @@ -123,6 +126,8 @@ public void testSizeToCountTransformationWithMap1() { } @Test + // NOTE: Hibernate ORM bug: https://hibernate.atlassian.net/browse/HHH-18838 + @Category({ NoOracle.class }) public void testSizeToCountTransformationWithMap2() { CriteriaBuilder cb = cbf.create(em, Tuple.class).from(Document.class, "d") .select("SIZE(d.contacts)") diff --git a/integration/hibernate6-base/src/main/java/com/blazebit/persistence/integration/hibernate/base/HibernateExtendedQuerySupport.java b/integration/hibernate6-base/src/main/java/com/blazebit/persistence/integration/hibernate/base/HibernateExtendedQuerySupport.java index af942014cf..798db3a340 100644 --- a/integration/hibernate6-base/src/main/java/com/blazebit/persistence/integration/hibernate/base/HibernateExtendedQuerySupport.java +++ b/integration/hibernate6-base/src/main/java/com/blazebit/persistence/integration/hibernate/base/HibernateExtendedQuerySupport.java @@ -9,6 +9,7 @@ import com.blazebit.persistence.ReturningResult; import com.blazebit.persistence.spi.ConfigurationSource; import com.blazebit.persistence.spi.DbmsDialect; +import com.blazebit.persistence.spi.DbmsStatementType; import com.blazebit.persistence.spi.ExtendedQuerySupport; import com.blazebit.reflection.ReflectionUtils; import jakarta.persistence.EntityManager; @@ -156,6 +157,7 @@ public class HibernateExtendedQuerySupport implements ExtendedQuerySupport { private static final Logger LOG = Logger.getLogger(HibernateExtendedQuerySupport.class.getName()); + private static final String[] KNOWN_STATEMENTS = { "select ", "insert ", "update ", "delete " }; private static final Constructor TUPLE_METADATA_CONSTRUCTOR_62; private static final Constructor TUPLE_METADATA_CONSTRUCTOR_63; private static final RowTransformer ROW_TRANSFORMER_SINGULAR_RETURN; @@ -1169,20 +1171,48 @@ public boolean hasQueryExecutionToBeAddedToStatistics() { try { if (modificationBaseQuery != null) { - QuerySqmImpl querySqm = modificationBaseQuery.unwrap( QuerySqmImpl.class ); + QuerySqmImpl querySqm = modificationBaseQuery.unwrap(QuerySqmImpl.class); SqmStatement modificationSqmStatement = querySqm.getSqmStatement(); - if ( modificationSqmStatement instanceof SqmDmlStatement) { + if (modificationSqmStatement instanceof SqmDmlStatement) { SqmDmlStatement sqmDmlStatement = (SqmDmlStatement) modificationSqmStatement; - BulkOperationCleanupAction.schedule( executionContext.getSession(), sqmDmlStatement ); - if ( !dbmsDialect.supportsModificationQueryInWithClause() && sqmDmlStatement instanceof SqmDeleteStatement ) { + BulkOperationCleanupAction.schedule(executionContext.getSession(), sqmDmlStatement); + if (sqmDmlStatement instanceof SqmDeleteStatement && !dbmsDialect.supportsModificationQueryInWithClause()) { + CollectionTableDeleteInfo collectionTableDeletes = getCollectionTableDeletes(querySqm); + List deletes; + int withIndex; + if ((withIndex = finalSql.indexOf("with ")) != -1) { + int end = getCTEEnd(finalSql, withIndex); + + int maxLength = 0; + + for (JdbcOperationQueryMutation delete : collectionTableDeletes.deletes) { + maxLength = Math.max(maxLength, delete.getSqlString().length()); + } + + deletes = new ArrayList<>(collectionTableDeletes.deletes.size()); + StringBuilder newSb = new StringBuilder(end + maxLength); + // Prefix properly with cte + StringBuilder withClauseSb = new StringBuilder(end - withIndex); + withClauseSb.append(finalSql, withIndex, end); + + for (JdbcOperationQueryMutation delete : collectionTableDeletes.deletes) { + // TODO: The strings should also receive the simple CTE name instead of the complex one + newSb.append(delete.getSqlString()); + dbmsDialect.appendExtendedSql(newSb, DbmsStatementType.DELETE, false, false, withClauseSb, null, null, null, null, null); + deletes.add(new JdbcOperationQueryDelete(newSb.toString(), delete.getParameterBinders(), delete.getAffectedTableNames(), delete.getAppliedParameters())); + newSb.setLength(0); + } + } else { + deletes = collectionTableDeletes.deletes; + } + Function statementCreator = sql -> session.getJdbcCoordinator() .getStatementPreparer() - .prepareStatement( sql ); + .prepareStatement(sql); BiConsumer expectationCheck = (integer, preparedStatement) -> { }; SqmJdbcExecutionContextAdapter executionContextAdapter = SqmJdbcExecutionContextAdapter.usingLockingAndPaging( - modificationBaseQuery.unwrap( DomainQueryExecutionContext.class ) ); - CollectionTableDeleteInfo collectionTableDeletes = getCollectionTableDeletes( querySqm ); - for ( JdbcOperationQueryMutation delete : collectionTableDeletes.deletes ) { + modificationBaseQuery.unwrap(DomainQueryExecutionContext.class)); + for (JdbcOperationQueryMutation delete : deletes) { session.getFactory().getJdbcServices().getJdbcMutationExecutor().execute( delete, collectionTableDeletes.parameterBindings, @@ -1223,6 +1253,38 @@ public boolean hasQueryExecutionToBeAddedToStatistics() { } } + private int getCTEEnd(String sql, int start) { + int parenthesis = 0; + QuoteMode mode = QuoteMode.NONE; + boolean started = false; + + int i = start; + int end = sql.length(); + OUTER: while (i < end) { + final char c = sql.charAt(i); + mode = mode.onChar(c); + + if (mode == QuoteMode.NONE) { + if (c == '(') { + started = true; + parenthesis++; + } else if (c == ')') { + parenthesis--; + } else if (started && parenthesis == 0 && c != ',' && !Character.isWhitespace(c)) { + for (String statementType : KNOWN_STATEMENTS) { + if (sql.regionMatches(true, i, statementType, 0, statementType.length())) { + break OUTER; + } + } + } + } + + i++; + } + + return i; + } + private static String[][] getReturningColumns(boolean caseInsensitive, String exampleQuerySql) { int fromIndex = exampleQuerySql.indexOf("from"); int selectIndex = exampleQuerySql.indexOf("select");