squash! HHH-16516 test for automatic quoting

Author: gavinking

hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/HSQLLegacyDialect.java

@@ -927,6 +927,7 @@ public String translateExtractField(TemporalUnit unit) {
 	public IdentifierHelper buildIdentifierHelper(IdentifierHelperBuilder builder, DatabaseMetaData dbMetaData)
 			throws SQLException {
 		builder.setAutoQuoteInitialUnderscore(true);
+		builder.setAutoQuoteDollar(true);
 		return super.buildIdentifierHelper(builder, dbMetaData);
 	}
 

hibernate-core/src/main/java/org/hibernate/boot/model/naming/Identifier.java

@@ -68,19 +68,19 @@ public static Identifier toIdentifier(String text, boolean quote) {
 	/**
 	 * Means to generate an {@link Identifier} instance from its simple text form.
 	 * <p>
-	 * If passed text is {@code null}, {@code null} is returned.
+	 * If passed {@code text} is {@code null}, {@code null} is returned.
 	 * <p>
-	 * If passed text is surrounded in quote markers, the generated Identifier
-	 * is considered quoted.  Quote markers include back-ticks (`),
-	 * double-quotes (") and brackets ([ and ]).
+	 * If passed {@code text} is surrounded in quote markers, the returned Identifier
+	 * is considered quoted. Quote markers include back-ticks (`), double-quotes ("),
+	 * and brackets ([ and ]).
 	 *
 	 * @param text The text form
 	 * @param quote Whether to quote unquoted text forms
-	 * @param quoteOnNonIdentifierChar Controls whether to treat the result as quoted if text contains characters that are invalid for identifiers
+	 * @param autoquote Whether to quote the result if it contains special characters
 	 *
 	 * @return The identifier form, or {@code null} if text was {@code null}
 	 */
-	public static Identifier toIdentifier(String text, boolean quote, boolean quoteOnNonIdentifierChar) {
+	public static Identifier toIdentifier(String text, boolean quote, boolean autoquote) {
 		if ( isBlank( text ) ) {
 			return null;
 		}
@@ -103,25 +103,40 @@ public static Identifier toIdentifier(String text, boolean quote, boolean quoteO
 			end--;
 			quote = true;
 		}
-		else if ( quoteOnNonIdentifierChar && !quote ) {
-			// Check the letters to determine if we must quote the text
-			char c = text.charAt( start );
-			if ( !isLetter( c ) && c != '_' ) {
-				// SQL identifiers must begin with a letter or underscore
-				quote = true;
-			}
-			else {
-				for ( int i = start + 1; i < end; i++ ) {
-					c = text.charAt( i );
-					// every database except HSQLDB also allows a $ here
-					if ( !isLetterOrDigit( c ) && c != '_' ) {
-						quote = true;
-						break;
-					}
+		else if ( autoquote && !quote ) {
+			quote = autoquote( text, start, end );
+		}
+		return new Identifier( text.substring( start, end ), quote );
+	}
+
+	private static boolean autoquote(String text, int start, int end) {
+		// Check the letters to determine if we must quote the text
+		if ( !isLegalFirstChar( text.charAt( start ) ) ) {
+			// SQL identifiers must begin with a letter or underscore
+			return true;
+		}
+		else {
+			for ( int i = start + 1; i < end; i++ ) {
+				if ( !isLegalChar( text.charAt( i ) ) ) {
+					return true;
 				}
 			}
 		}
-		return new Identifier( text.substring( start, end ), quote );
+		return false;
+	}
+
+	private static boolean isLegalChar(char current) {
+		return isLetterOrDigit( current )
+			// every database also allows _ here
+			|| current == '_'
+			// every database except HSQLDB also allows $ here
+			|| current == '$';
+	}
+
+	private static boolean isLegalFirstChar(char first) {
+		return isLetter( first )
+			// many databases also allow _ here
+			|| first == '_';
 	}
 
 	/**
@@ -143,21 +158,22 @@ public static boolean isQuoted(String name) {
 
 	public static boolean isQuoted(String name, int start, int end) {
 		if ( start + 2 < end ) {
-			switch ( name.charAt( start ) ) {
-				case '`':
-					return name.charAt( end - 1 ) == '`';
-				case '[':
-					return name.charAt( end - 1 ) == ']';
-				case '"':
-					return name.charAt( end - 1 ) == '"';
-			}
+			final char first = name.charAt( start );
+			final char last = name.charAt( end - 1 );
+			return switch ( first ) {
+				case '`' -> last == '`';
+				case '[' -> last == ']';
+				case '"' -> last == '"';
+				default -> false;
+			};
+		}
+		else {
+			return false;
 		}
-		return false;
 	}
 
 	public static String unQuote(String name) {
 		assert isQuoted( name );
-
 		return name.substring( 1, name.length() - 1 );
 	}
 
@@ -240,7 +256,7 @@ public String toString() {
 	@Override
 	public boolean equals(Object object) {
 		return object instanceof Identifier that
-		   && getCanonicalName().equals( that.getCanonicalName() );
+			&& getCanonicalName().equals( that.getCanonicalName() );
 	}
 
 	public boolean matches(String name) {
@@ -267,7 +283,7 @@ public static Identifier quote(Identifier identifier) {
 	}
 
 	@Override
-	public int compareTo(Identifier o) {
-		return getCanonicalName().compareTo( o.getCanonicalName() );
+	public int compareTo(Identifier identifier) {
+		return getCanonicalName().compareTo( identifier.getCanonicalName() );
 	}
 }

hibernate-core/src/main/java/org/hibernate/dialect/HSQLDialect.java

@@ -712,6 +712,7 @@ public String translateExtractField(TemporalUnit unit) {
 	public IdentifierHelper buildIdentifierHelper(IdentifierHelperBuilder builder, DatabaseMetaData dbMetaData)
 			throws SQLException {
 		builder.setAutoQuoteInitialUnderscore( true );
+		builder.setAutoQuoteDollar( true );
 		return super.buildIdentifierHelper( builder, dbMetaData );
 	}
 

hibernate-core/src/main/java/org/hibernate/engine/jdbc/env/internal/NormalizingIdentifierHelperImpl.java

@@ -27,6 +27,7 @@ public class NormalizingIdentifierHelperImpl implements IdentifierHelper {
 	private final boolean globallyQuoteIdentifiersSkipColumnDefinitions;
 	private final boolean autoQuoteKeywords;
 	private final boolean autoQuoteInitialUnderscore;
+	private final boolean autoQuoteDollar;
 	private final TreeSet<String> reservedWords;
 	private final IdentifierCaseStrategy unquotedCaseStrategy;
 	private final IdentifierCaseStrategy quotedCaseStrategy;
@@ -38,6 +39,7 @@ public NormalizingIdentifierHelperImpl(
 			boolean globallyQuoteIdentifiersSkipColumnDefinitions,
 			boolean autoQuoteKeywords,
 			boolean autoQuoteInitialUnderscore,
+			boolean autoQuoteDollar,
 			TreeSet<String> reservedWords, //careful, we intentionally omit making a defensive copy to not waste memory
 			IdentifierCaseStrategy unquotedCaseStrategy,
 			IdentifierCaseStrategy quotedCaseStrategy) {
@@ -47,6 +49,7 @@ public NormalizingIdentifierHelperImpl(
 		this.globallyQuoteIdentifiersSkipColumnDefinitions = globallyQuoteIdentifiersSkipColumnDefinitions;
 		this.autoQuoteKeywords = autoQuoteKeywords;
 		this.autoQuoteInitialUnderscore = autoQuoteInitialUnderscore;
+		this.autoQuoteDollar = autoQuoteDollar;
 		this.reservedWords = reservedWords;
 		this.unquotedCaseStrategy = unquotedCaseStrategy == null ? IdentifierCaseStrategy.UPPER : unquotedCaseStrategy;
 		this.quotedCaseStrategy = quotedCaseStrategy == null ? IdentifierCaseStrategy.MIXED : quotedCaseStrategy;
@@ -57,24 +60,22 @@ public Identifier normalizeQuoting(Identifier identifier) {
 		if ( identifier == null ) {
 			return null;
 		}
-
-		if ( identifier.isQuoted() ) {
+		else if ( identifier.isQuoted() ) {
 			return identifier;
 		}
-
-		if ( globallyQuoteIdentifiers ) {
-			return Identifier.toIdentifier( identifier.getText(), true );
-		}
-
-		if ( autoQuoteKeywords && isReservedWord( identifier.getText() ) ) {
+		else if ( mustQuote( identifier ) ) {
 			return Identifier.toIdentifier( identifier.getText(), true );
 		}
-
-		if ( autoQuoteInitialUnderscore && identifier.getText().startsWith("_") ) {
-			return Identifier.toIdentifier( identifier.getText(), true );
+		else {
+			return identifier;
 		}
+	}
 
-		return identifier;
+	private boolean mustQuote(Identifier identifier) {
+		return globallyQuoteIdentifiers
+			|| autoQuoteKeywords && isReservedWord( identifier.getText() )
+			|| autoQuoteInitialUnderscore && identifier.getText().startsWith( "_" )
+			|| autoQuoteDollar && identifier.getText().contains( "$" );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/engine/jdbc/env/spi/IdentifierHelperBuilder.java

@@ -40,6 +40,7 @@ public class IdentifierHelperBuilder {
 	private boolean skipGlobalQuotingForColumnDefinitions = false;
 	private boolean autoQuoteKeywords = true;
 	private boolean autoQuoteInitialUnderscore = false;
+	private boolean autoQuoteDollar = false;
 	private IdentifierCaseStrategy unquotedCaseStrategy = IdentifierCaseStrategy.UPPER;
 	private IdentifierCaseStrategy quotedCaseStrategy = IdentifierCaseStrategy.MIXED;
 
@@ -150,6 +151,10 @@ public void setAutoQuoteInitialUnderscore(boolean autoQuoteInitialUnderscore) {
 		this.autoQuoteInitialUnderscore = autoQuoteInitialUnderscore;
 	}
 
+	public void setAutoQuoteDollar(boolean autoQuoteDollar) {
+		this.autoQuoteDollar = autoQuoteDollar;
+	}
+
 	public NameQualifierSupport getNameQualifierSupport() {
 		return nameQualifierSupport;
 	}
@@ -215,6 +220,7 @@ public IdentifierHelper build() {
 				skipGlobalQuotingForColumnDefinitions,
 				autoQuoteKeywords,
 				autoQuoteInitialUnderscore,
+				autoQuoteDollar,
 				reservedWords,
 				unquotedCaseStrategy,
 				quotedCaseStrategy

hibernate-core/src/test/java/org/hibernate/orm/test/mapping/autoquote/SpecialCharactersTest.java

@@ -27,5 +27,9 @@ public class SpecialCharactersTest {
 		String nameWithHash;
 		@Column(name="NAME NAME")
 		String nameWithSpace;
+		@Column(name="NAME_NAME")
+		String nameWithUnderscore;
+		@Column(name="_NAME")
+		String nameWithInitialUnderscore;
 	}
 }