From c284315931d58720df2d17f35b00a9f90ef940f3 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Thu, 6 Jan 2022 18:24:12 +0100 Subject: [PATCH] Add validation for format function and remove `aa` format as no database supports long/short AM/PM markers --- .../community/dialect/InformixDialect.java | 1 - .../community/dialect/SQLiteDialect.java | 1 - .../hibernate/dialect/CockroachDialect.java | 2 +- .../org/hibernate/dialect/MySQLDialect.java | 1 - .../org/hibernate/dialect/OracleDialect.java | 1 - .../hibernate/dialect/SQLServerDialect.java | 1 - .../org/hibernate/dialect/SpannerDialect.java | 2 +- .../function/CommonFunctionFactory.java | 24 +++++++++++------ .../dialect/function/DB2FormatEmulation.java | 7 +---- .../function/SQLServerFormatEmulation.java | 5 +--- .../hql/internal/SemanticQueryBuilder.java | 27 +------------------ .../query/sqm/tree/expression/SqmFormat.java | 17 ++++++++++++ .../orm/test/query/hql/FunctionTests.java | 4 +-- .../test/query/hql/StandardFunctionTests.java | 4 +-- 14 files changed, 42 insertions(+), 55 deletions(-) diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/InformixDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/InformixDialect.java index d03935fd47..d130c86f78 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/InformixDialect.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/InformixDialect.java @@ -517,7 +517,6 @@ public class InformixDialect extends Dialect { .replace("d", "%e") //am pm - .replace("aa", "%p") //????? .replace("a", "%p") //????? //hour diff --git a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SQLiteDialect.java b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SQLiteDialect.java index cea98c8205..e1ce532bf1 100644 --- a/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SQLiteDialect.java +++ b/hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/SQLiteDialect.java @@ -593,7 +593,6 @@ public class SQLiteDialect extends Dialect { .replace("d", "%d") //????? //am pm - .replace("aa", "%p") //????? .replace("a", "%p") //????? //hour diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/CockroachDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/CockroachDialect.java index b660ade85c..1b41c183df 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/CockroachDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/CockroachDialect.java @@ -231,7 +231,7 @@ public class CockroachDialect extends Dialect { queryEngine.getSqmFunctionRegistry().namedDescriptorBuilder("format", "experimental_strftime") .setInvariantType( stringType ) - .setExactArgumentCount( 2 ) + .setArgumentsValidator( CommonFunctionFactory.formatValidator() ) .setArgumentListSignature("(TEMPORAL datetime as STRING pattern)") .register(); } 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 b06183955c..d2fd0cfcce 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/MySQLDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/MySQLDialect.java @@ -1066,7 +1066,6 @@ public class MySQLDialect extends Dialect { .replace("D", "%j") //am pm - .replace("aa", "%p") .replace("a", "%p") //hour diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/OracleDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/OracleDialect.java index a8518fe573..91645e6ad4 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/OracleDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/OracleDialect.java @@ -1201,7 +1201,6 @@ public class OracleDialect extends Dialect { .replace("D", fm + "DDD" + fmReset) //am pm - .replace("aa", "AM") .replace("a", "AM") //hour diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/SQLServerDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/SQLServerDialect.java index ec14c519f1..3d7ba6822f 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/SQLServerDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/SQLServerDialect.java @@ -731,7 +731,6 @@ public class SQLServerDialect extends AbstractTransactSQLDialect { //D no equivalent //am pm - .replace("aa", "tt") .replace("a", "tt") //h nothing to do diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/SpannerDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/SpannerDialect.java index 93516f4ac2..c4240859cc 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/SpannerDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/SpannerDialect.java @@ -423,7 +423,7 @@ public class SpannerDialect extends Dialect { queryEngine.getSqmFunctionRegistry().patternDescriptorBuilder("format", "format_timestamp(?2,?1)") .setInvariantType( stringType ) - .setExactArgumentCount( 2 ) + .setArgumentsValidator( CommonFunctionFactory.formatValidator() ) .setArgumentListSignature("(TIMESTAMP datetime as STRING pattern)") .register(); } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/function/CommonFunctionFactory.java b/hibernate-core/src/main/java/org/hibernate/dialect/function/CommonFunctionFactory.java index 05a4ab4631..dc27246cc5 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/function/CommonFunctionFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/function/CommonFunctionFactory.java @@ -9,20 +9,28 @@ package org.hibernate.dialect.function; import java.math.BigDecimal; import java.math.BigInteger; import java.sql.Types; +import java.time.format.DateTimeFormatter; import java.util.Date; import java.util.List; import java.util.Arrays; +import java.util.Locale; import java.util.function.Supplier; +import org.hibernate.QueryException; import org.hibernate.dialect.Dialect; import org.hibernate.metamodel.mapping.BasicValuedMapping; import org.hibernate.query.ReturnableType; import org.hibernate.metamodel.mapping.JdbcMapping; import org.hibernate.query.spi.QueryEngine; +import org.hibernate.query.sqm.produce.function.ArgumentTypesValidator; +import org.hibernate.query.sqm.produce.function.ArgumentsValidator; import org.hibernate.query.sqm.produce.function.FunctionReturnTypeResolver; +import org.hibernate.query.sqm.produce.function.StandardArgumentsValidators; import org.hibernate.query.sqm.produce.function.StandardFunctionReturnTypeResolvers; import org.hibernate.query.sqm.tree.SqmTypedNode; +import org.hibernate.query.sqm.tree.expression.SqmFormat; +import org.hibernate.query.sqm.tree.expression.SqmLiteral; import org.hibernate.sql.ast.SqlAstNodeRenderingMode; import org.hibernate.sql.ast.tree.SqlAstNode; import org.hibernate.type.BasicType; @@ -2331,8 +2339,7 @@ public class CommonFunctionFactory { .setInvariantType( queryEngine.getTypeConfiguration().getBasicTypeRegistry().resolve( StandardBasicTypes.STRING ) ) - .setExactArgumentCount( 2 ) - .setParameterTypes(TEMPORAL, STRING) + .setArgumentsValidator( formatValidator() ) .setArgumentListSignature( "(TEMPORAL datetime as STRING pattern)" ) .register(); } @@ -2347,8 +2354,7 @@ public class CommonFunctionFactory { .setInvariantType( queryEngine.getTypeConfiguration().getBasicTypeRegistry().resolve( StandardBasicTypes.STRING ) ) - .setExactArgumentCount( 2 ) - .setParameterTypes(TEMPORAL, STRING) + .setArgumentsValidator( formatValidator() ) .setArgumentListSignature( "(TEMPORAL datetime as STRING pattern)" ) .register(); } @@ -2363,8 +2369,7 @@ public class CommonFunctionFactory { .setInvariantType( queryEngine.getTypeConfiguration().getBasicTypeRegistry().resolve( StandardBasicTypes.STRING ) ) - .setExactArgumentCount( 2 ) - .setParameterTypes(TEMPORAL, STRING) + .setArgumentsValidator( formatValidator() ) .setArgumentListSignature( "(TEMPORAL datetime as STRING pattern)" ) .register(); } @@ -2379,12 +2384,15 @@ public class CommonFunctionFactory { .setInvariantType( queryEngine.getTypeConfiguration().getBasicTypeRegistry().resolve( StandardBasicTypes.STRING ) ) - .setExactArgumentCount( 2 ) - .setParameterTypes(TEMPORAL, STRING) + .setArgumentsValidator( formatValidator() ) .setArgumentListSignature( "(TEMPORAL datetime as STRING pattern)" ) .register(); } + public static ArgumentsValidator formatValidator() { + return new ArgumentTypesValidator( StandardArgumentsValidators.exactly( 2 ), TEMPORAL, STRING ); + } + /** * Use the 'collate' operator which exists on at least Postgres, MySQL, Oracle, and SQL Server */ diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/function/DB2FormatEmulation.java b/hibernate-core/src/main/java/org/hibernate/dialect/function/DB2FormatEmulation.java index dff709488c..7b6148e75e 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/function/DB2FormatEmulation.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/function/DB2FormatEmulation.java @@ -8,8 +8,6 @@ package org.hibernate.dialect.function; import org.hibernate.dialect.OracleDialect; import org.hibernate.query.sqm.function.AbstractSqmSelfRenderingFunctionDescriptor; -import org.hibernate.query.sqm.produce.function.ArgumentTypesValidator; -import org.hibernate.query.sqm.produce.function.StandardArgumentsValidators; import org.hibernate.query.sqm.produce.function.StandardFunctionReturnTypeResolvers; import org.hibernate.sql.ast.SqlAstTranslator; import org.hibernate.sql.ast.spi.SqlAppender; @@ -22,9 +20,6 @@ import org.hibernate.type.spi.TypeConfiguration; import java.util.List; import jakarta.persistence.TemporalType; -import static org.hibernate.query.sqm.produce.function.FunctionParameterType.STRING; -import static org.hibernate.query.sqm.produce.function.FunctionParameterType.TEMPORAL; - /** * DB2's varchar_format() can't handle quoted literal strings in * the format pattern. So just split the pattern into bits, call @@ -39,7 +34,7 @@ public class DB2FormatEmulation public DB2FormatEmulation(TypeConfiguration typeConfiguration) { super( "format", - new ArgumentTypesValidator( StandardArgumentsValidators.exactly( 2 ), TEMPORAL, STRING ), + CommonFunctionFactory.formatValidator(), StandardFunctionReturnTypeResolvers.invariant( typeConfiguration.getBasicTypeRegistry().resolve( StandardBasicTypes.STRING ) ) diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/function/SQLServerFormatEmulation.java b/hibernate-core/src/main/java/org/hibernate/dialect/function/SQLServerFormatEmulation.java index e6e00807a3..8278344719 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/function/SQLServerFormatEmulation.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/function/SQLServerFormatEmulation.java @@ -22,9 +22,6 @@ import org.hibernate.sql.ast.tree.expression.Format; import org.hibernate.type.StandardBasicTypes; import org.hibernate.type.spi.TypeConfiguration; -import static org.hibernate.query.sqm.produce.function.FunctionParameterType.STRING; -import static org.hibernate.query.sqm.produce.function.FunctionParameterType.TEMPORAL; - /** * SQL Server behaves strangely when the first argument to format is of the type time, so we cast to datetime. * @@ -37,7 +34,7 @@ public class SQLServerFormatEmulation extends AbstractSqmSelfRenderingFunctionDe public SQLServerFormatEmulation(SQLServerDialect dialect, TypeConfiguration typeConfiguration) { super( "format", - new ArgumentTypesValidator( StandardArgumentsValidators.exactly( 2 ), TEMPORAL, STRING ), + CommonFunctionFactory.formatValidator(), StandardFunctionReturnTypeResolvers.invariant( typeConfiguration.getBasicTypeRegistry().resolve( StandardBasicTypes.STRING ) ) diff --git a/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java b/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java index bd1faa424f..573cb80dc3 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java @@ -3481,34 +3481,9 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem ); } - // G era - // y year in era - // Y week year (ISO) - // M month in year - // w week in year (ISO) - // W week in month - // E day name in week - // e day number in week (*very* inconsistent across DBs) - // d day in month - // D day in year - // a AM/PM - // H hour of day (0-23) - // h clock hour of am/pm (1-12) - // m minute of hour - // s second of minute - // S fraction of second - // z time zone name e.g. PST - // x zone offset e.g. +03, +0300, +03:00 - // Z zone offset e.g. +0300 - // see https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html - private static final Pattern FORMAT = Pattern.compile("('[^']+'|[:;/,.!@#$^&?~`|()\\[\\]{}<>\\-+*=]|\\s|G{1,2}|[yY]{1,4}|M{1,4}|w{1,2}|W|E{3,4}|e{1,2}|d{1,2}|D{1,3}|a{1,2}|[Hhms]{1,2}|S{1,6}|[zZx]{1,3})*"); - @Override public Object visitFormat(HqlParser.FormatContext ctx) { - String format = QuotingHelper.unquoteStringLiteral( ctx.getChild( 0 ).getText() ); - if (!FORMAT.matcher(format).matches()) { - throw new SemanticException("illegal format pattern '" + format + "'"); - } + final String format = QuotingHelper.unquoteStringLiteral( ctx.getChild( 0 ).getText() ); return new SqmFormat( format, resolveExpressableTypeBasic( String.class ), diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/expression/SqmFormat.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/expression/SqmFormat.java index 81608f2981..e8b583a8e2 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/expression/SqmFormat.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/expression/SqmFormat.java @@ -6,6 +6,10 @@ */ package org.hibernate.query.sqm.tree.expression; +import java.time.format.DateTimeFormatter; +import java.util.Locale; + +import org.hibernate.query.SemanticException; import org.hibernate.query.sqm.NodeBuilder; import org.hibernate.query.sqm.SemanticQueryWalker; import org.hibernate.query.sqm.SqmExpressable; @@ -21,6 +25,19 @@ public class SqmFormat extends SqmLiteral { SqmExpressable inherentType, NodeBuilder nodeBuilder) { super(value, inherentType, nodeBuilder); + try { + DateTimeFormatter.ofPattern( value ); + } + catch (IllegalArgumentException ex) { + throw new SemanticException( + String.format( + Locale.ROOT, + "Format [%s] does not conform to the expected format of java.time.DateTimeFormatter!", + value + ), + ex + ); + } } @Override 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 a8fe1b15e3..0da4202126 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 @@ -1253,7 +1253,7 @@ public class FunctionTests { public void testFormat(SessionFactoryScope scope) { scope.inTransaction( session -> { - session.createQuery("select format(e.theTime as 'hh:mm:ss aa') from EntityOfBasics e") + session.createQuery("select format(e.theTime as 'hh:mm:ss a') from EntityOfBasics e") .list(); session.createQuery("select format(e.theDate as 'dd/MM/yy'), format(e.theDate as 'EEEE, MMMM dd, yyyy') from EntityOfBasics e") .list(); @@ -1265,7 +1265,7 @@ public class FunctionTests { is("Monday, 25/03/1974") ); assertThat( - session.createQuery("select format(theTime as '''Hello'', hh:mm:ss aa') from EntityOfBasics where id=123").getResultList().get(0), + session.createQuery("select format(theTime as '''Hello'', hh:mm:ss a') from EntityOfBasics where id=123").getResultList().get(0), is("Hello, 08:10:08 PM") ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/StandardFunctionTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/StandardFunctionTests.java index c6938531af..6323ec4e48 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/StandardFunctionTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/StandardFunctionTests.java @@ -881,7 +881,7 @@ public class StandardFunctionTests { public void testFormat(SessionFactoryScope scope) { scope.inTransaction( session -> { - session.createQuery( "select format(e.theTime as 'hh:mm:ss aa') from EntityOfBasics e" ) + session.createQuery( "select format(e.theTime as 'hh:mm:ss a') from EntityOfBasics e" ) .list(); session.createQuery( "select format(e.theDate as 'dd/MM/yy'), format(e.theDate as 'EEEE, MMMM dd, yyyy') from EntityOfBasics e" ) @@ -899,7 +899,7 @@ public class StandardFunctionTests { ); assertThat( session.createQuery( - "select format(e.theTime as '''Hello'', hh:mm:ss aa') from EntityOfBasics e" ) + "select format(e.theTime as '''Hello'', hh:mm:ss a') from EntityOfBasics e" ) .getResultList() .get( 0 ), is( "Hello, 08:10:08 PM" )