From d1e1740d1b7d539a84d36809e3cb19dfd5232341 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Cedomir=20Igaly?= Date: Sun, 3 Nov 2024 16:06:02 +0100 Subject: [PATCH 1/3] HHH-18783 Removed @SkipForDialect from test case Method getSqlType for java.lang.Character without length should return "char(1)", not "char" --- .../dialect/MySQLSqlAstTranslator.java | 20 ++++++++++++------- .../orm/test/query/hql/FunctionTests.java | 1 - 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/MySQLSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/MySQLSqlAstTranslator.java index 16bea157e52a..cd34a9ca33d4 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/MySQLSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/MySQLSqlAstTranslator.java @@ -4,10 +4,6 @@ */ package org.hibernate.dialect; -import java.util.ArrayList; -import java.util.List; -import java.util.Locale; - import org.hibernate.engine.jdbc.Size; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.internal.util.collections.Stack; @@ -41,6 +37,10 @@ import org.hibernate.sql.exec.spi.JdbcOperation; import org.hibernate.sql.exec.spi.JdbcOperationQueryInsert; +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; + /** * A SQL AST translator for MySQL. * @@ -81,9 +81,15 @@ private static String getSqlType(CastTarget castTarget, String sqlType, Dialect case "varchar": case "nchar": case "nvarchar": - return castTarget.getLength() == null - ? "char" - : ( "char(" + castTarget.getLength() + ")" ); + if ( castTarget.getLength() == null ) { + if ( castTarget.getJdbcMapping().getJdbcJavaType().getJavaType() == Character.class ) { + return "char(1)"; + } + else { + return "char"; + } + } + return "char(" + castTarget.getLength() + ")"; case "binary": case "varbinary": return castTarget.getLength() == null diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/FunctionTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/FunctionTests.java index 7471f02d8845..d954d30936bd 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/FunctionTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/FunctionTests.java @@ -1131,7 +1131,6 @@ public void testCastFunctionHexToBinary(SessionFactoryScope scope) { @Test @SkipForDialect( dialectClass = AltibaseDialect.class, reason = "Altibase cast to char does not do truncatation") - @SkipForDialect( dialectClass = MySQLDialect.class, matchSubTypes = true, reason = "MySQL cast does not do truncatation") public void testCastFunctionWithLength(SessionFactoryScope scope) { scope.inTransaction( session -> { From d56a523962e6eaeed29d431ad14beb728bdce5d2 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Mon, 4 Nov 2024 17:41:12 +0100 Subject: [PATCH 2/3] HHH-18783 test for trailing space handling in cast() Signed-off-by: Gavin King --- .../org/hibernate/orm/test/query/hql/FunctionTests.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/FunctionTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/FunctionTests.java index d954d30936bd..f0aa28a40c48 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/FunctionTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/FunctionTests.java @@ -1139,9 +1139,18 @@ public void testCastFunctionWithLength(SessionFactoryScope scope) { assertEquals( 'A', session.createQuery("select cast('ABCDEF' as Character)", Character.class) .getSingleResult() ); + assertEquals( ' ', + session.createQuery("select cast(' X ' as Character)", Character.class) + .getSingleResult() ); assertEquals( "ABC", session.createQuery("select cast('ABCDEF' as String(3))", String.class) .getSingleResult() ); + assertEquals( "ABC", + session.createQuery("select cast('ABC' as String(6))", String.class) + .getSingleResult() ); + assertEquals( "ABC ", + session.createQuery("select cast('ABC DEF' as String(4))", String.class) + .getSingleResult() ); } ); } From 58c42e627bced230c7700c71194d0489be19a925 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Mon, 4 Nov 2024 20:09:47 +0100 Subject: [PATCH 3/3] HHH-18783 some cleanups, and leave a big TODO Signed-off-by: Gavin King --- .../dialect/MariaDBSqlAstTranslator.java | 1 - .../org/hibernate/dialect/MySQLDialect.java | 1 + .../dialect/MySQLSqlAstTranslator.java | 13 +++++++++---- .../sql/ast/spi/AbstractSqlAstTranslator.java | 18 +++++++++--------- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/MariaDBSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/MariaDBSqlAstTranslator.java index f06bf0b48903..ea540352d8c5 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/MariaDBSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/MariaDBSqlAstTranslator.java @@ -401,5 +401,4 @@ protected void renderStringContainsExactlyPredicate(Expression haystack, Express needle.accept( this ); appendSql( ",'~','~~'),'?','~?'),'%','~%'),'%') escape '~'" ); } - } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/MySQLDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/MySQLDialect.java index b7e1820c83b7..93ad2d709f59 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/MySQLDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/MySQLDialect.java @@ -129,6 +129,7 @@ public class MySQLDialect extends Dialect { private static final DatabaseVersion MINIMUM_VERSION = DatabaseVersion.make( 8 ); private final MySQLStorageEngine storageEngine = createStorageEngine(); + private final SizeStrategy sizeStrategy = new SizeStrategyImpl() { @Override public Size resolveSize( diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/MySQLSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/dialect/MySQLSqlAstTranslator.java index cd34a9ca33d4..b672914dfaf4 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/MySQLSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/MySQLSqlAstTranslator.java @@ -57,6 +57,10 @@ public static String getSqlType(CastTarget castTarget, SessionFactoryImplementor return getSqlType( castTarget, sqlType, factory.getJdbcServices().getDialect() ); } + //TODO: this is really, really bad since it circumvents the whole machinery we have in DdlType + // and in the Dialect for doing this in a unified way! These mappings should be held in + // the DdlTypes themselves and should be set up in registerColumnTypes(). Doing it here + // means we have problems distinguishing, say, the 'as Character' special case private static String getSqlType(CastTarget castTarget, String sqlType, Dialect dialect) { if ( sqlType != null ) { int parenthesesIndex = sqlType.indexOf( '(' ); @@ -72,9 +76,9 @@ private static String getSqlType(CastTarget castTarget, String sqlType, Dialect case "float": case "real": case "double precision": - final int precision = castTarget.getPrecision() == null ? - dialect.getDefaultDecimalPrecision() : - castTarget.getPrecision(); + final int precision = castTarget.getPrecision() == null + ? dialect.getDefaultDecimalPrecision() + : castTarget.getPrecision(); final int scale = castTarget.getScale() == null ? Size.DEFAULT_SCALE : castTarget.getScale(); return "decimal(" + precision + "," + scale + ")"; case "char": @@ -82,6 +86,7 @@ private static String getSqlType(CastTarget castTarget, String sqlType, Dialect case "nchar": case "nvarchar": if ( castTarget.getLength() == null ) { + // TODO: this is ugly and fragile, but could easily be handled in a DdlType if ( castTarget.getJdbcMapping().getJdbcJavaType().getJavaType() == Character.class ) { return "char(1)"; } @@ -94,7 +99,7 @@ private static String getSqlType(CastTarget castTarget, String sqlType, Dialect case "varbinary": return castTarget.getLength() == null ? "binary" - : ( "binary(" + castTarget.getLength() + ")" ); + : "binary(" + castTarget.getLength() + ")"; } } return sqlType; 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 b81cb0e3af1b..85d5710bae91 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 @@ -5746,11 +5746,11 @@ else if ( isParameter( expression ) ) { renderCasted( expression ); } } - else if ( expression instanceof CaseSimpleExpression ) { - visitCaseSimpleExpression( (CaseSimpleExpression) expression, true ); + else if ( expression instanceof CaseSimpleExpression caseSimpleExpression ) { + visitCaseSimpleExpression( caseSimpleExpression, true ); } - else if ( expression instanceof CaseSearchedExpression ) { - visitCaseSearchedExpression( (CaseSearchedExpression) expression, true ); + else if ( expression instanceof CaseSearchedExpression caseSearchedExpression ) { + visitCaseSearchedExpression( caseSearchedExpression, true ); } else { renderExpressionAsClauseItem( expression ); @@ -5758,8 +5758,8 @@ else if ( expression instanceof CaseSearchedExpression ) { } protected void renderCasted(Expression expression) { - if ( expression instanceof SqmParameterInterpretation ) { - expression = ( (SqmParameterInterpretation) expression ).getResolvedExpression(); + if ( expression instanceof SqmParameterInterpretation parameterInterpretation ) { + expression = parameterInterpretation.getResolvedExpression(); } final List arguments = new ArrayList<>( 2 ); arguments.add( expression ); @@ -5935,8 +5935,8 @@ assert getStatementStack().getCurrent() instanceof UpdateStatement processNestedTableGroupJoins( tableGroup, null ); processTableGroupJoins( tableGroup ); ModelPartContainer modelPart = tableGroup.getModelPart(); - if ( modelPart instanceof EntityPersister ) { - String[] querySpaces = (String[]) ( (EntityPersister) modelPart ).getQuerySpaces(); + if ( modelPart instanceof EntityPersister persister ) { + final String[] querySpaces = (String[]) persister.getQuerySpaces(); for ( int i = 0; i < querySpaces.length; i++ ) { registerAffectedTable( querySpaces[i] ); } @@ -6113,7 +6113,7 @@ else if ( referenceJoinIndexForPredicateSwap == TableGroupHelper.NO_TABLE_GROUP_ ModelPartContainer modelPart = tableGroup.getModelPart(); if ( modelPart instanceof EntityPersister persister ) { - String[] querySpaces = (String[]) persister.getQuerySpaces(); + final String[] querySpaces = (String[]) persister.getQuerySpaces(); for ( int i = 0; i < querySpaces.length; i++ ) { registerAffectedTable( querySpaces[i] ); }