major rework of Template + battery of new tests

I discovered that the over-complex support for ANSI trim() was
completely broken, unsurprisingly, given the complexity of the
implementation, and the absence of tests.

Signed-off-by: Gavin King <gavin@hibernate.org>

hibernate-core/src/main/java/org/hibernate/internal/FilterHelper.java

@@ -83,8 +83,7 @@ public class FilterHelper {
 						filter.getCondition(),
 						FilterImpl.MARKER,
 						factory.getJdbcServices().getDialect(),
-						factory.getTypeConfiguration(),
+						factory.getTypeConfiguration()
-						factory.getQueryEngine().getSqmFunctionRegistry()
 				);
 				filterConditions[filterCount] = safeInterning( autoAliasedCondition );
 				filterAutoAliasFlags[filterCount] = true;

hibernate-core/src/main/java/org/hibernate/mapping/Formula.java

@@ -43,7 +43,7 @@ public class Formula implements Selectable, Serializable {
 
 	@Override
 	public String getTemplate(Dialect dialect, TypeConfiguration typeConfiguration, SqmFunctionRegistry registry) {
-		final String template = renderWhereStringTemplate( formula, dialect, typeConfiguration, registry );
+		final String template = renderWhereStringTemplate( formula, dialect, typeConfiguration );
 		return safeInterning( replace( template, "{alias}", TEMPLATE ) );
 	}
 

hibernate-core/src/main/java/org/hibernate/mapping/PersistentClass.java

@@ -1005,9 +1005,9 @@ public abstract class PersistentClass implements IdentifiableTypeClass, Attribut
 		this.superMappedSuperclass = superMappedSuperclass;
 	}
 
-	public void assignCheckConstraintsToTable(Dialect dialect, TypeConfiguration types, SqmFunctionRegistry functions) {
+	public void assignCheckConstraintsToTable(Dialect dialect, TypeConfiguration types) {
 		for ( CheckConstraint checkConstraint : checkConstraints ) {
-			container( collectColumnNames( checkConstraint.getConstraint(), dialect, types, functions ) )
+			container( collectColumnNames( checkConstraint.getConstraint(), dialect, types ) )
 					.getTable().addCheck( checkConstraint );
 		}
 	}

hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java

@@ -332,8 +332,7 @@ public abstract class AbstractCollectionPersister
 			sqlWhereStringTemplate = Template.renderWhereStringTemplate(
 					sqlWhereString,
 					dialect,
-					creationContext.getTypeConfiguration(),
+					creationContext.getTypeConfiguration()
-					creationContext.getFunctionRegistry()
 			);
 		}
 		else {
@@ -575,8 +574,7 @@ public abstract class AbstractCollectionPersister
 			manyToManyWhereTemplate = Template.renderWhereStringTemplate(
 					manyToManyWhereString,
 					creationContext.getDialect(),
-					creationContext.getTypeConfiguration(),
+					creationContext.getTypeConfiguration()
-					creationContext.getFunctionRegistry()
 			);
 		}
 

hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java

@@ -556,9 +556,9 @@ public abstract class AbstractEntityPersister
 		final TypeConfiguration typeConfiguration = creationContext.getTypeConfiguration();
 		final SqmFunctionRegistry functionRegistry = creationContext.getFunctionRegistry();
 
-		List<Column> columns = persistentClass.getIdentifier().getColumns();
+		final List<Column> columns = persistentClass.getIdentifier().getColumns();
 		for (int i = 0; i < columns.size(); i++ ) {
-			Column column = columns.get(i);
+			final Column column = columns.get(i);
 			rootTableKeyColumnNames[i] = column.getQuotedName( dialect );
 			rootTableKeyColumnReaders[i] = column.getReadExpr( dialect );
 			rootTableKeyColumnReaderTemplates[i] = column.getTemplate(
@@ -594,8 +594,7 @@ public abstract class AbstractEntityPersister
 			sqlWhereStringTemplate = Template.renderWhereStringTemplate(
 					"(" + persistentClass.getWhere() + ")",
 					dialect,
-					typeConfiguration,
+					typeConfiguration
-					functionRegistry
 			);
 		}
 

hibernate-core/src/main/java/org/hibernate/query/sqm/function/SqmFunctionRegistry.java

@@ -64,8 +64,8 @@ public class SqmFunctionRegistry {
 	}
 
 	/**
-	 * Find a SqmFunctionTemplate by name.  Returns {@code null} if
+	 * Find a {@link SqmFunctionDescriptor} by name.
-	 * no such function is found.
+	 * Returns {@code null} if no such function is found.
 	 */
 	public SqmFunctionDescriptor findFunctionDescriptor(String functionName) {
 		SqmFunctionDescriptor found = null;

hibernate-core/src/main/java/org/hibernate/sql/Template.java

@@ -7,16 +7,13 @@
 package org.hibernate.sql;
 
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 import java.util.StringTokenizer;
+import java.util.function.Function;
 
-import org.hibernate.HibernateException;
 import org.hibernate.dialect.Dialect;
-import org.hibernate.query.sqm.function.SqmFunctionDescriptor;
-import org.hibernate.query.sqm.function.SqmFunctionRegistry;
 import org.hibernate.type.spi.TypeConfiguration;
 
 import static java.lang.Boolean.parseBoolean;
@@ -24,80 +21,80 @@ import static java.lang.Character.isLetter;
 import static org.hibernate.internal.util.StringHelper.WHITESPACE;
 
 /**
- * Parses SQL fragments specified in mapping documents.
+ * Parses SQL fragments specified in mapping documents. The SQL fragment
+ * should be written in the native SQL dialect of the target database,
+ * with the following special exceptions:
+ * <ul>
+ *     <li>any backtick-quoted identifiers, for example {@code `hello`},
+ *     is interpreted as a quoted identifier and re-quoted using the
+ *     {@linkplain Dialect#quote native quoted identifier syntax} of
+ *     the database, and</li>
+ *     <li>the literal identifiers {@code true} and {@code false} are
+ *     interpreted are literal boolean values, and replaced with
+ *     {@linkplain Dialect#toBooleanValueString dialect-specific
+ *     literal values}.
+ *     </li>
+ * </ul>
+ *
+ * @implNote This is based on a simple scanner-based state machine.
+ *           It is NOT in any way, shape, nor form, a parser, since
+ *           we simply cannot recognize the syntax of every dialect
+ *           of SQL we support.
  *
  * @author Gavin King
  */
 public final class Template {
 
-	private static final Set<String> KEYWORDS = new HashSet<>();
+	private static final Set<String> KEYWORDS = Set.of(
-	private static final Set<String> BEFORE_TABLE_KEYWORDS = new HashSet<>();
+			"and",
-	private static final Set<String> FUNCTION_KEYWORDS = new HashSet<>();
+			"or",
-	private static final Set<String> LITERAL_PREFIXES = new HashSet<>();
+			"not",
-	public static final String PUNCTUATION = "=><!+-*/()',|&`";
+			"like",
+			"escape",
+			"is",
+			"in",
+			"between",
+			"null",
+			"select",
+			"distinct",
+			"from",
+			"join",
+			"inner",
+			"outer",
+			"left",
+			"right",
+			"on",
+			"where",
+			"having",
+			"group",
+			"order",
+			"by",
+			"desc",
+			"asc",
+			"limit",
+			"any",
+			"some",
+			"exists",
+			"all",
+			"union",
+			"minus",
+			"except",
+			"intersect",
+			"partition");
+	private static final Set<String> BEFORE_TABLE_KEYWORDS
+			= Set.of("from", "join");
+	private static final Set<String> FUNCTION_KEYWORDS
+			= Set.of("as", "leading", "trailing", "from", "case", "when", "then", "else", "end");
+	private static final Set<String> FUNCTION_WITH_FROM_KEYWORDS
+			= Set.of("extract", "trim");
+	private static final Set<String> SOFT_KEYWORDS
+			= Set.of("date", "time");
+	private static final Set<String> LITERAL_PREFIXES
+			= Set.of("n", "x", "varbyte", "bx", "bytea", "date", "time", "timestamp", "zone");
 
-	static {
+	private static final String PUNCTUATION = "=><!+-*/()',|&`";
-		KEYWORDS.add("and");
-		KEYWORDS.add("or");
-		KEYWORDS.add("not");
-		KEYWORDS.add("like");
-		KEYWORDS.add("escape");
-		KEYWORDS.add("is");
-		KEYWORDS.add("in");
-		KEYWORDS.add("between");
-		KEYWORDS.add("null");
-		KEYWORDS.add("select");
-		KEYWORDS.add("distinct");
-		KEYWORDS.add("from");
-		KEYWORDS.add("join");
-		KEYWORDS.add("inner");
-		KEYWORDS.add("outer");
-		KEYWORDS.add("left");
-		KEYWORDS.add("right");
-		KEYWORDS.add("on");
-		KEYWORDS.add("where");
-		KEYWORDS.add("having");
-		KEYWORDS.add("group");
-		KEYWORDS.add("order");
-		KEYWORDS.add("by");
-		KEYWORDS.add("desc");
-		KEYWORDS.add("asc");
-		KEYWORDS.add("limit");
-		KEYWORDS.add("any");
-		KEYWORDS.add("some");
-		KEYWORDS.add("exists");
-		KEYWORDS.add("all");
-		KEYWORDS.add("union");
-		KEYWORDS.add("minus");
-		KEYWORDS.add("except");
-		KEYWORDS.add("intersect");
-		KEYWORDS.add("partition");
 
-		BEFORE_TABLE_KEYWORDS.add("from");
+	public static final String TEMPLATE = "{@}";
-		BEFORE_TABLE_KEYWORDS.add("join");
-
-		FUNCTION_KEYWORDS.add("as");
-		FUNCTION_KEYWORDS.add("leading");
-		FUNCTION_KEYWORDS.add("trailing");
-		FUNCTION_KEYWORDS.add("from");
-		FUNCTION_KEYWORDS.add("case");
-		FUNCTION_KEYWORDS.add("when");
-		FUNCTION_KEYWORDS.add("then");
-		FUNCTION_KEYWORDS.add("else");
-		FUNCTION_KEYWORDS.add("end");
-
-		LITERAL_PREFIXES.add("n");
-		LITERAL_PREFIXES.add("x");
-		LITERAL_PREFIXES.add("varbyte");
-		LITERAL_PREFIXES.add("bx");
-		LITERAL_PREFIXES.add("bytea");
-		LITERAL_PREFIXES.add("date");
-		LITERAL_PREFIXES.add("time");
-		LITERAL_PREFIXES.add("timestamp");
-		LITERAL_PREFIXES.add("zone");
-	}
-
-	public static final String TEMPLATE = "$PlaceHolder$";
 
 	private Template() {}
 
@@ -111,39 +108,56 @@ public final class Template {
 		return fragment;
 	}
 
+	/**
+	 * Takes the SQL fragment provided in the mapping attribute and interpolates the default
+	 * {@linkplain #TEMPLATE placeholder value}, which is {@value #TEMPLATE}, using it to
+	 * qualify every unqualified column name.
+	 * <p>
+	 * Handles subselects, quoted identifiers, quoted strings, expressions, SQL functions,
+	 * named parameters, literals.
+	 *
+	 * @param sql The SQL string into which to interpolate the placeholder value
+	 * @param dialect The dialect to apply
+	 * @return The rendered SQL fragment
+	 */
 	public static String renderWhereStringTemplate(
-			String sqlWhereString,
+			String sql,
 			Dialect dialect,
-			TypeConfiguration typeConfiguration,
+			TypeConfiguration typeConfiguration) {
-			SqmFunctionRegistry functionRegistry) {
+		return renderWhereStringTemplate( sql, TEMPLATE, dialect, typeConfiguration );
-		return renderWhereStringTemplate( sqlWhereString, TEMPLATE, dialect, typeConfiguration, functionRegistry );
 	}
 
 	/**
-	 * Takes the where condition provided in the mapping attribute and interpolates the alias.
+	 * Takes the SQL fragment provided in the mapping attribute and interpolates the given
-	 * Handles sub-selects, quoted identifiers, quoted strings, expressions, SQL functions,
+	 * alias, using it to qualify every unqualified column name.
-	 * named parameters.
+	 * <p>
+	 * Handles subselects, quoted identifiers, quoted strings, expressions, SQL functions,
+	 * named parameters, literals.
 	 *
-	 * @param sqlWhereString The string into which to interpolate the placeholder value
+	 * @param sql The SQL string into which to interpolate the alias value
-	 * @param placeholder The value to be interpolated into the sqlWhereString
+	 * @param alias The alias to be interpolated into the SQL
 	 * @param dialect The dialect to apply
-	 * @param functionRegistry The registry of all sql functions
+	 * @return The rendered SQL fragment
-	 * @return The rendered sql fragment
 	 */
 	public static String renderWhereStringTemplate(
-			String sqlWhereString,
+			String sql,
-			String placeholder,
+			String alias,
 			Dialect dialect,
-			TypeConfiguration typeConfiguration,
+			TypeConfiguration typeConfiguration) {
-			SqmFunctionRegistry functionRegistry) {
 
-		// IMPL NOTE : The basic process here is to tokenize the incoming string and to iterate over each token
+		// IMPL NOTE: The basic process here is to tokenize the incoming string and to iterate over each token
 		//		in turn. As we process each token, we set a series of flags used to indicate the type of context in
 		// 		which the tokens occur. Depending on the state of those flags we decide whether we need to qualify
 		//		identifier references.
 
+		// WARNING TO MAINTAINERS: This is a simple scanner-based state machine. Please don't attempt to turn it into
+		//      a parser for SQL, no matter how "special" your case is. What I mean by this is: don't write code which
+		//      attempts to recognize the grammar of SQL, not even little bits of SQL. Previous "enhancements" to this
+		//      function did not respect this concept, and resulted in code which was fragile and unmaintainable. If
+		//      lookahead is truly necessary, use the lookahead() function provided below.
+
 		final String symbols = PUNCTUATION + WHITESPACE + dialect.openQuote() + dialect.closeQuote();
-		final StringTokenizer tokens = new StringTokenizer( sqlWhereString, symbols, true );
+		final StringTokenizer tokens = new StringTokenizer( sql, symbols, true );
 		final StringBuilder result = new StringBuilder();
 
 		boolean quoted = false;
@@ -151,6 +165,7 @@ public final class Template {
 		boolean beforeTable = false;
 		boolean inFromClause = false;
 		boolean afterFromTable = false;
+		boolean inExtractOrTrim = false;
 
 		boolean hasMore = tokens.hasMoreTokens();
 		String nextToken = hasMore ? tokens.nextToken() : null;
@@ -191,7 +206,7 @@ public final class Template {
 					isOpenQuote = false;
 				}
 				if ( isOpenQuote ) {
-					result.append( placeholder ).append( '.' );
+					result.append( alias ).append( '.' );
 				}
 			}
 
@@ -215,27 +230,23 @@ public final class Template {
 			else if ( isNamedParameter(token) ) {
 				result.append(token);
 			}
-			else if ( isExtractFunction( lcToken, nextToken ) ) {
+			else if ( FUNCTION_WITH_FROM_KEYWORDS.contains(lcToken) && "(".equals( nextToken ) ) {
-				// Special processing for ANSI SQL EXTRACT function
+				result.append(token);
-				handleExtractFunction( placeholder, dialect, typeConfiguration, functionRegistry, tokens, result );
+				inExtractOrTrim = true;
-				hasMore = tokens.hasMoreTokens();
-				nextToken = hasMore ? tokens.nextToken() : null;
-			}
-			else if ( isTrimFunction( lcToken, nextToken ) ) {
-				// Special processing for ANSI SQL TRIM function
-				handleTrimFunction( placeholder, dialect, typeConfiguration, functionRegistry, tokens, result );
-				hasMore = tokens.hasMoreTokens();
-				nextToken = hasMore ? tokens.nextToken() : null;
 			}
 			else if ( isIdentifier(token)
-					&& !isFunctionOrKeyword( lcToken, nextToken, dialect, typeConfiguration, functionRegistry )
+					&& !isFunctionOrKeyword( lcToken, nextToken, dialect, typeConfiguration )
-					&& !isLiteral( lcToken, nextToken, sqlWhereString, symbols, tokens ) ) {
+					&& !isLiteral( lcToken, nextToken, sql, symbols, tokens ) ) {
-				result.append(placeholder)
+				result.append(alias)
 						.append('.')
 						.append( dialect.quote(token) );
 			}
 			else {
-				if ( BEFORE_TABLE_KEYWORDS.contains(lcToken) ) {
+				if ( ")".equals( lcToken) ) {
+					inExtractOrTrim = false;
+				}
+				else if ( !inExtractOrTrim
+						&& BEFORE_TABLE_KEYWORDS.contains(lcToken) ) {
 					beforeTable = true;
 					inFromClause = true;
 				}
@@ -259,14 +270,6 @@ public final class Template {
 		return result.toString();
 	}
 
-	private static boolean isTrimFunction(String lcToken, String nextToken) {
-		return "trim".equals(lcToken) && "(".equals(nextToken);
-	}
-
-	private static boolean isExtractFunction(String lcToken, String nextToken) {
-		return "extract".equals(lcToken) && "(".equals(nextToken);
-	}
-
 	private static boolean isLiteral(
 			String lcToken, String next,
 			String sqlWhereString, String symbols, StringTokenizer tokens) {
@@ -281,159 +284,58 @@ public final class Template {
 			else {
 				// we need to look ahead in the token stream
 				// to find the first non-blank token
-				final StringTokenizer lookahead =
+				return lookPastBlankTokens( sqlWhereString, symbols, tokens, 1,
-						new StringTokenizer( sqlWhereString, symbols, true );
+						(String nextToken)
-				while ( lookahead.countTokens() > tokens.countTokens()+1 ) {
+								-> "'".equals(nextToken)
-					lookahead.nextToken();
+								|| lcToken.equals("time") && "with".equals(nextToken)
+								|| lcToken.equals("timestamp") && "with".equals(nextToken)
+								|| lcToken.equals("time") && "zone".equals(nextToken) );
 			}
+		}
+		else {
+			return false;
+		}
+	}
+
+	private static boolean lookPastBlankTokens(
+			String sqlWhereString, String symbols, StringTokenizer tokens,
+			@SuppressWarnings("SameParameterValue") int skip,
+			Function<String, Boolean> check) {
+		final StringTokenizer lookahead = lookahead( sqlWhereString, symbols, tokens, skip );
 		if ( lookahead.hasMoreTokens() ) {
 			String nextToken;
 			do {
 				nextToken = lookahead.nextToken().toLowerCase(Locale.ROOT);
 			}
 			while ( nextToken.isBlank() && lookahead.hasMoreTokens() );
-					return "'".equals( nextToken )
+			return check.apply( nextToken );
-						|| lcToken.equals( "time" ) && "with".equals( nextToken )
-						|| lcToken.equals( "timestamp" ) && "with".equals( nextToken )
-						|| lcToken.equals( "time" ) && "zone".equals( nextToken );
-				}
-				else {
-					return false;
-				}
-			}
 		}
 		else {
 			return false;
 		}
 	}
 
-	private static void handleTrimFunction(
+	/**
-			String placeholder, Dialect dialect,
+	 * Clone the given token stream, returning a token stream which begins
-			TypeConfiguration typeConfiguration,
+	 * from the next token.
-			SqmFunctionRegistry functionRegistry,
+	 *
-			StringTokenizer tokens,
+	 * @param sql the full SQL we are scanning
-			StringBuilder result) {
+	 * @param symbols the delimiter symbols
-		final List<String> operands = new ArrayList<>();
+	 * @param tokens the current token stream
-		final StringBuilder builder = new StringBuilder();
+	 * @param skip the number of tokens to skip
-
+	 * @return a cloned token stream
-		boolean hasMoreOperands = true;
+	 */
-		String operandToken = tokens.nextToken();
+	private static StringTokenizer lookahead(String sql, String symbols, StringTokenizer tokens, int skip) {
-		switch ( operandToken.toLowerCase( Locale.ROOT ) ) {
+		final StringTokenizer lookahead =
-			case "leading":
+				new StringTokenizer( sql, symbols, true );
-			case "trailing":
+		while ( lookahead.countTokens() > tokens.countTokens() + skip ) {
-			case "both":
+			lookahead.nextToken();
-				operands.add( operandToken );
-				if ( hasMoreOperands = tokens.hasMoreTokens() ) {
-					operandToken = tokens.nextToken();
 		}
-				break;
+		return lookahead;
-		}
-		boolean quotedOperand = false;
-		int parenthesis = 0;
-		while ( hasMoreOperands ) {
-			final boolean isQuote = "'".equals( operandToken );
-			if ( isQuote ) {
-				quotedOperand = !quotedOperand;
-				if ( !quotedOperand ) {
-					operands.add( builder.append( '\'' ).toString() );
-					builder.setLength( 0 );
-				}
-				else {
-					builder.append( '\'' );
-				}
-			}
-			else if ( quotedOperand ) {
-				builder.append( operandToken );
-			}
-			else if ( parenthesis != 0 ) {
-				builder.append( operandToken );
-				switch ( operandToken ) {
-					case "(":
-						parenthesis++;
-						break;
-					case ")":
-						parenthesis--;
-						break;
-				}
-			}
-			else {
-				builder.append( operandToken );
-				switch ( operandToken.toLowerCase( Locale.ROOT ) ) {
-					case "(":
-						parenthesis++;
-						break;
-					case ")":
-						parenthesis--;
-						break;
-					case "from":
-						if ( !builder.isEmpty() ) {
-							operands.add( builder.substring( 0, builder.length() - 4 ) );
-							builder.setLength( 0 );
-							operands.add( operandToken );
-						}
-						break;
-				}
-			}
-			operandToken = tokens.nextToken();
-			hasMoreOperands = tokens.hasMoreTokens()
-					&& ( parenthesis != 0 || ! ")".equals( operandToken ) );
-		}
-		if ( !builder.isEmpty() ) {
-			operands.add( builder.toString() );
 	}
 
-		final TrimOperands trimOperands = new TrimOperands( operands );
+	public static List<String> collectColumnNames(String sql, Dialect dialect, TypeConfiguration typeConfiguration) {
-		result.append( "trim(" );
+		return collectColumnNames( renderWhereStringTemplate( sql, dialect, typeConfiguration ) );
-		if ( trimOperands.trimSpec != null ) {
-			result.append( trimOperands.trimSpec ).append( ' ' );
-		}
-		if ( trimOperands.trimChar != null ) {
-			if ( trimOperands.trimChar.startsWith( "'" ) && trimOperands.trimChar.endsWith( "'" ) ) {
-				result.append( trimOperands.trimChar );
-			}
-			else {
-				result.append(
-						renderWhereStringTemplate( trimOperands.trimSpec, placeholder, dialect, typeConfiguration, functionRegistry )
-				);
-			}
-			result.append( ' ' );
-		}
-		if ( trimOperands.from != null ) {
-			result.append( trimOperands.from ).append( ' ' );
-		}
-		else if ( trimOperands.trimSpec != null || trimOperands.trimChar != null ) {
-			// I think ANSI SQL says that the 'from' is not optional if either trim-spec or trim-char is specified
-			result.append( "from " );
-		}
-
-		result.append( renderWhereStringTemplate( trimOperands.trimSource, placeholder, dialect, typeConfiguration, functionRegistry ) )
-				.append( ')' );
-	}
-
-	private static void handleExtractFunction(
-			String placeholder,
-			Dialect dialect,
-			TypeConfiguration typeConfiguration,
-			SqmFunctionRegistry functionRegistry,
-			StringTokenizer tokens,
-			StringBuilder result) {
-		final String field = extractUntil( tokens, "from" );
-		final String source = renderWhereStringTemplate(
-				extractUntil( tokens, ")" ),
-				placeholder,
-				dialect,
-				typeConfiguration,
-				functionRegistry
-		);
-		result.append( "extract(" ).append( field ).append( " from " ).append( source ).append( ')' );
-	}
-
-	public static List<String> collectColumnNames(
-			String sql,
-			Dialect dialect,
-			TypeConfiguration typeConfiguration,
-			SqmFunctionRegistry functionRegistry) {
-		return collectColumnNames( renderWhereStringTemplate( sql, dialect, typeConfiguration, functionRegistry ) );
 	}
 
 	public static List<String> collectColumnNames(String template) {
@@ -461,302 +363,6 @@ public final class Template {
 		return names;
 	}
 
-//	/**
-//	 * Takes the where condition provided in the mapping attribute and interpolates the alias.
-//	 * Handles sub-selects, quoted identifiers, quoted strings, expressions, SQL functions,
-//	 * named parameters.
-//	 *
-//	 * @param sqlWhereString The string into which to interpolate the placeholder value
-//	 * @param placeholder The value to be interpolated into the sqlWhereString
-//	 * @param dialect The dialect to apply
-//	 * @param functionRegistry The registry of all sql functions
-//	 *
-//	 * @return The rendered sql fragment
-//	 */
-//	public static String renderWhereStringTemplate(
-//			String sqlWhereString,
-//			String placeholder,
-//			Dialect dialect,
-//			SQLFunctionRegistry functionRegistry) {
-//
-//		// IMPL NOTE : The basic process here is to tokenize the incoming string and to iterate over each token
-//		//		in turn.  As we process each token, we set a series of flags used to indicate the type of context in
-//		// 		which the tokens occur.  Depending on the state of those flags we decide whether we need to qualify
-//		//		identifier references.
-//
-//		final String dialectOpenQuote = Character.toString( dialect.openQuote() );
-//		final String dialectCloseQuote = Character.toString( dialect.closeQuote() );
-//
-//		String symbols = new StringBuilder()
-//				.append( "=><!+-*/()',|&`" )
-//				.append( StringHelper.WHITESPACE )
-//				.append( dialect.openQuote() )
-//				.append( dialect.closeQuote() )
-//				.toString();
-//		StringTokenizer tokens = new StringTokenizer( sqlWhereString, symbols, true );
-//		ProcessingState state = new ProcessingState();
-//
-//		StringBuilder quotedBuffer = new StringBuilder();
-//		StringBuilder result = new StringBuilder();
-//
-//		boolean hasMore = tokens.hasMoreTokens();
-//		String nextToken = hasMore ? tokens.nextToken() : null;
-//		while ( hasMore ) {
-//			String token = nextToken;
-//			String lcToken = token.toLowerCase(Locale.ROOT);
-//			hasMore = tokens.hasMoreTokens();
-//			nextToken = hasMore ? tokens.nextToken() : null;
-//
-//			// First, determine quoting which might be based on either:
-//			// 		1) back-tick
-//			// 		2) single quote (ANSI SQL standard)
-//			// 		3) or dialect defined quote character(s)
-//			QuotingCharacterDisposition quotingCharacterDisposition = QuotingCharacterDisposition.NONE;
-//			if ( "`".equals( token ) ) {
-//				state.quoted = !state.quoted;
-//				quotingCharacterDisposition = state.quoted
-//						? QuotingCharacterDisposition.OPEN
-//						: QuotingCharacterDisposition.CLOSE;
-//				// replace token with the appropriate dialect quoting char
-//				token = lcToken = ( quotingCharacterDisposition == QuotingCharacterDisposition.OPEN )
-//						? dialectOpenQuote
-//						: dialectCloseQuote;
-//			}
-//			else if ( "'".equals( token ) ) {
-//				state.quoted = !state.quoted;
-//				quotingCharacterDisposition = state.quoted
-//						? QuotingCharacterDisposition.OPEN
-//						: QuotingCharacterDisposition.CLOSE;
-//			}
-//			else if ( !state.quoted && dialectOpenQuote.equals( token ) ) {
-//				state.quoted = true;
-//				quotingCharacterDisposition = QuotingCharacterDisposition.OPEN;
-//			}
-//			else if ( state.quoted && dialectCloseQuote.equals( token ) ) {
-//				state.quoted = false;
-//				quotingCharacterDisposition = QuotingCharacterDisposition.CLOSE;
-//			}
-//
-//			if ( state.quoted ) {
-//				quotedBuffer.append( token );
-//				continue;
-//			}
-//
-//			// if we were previously processing quoted state and just encountered the close quote, then handle that
-//			// quoted text
-//			if ( quotingCharacterDisposition == QuotingCharacterDisposition.CLOSE ) {
-//				token = quotedBuffer.toString();
-//				quotedBuffer.setLength( 0 );
-//				result.append( placeholder ).append( '.' )
-//						.append( dialectOpenQuote ).append( token ).append( dialectCloseQuote );
-//				continue;
-//			}
-//
-//			// Special processing for ANSI SQL EXTRACT function
-//			if ( "extract".equals( lcToken ) && "(".equals( nextToken ) ) {
-//				final String field = extractUntil( tokens, "from" );
-//				final String source = renderWhereStringTemplate(
-//						extractUntil( tokens, ")" ),
-//						placeholder,
-//						dialect,
-//						functionRegistry
-//				);
-//				result.append( "extract(" ).append( field ).append( " from " ).append( source ).append( ')' );
-//
-//				hasMore = tokens.hasMoreTokens();
-//				nextToken = hasMore ? tokens.nextToken() : null;
-//
-//				continue;
-//			}
-//
-//			// Special processing for ANSI SQL TRIM function
-//			if ( "trim".equals( lcToken ) && "(".equals( nextToken ) ) {
-//				List<String> operands = new ArrayList<String>();
-//				StringBuilder builder = new StringBuilder();
-//
-//				boolean hasMoreOperands = true;
-//				String operandToken = tokens.nextToken();
-//				boolean quoted = false;
-//				while ( hasMoreOperands ) {
-//					final boolean isQuote = "'".equals( operandToken );
-//					if ( isQuote ) {
-//						quoted = !quoted;
-//						if ( !quoted ) {
-//							operands.add( builder.append( '\'' ).toString() );
-//							builder.setLength( 0 );
-//						}
-//						else {
-//							builder.append( '\'' );
-//						}
-//					}
-//					else if ( quoted ) {
-//						builder.append( operandToken );
-//					}
-//					else if ( operandToken.length() == 1 && Character.isWhitespace( operandToken.charAt( 0 ) ) ) {
-//						// do nothing
-//					}
-//					else {
-//						operands.add( operandToken );
-//					}
-//					operandToken = tokens.nextToken();
-//					hasMoreOperands = tokens.hasMoreTokens() && ! ")".equals( operandToken );
-//				}
-//
-//				TrimOperands trimOperands = new TrimOperands( operands );
-//				result.append( "trim(" );
-//				if ( trimOperands.trimSpec != null ) {
-//					result.append( trimOperands.trimSpec ).append( ' ' );
-//				}
-//				if ( trimOperands.trimChar != null ) {
-//					if ( trimOperands.trimChar.startsWith( "'" ) && trimOperands.trimChar.endsWith( "'" ) ) {
-//						result.append( trimOperands.trimChar );
-//					}
-//					else {
-//						result.append(
-//								renderWhereStringTemplate( trimOperands.trimSpec, placeholder, dialect, functionRegistry )
-//						);
-//					}
-//					result.append( ' ' );
-//				}
-//				if ( trimOperands.from != null ) {
-//					result.append( trimOperands.from ).append( ' ' );
-//				}
-//				else if ( trimOperands.trimSpec != null || trimOperands.trimChar != null ) {
-//					// I think ANSI SQL says that the 'from' is not optional if either trim-spec or trim-char are specified
-//					result.append( "from " );
-//				}
-//
-//				result.append( renderWhereStringTemplate( trimOperands.trimSource, placeholder, dialect, functionRegistry ) )
-//						.append( ')' );
-//
-//				hasMore = tokens.hasMoreTokens();
-//				nextToken = hasMore ? tokens.nextToken() : null;
-//
-//				continue;
-//			}
-//
-//
-//			// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-//
-//			if ( Character.isWhitespace( token.charAt( 0 ) ) ) {
-//				result.append( token );
-//			}
-//			else if ( state.beforeTable ) {
-//				result.append( token );
-//				state.beforeTable = false;
-//				state.afterFromTable = true;
-//			}
-//			else if ( state.afterFromTable ) {
-//				if ( !"as".equals(lcToken) ) {
-//					state.afterFromTable = false;
-//				}
-//				result.append(token);
-//			}
-//			else if ( isNamedParameter(token) ) {
-//				result.append(token);
-//			}
-//			else if ( isIdentifier(token, dialect)
-//					&& !isFunctionOrKeyword(lcToken, nextToken, dialect , functionRegistry) ) {
-//				result.append(placeholder)
-//						.append('.')
-//						.append( dialect.quote(token) );
-//			}
-//			else {
-//				if ( BEFORE_TABLE_KEYWORDS.contains(lcToken) ) {
-//					state.beforeTable = true;
-//					state.inFromClause = true;
-//				}
-//				else if ( state.inFromClause && ",".equals(lcToken) ) {
-//					state.beforeTable = true;
-//				}
-//				result.append(token);
-//			}
-//
-//			//Yuck:
-//			if ( state.inFromClause
-//					&& KEYWORDS.contains( lcToken ) //"as" is not in KEYWORDS
-//					&& !BEFORE_TABLE_KEYWORDS.contains( lcToken ) ) {
-//				state.inFromClause = false;
-//			}
-//		}
-//
-//		return result.toString();
-//	}
-//
-//	private static class ProcessingState {
-//		boolean quoted = false;
-//		boolean quotedIdentifier = false;
-//		boolean beforeTable = false;
-//		boolean inFromClause = false;
-//		boolean afterFromTable = false;
-//	}
-//
-//	private static enum QuotingCharacterDisposition { NONE, OPEN, CLOSE }
-
-	private static class TrimOperands {
-		private final String trimSpec;
-		private final String trimChar;
-		private final String from;
-		private final String trimSource;
-
-		private TrimOperands(List<String> operands) {
-			final int size = operands.size();
-			if ( size == 1 ) {
-				trimSpec = null;
-				trimChar = null;
-				from = null;
-				trimSource = operands.get(0);
-			}
-			else if ( size == 4 ) {
-				trimSpec = operands.get(0);
-				trimChar = operands.get(1);
-				from = operands.get(2);
-				trimSource = operands.get(3);
-			}
-			else {
-				if ( size < 1 || size > 4 ) {
-					throw new HibernateException( "Unexpected number of trim function operands : " + size );
-				}
-
-				// trim-source will always be the last operand
-				trimSource = operands.get( size - 1 );
-
-				// ANSI SQL says that more than one operand means that the FROM is required
-				if ( ! "from".equals( operands.get( size - 2 ) ) ) {
-					throw new HibernateException( "Expecting FROM, found : " + operands.get( size - 2 ) );
-				}
-				from = operands.get( size - 2 );
-
-				// trim-spec, if there is one will always be the first operand
-				if ( "leading".equalsIgnoreCase( operands.get(0) )
-						|| "trailing".equalsIgnoreCase( operands.get(0) )
-						|| "both".equalsIgnoreCase( operands.get(0) ) ) {
-					trimSpec = operands.get(0);
-					trimChar = null;
-				}
-				else {
-					trimSpec = null;
-					if ( size - 2 == 0 ) {
-						trimChar = null;
-					}
-					else {
-						trimChar = operands.get( 0 );
-					}
-				}
-			}
-		}
-	}
-
-	private static String extractUntil(StringTokenizer tokens, String delimiter) {
-		final StringBuilder valueBuilder = new StringBuilder();
-		String token = tokens.nextToken();
-		while ( ! delimiter.equalsIgnoreCase( token ) ) {
-			valueBuilder.append( token );
-			token = tokens.nextToken();
-		}
-		return valueBuilder.toString().trim();
-	}
-
 	private static boolean isNamedParameter(String token) {
 		return token.startsWith( ":" );
 	}
@@ -765,12 +371,11 @@ public final class Template {
 			String lcToken,
 			String nextToken,
 			Dialect dialect,
-			TypeConfiguration typeConfiguration,
+			TypeConfiguration typeConfiguration) {
-			SqmFunctionRegistry functionRegistry) {
 		if ( "(".equals( nextToken ) ) {
 			return true;
 		}
-		else if ( "date".equals( lcToken ) || "time".equals( lcToken ) ) {
+		else if ( SOFT_KEYWORDS.contains( lcToken ) ) {
 			// these can be column names on some databases
 			// TODO: treat 'current date' as a function
 			return false;
@@ -778,7 +383,6 @@ public final class Template {
 		else {
 			return KEYWORDS.contains( lcToken )
 				|| isType( lcToken, typeConfiguration )
-				|| isFunction( lcToken, nextToken, functionRegistry )
 				|| dialect.getKeywords().contains( lcToken )
 				|| FUNCTION_KEYWORDS.contains( lcToken );
 		}
@@ -788,17 +392,6 @@ public final class Template {
 		return typeConfiguration.getDdlTypeRegistry().isTypeNameRegistered( lcToken );
 	}
 
-	private static boolean isFunction(String lcToken, String nextToken, SqmFunctionRegistry functionRegistry) {
-		// checking for "(" is currently redundant because it is checked before getting here;
-		// doing the check anyhow, in case that earlier check goes away;
-		if ( "(".equals( nextToken ) ) {
-			return true;
-		}
-
-		final SqmFunctionDescriptor function = functionRegistry.findFunctionDescriptor( lcToken );
-		return function != null;
-	}
-
 	private static boolean isIdentifier(String token) {
 		if ( isBoolean( token ) ) {
 			return false;

hibernate-core/src/test/java/org/hibernate/orm/test/sql/TemplateTest.java

@@ -7,6 +7,7 @@
 package org.hibernate.orm.test.sql;
 
 
+import org.hibernate.dialect.Dialect;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.sql.Template;
 
@@ -26,6 +27,33 @@ public class TemplateTest {
 	@JiraKey("HHH-18256")
 	public void templateLiterals(SessionFactoryScope scope) {
 		SessionFactoryImplementor factory = scope.getSessionFactory();
+		Dialect dialect = factory.getJdbcServices().getDialect();
+		assertWhereStringTemplate( "'Knock, knock! Who''s there?'",
+				"'Knock, knock! Who''s there?'", factory );
+		assertWhereStringTemplate( "1e-5 + 2 * 3.0",
+				"1e-5 + 2 * 3.0", factory );
+		assertWhereStringTemplate( "hello",
+				"{@}.hello", factory );
+		assertWhereStringTemplate( "`hello`",
+				"{@}." + dialect.quote("`hello`"), factory );
+		assertWhereStringTemplate( dialect.openQuote() + "hello" + dialect.closeQuote(),
+				"{@}." + dialect.quote("`hello`"), factory );
+		assertWhereStringTemplate( "hello.world",
+				"hello.world", factory );
+		assertWhereStringTemplate( "'hello there' || ' ' || 'world'",
+				"'hello there' || ' ' || 'world'", factory );
+		assertWhereStringTemplate( "hello + world",
+				"{@}.hello + {@}.world", factory );
+		assertWhereStringTemplate( "upper(hello) || lower(world)",
+				"upper({@}.hello) || lower({@}.world)", factory );
+		assertWhereStringTemplate( "extract(hour from time)",
+				"extract(hour from {@}.time)", factory );
+		assertWhereStringTemplate( "extract(day from date)",
+				"extract(day from {@}.date)", factory );
+		assertWhereStringTemplate( "trim(leading '_' from string)",
+				"trim(leading '_' from {@}.string)", factory );
+		assertWhereStringTemplate( "left(hello,4) || right(world,5)",
+				"left({@}.hello,4) || right({@}.world,5)", factory );
 		assertWhereStringTemplate( "N'a'", factory );
 		assertWhereStringTemplate( "X'a'", factory );
 		assertWhereStringTemplate( "BX'a'", factory);
@@ -37,35 +65,38 @@ public class TemplateTest {
 		assertWhereStringTemplate( "timestamp 'a'", factory );
 		assertWhereStringTemplate( "timestamp with time zone 'a'", factory );
 		assertWhereStringTemplate( "time with time zone 'a'", factory );
-		assertWhereStringTemplate( "date", "$PlaceHolder$.date", factory );
+		assertWhereStringTemplate( "date", "{@}.date", factory );
-		assertWhereStringTemplate( "time", "$PlaceHolder$.time", factory );
+		assertWhereStringTemplate( "time", "{@}.time", factory );
-		assertWhereStringTemplate( "zone", "$PlaceHolder$.zone", factory );
+		assertWhereStringTemplate( "zone", "{@}.zone", factory );
 		assertWhereStringTemplate("select date from thetable",
-				"select $PlaceHolder$.date from thetable", factory );
+				"select {@}.date from thetable", factory );
 		assertWhereStringTemplate("select date '2000-12-1' from thetable",
 				"select date '2000-12-1' from thetable", factory );
 		assertWhereStringTemplate("where date between date '2000-12-1' and date '2002-12-2'",
-				"where $PlaceHolder$.date between date '2000-12-1' and date '2002-12-2'", factory );
+				"where {@}.date between date '2000-12-1' and date '2002-12-2'", factory );
+		assertWhereStringTemplate("where foo>10 and bar is not null",
+				"where {@}.foo>10 and {@}.bar is not null", factory );
+		assertWhereStringTemplate("select t.foo, o.bar from table as t left join other as o on t.id = o.id where t.foo>10 and o.bar is not null order by o.bar",
+				"select t.foo, o.bar from table as t left join other as o on t.id = o.id where t.foo>10 and o.bar is not null order by o.bar", factory );
+
 	}
 
 	private static void assertWhereStringTemplate(String sql, SessionFactoryImplementor sf) {
-		final String template = Template.renderWhereStringTemplate(
+		assertEquals( sql,
+				Template.renderWhereStringTemplate(
 						sql,
 						sf.getJdbcServices().getDialect(),
-				sf.getTypeConfiguration(),
+						sf.getTypeConfiguration()
-				sf.getQueryEngine().getSqmFunctionRegistry()
+				));
-		);
-		assertEquals( sql, template );
 	}
 
-	private static void assertWhereStringTemplate(String sql, String result, SessionFactoryImplementor sf) {
+	private static void assertWhereStringTemplate(String sql, String result, SessionFactoryImplementor factory) {
-		final String template = Template.renderWhereStringTemplate(
+		assertEquals( result,
+				Template.renderWhereStringTemplate(
 						sql,
-				sf.getJdbcServices().getDialect(),
+						factory.getJdbcServices().getDialect(),
-				sf.getTypeConfiguration(),
+						factory.getTypeConfiguration()
-				sf.getQueryEngine().getSqmFunctionRegistry()
+				) );
-		);
-		assertEquals( result, template );
 	}
 
 }