HHH-9491 - Add support to opt columnDefinitions out of globally-quoted-identifiers

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

@@ -93,6 +93,16 @@ public Identifier determineLogicalName(String explicitName, NamingStrategyHelper
 		return logicalName;
 	}
 
+	/**
+	 * Intended only for use in handling quoting requirements for {@code column-definition}
+	 * as defined by {@link javax.persistence.Column#columnDefinition()},
+	 *  {@link javax.persistence.JoinColumn#columnDefinition}, etc.  This method should not
+	 * be called in any other scenario.
+	 *
+	 * @param text The specified column definition
+	 *
+	 * @return The name with global quoting applied
+	 */
 	public String applyGlobalQuoting(String text) {
 		return database().getJdbcEnvironment().getIdentifierHelper().applyGlobalQuoting( text )
 				.render( database().getDialect() );

hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java

@@ -784,10 +784,22 @@ public interface AvailableSettings {
     String DEFAULT_ENTITY_MODE = "hibernate.default_entity_mode";
 
 	/**
-	 * Should all database identifiers be quoted.
+	 * Should all database identifiers be quoted.  A {@code true}/{@code false} option.
 	 */
 	String GLOBALLY_QUOTED_IDENTIFIERS = "hibernate.globally_quoted_identifiers";
 
+	/**
+	 * Assuming {@link #GLOBALLY_QUOTED_IDENTIFIERS}, this allows such global quoting
+	 * to skip column-definitions as defined by {@link javax.persistence.Column#columnDefinition()},
+	 * {@link javax.persistence.JoinColumn#columnDefinition}, etc.
+	 * <p/>
+	 * JPA states that column-definitions are subject to global quoting, so by default this setting
+	 * is {@code false} for JPA compliance.  Set to {@code true} to avoid column-definitions
+	 * being quoted due to global quoting (they will still be quoted if explicitly quoted in the
+	 * annotation/xml).
+	 */
+	String GLOBALLY_QUOTED_IDENTIFIERS_SKIP_COLUMN_DEFINITIONS = "hibernate.globally_quoted_identifiers_skip_column_definitions";
+
 	/**
 	 * Enable nullability checking.
 	 * Raises an exception if a property marked as not-null is null.

hibernate-core/src/main/java/org/hibernate/cfg/Ejb3JoinColumn.java

@@ -299,7 +299,7 @@ public void setJoinAnnotation(JoinColumn annJoin, String defaultName) {
 		else {
 			setImplicit( false );
 			if ( !BinderHelper.isEmptyAnnotationValue( annJoin.columnDefinition() ) ) {
-				setSqlType( annJoin.columnDefinition() );
+				setSqlType( getBuildingContext().getObjectNameNormalizer().applyGlobalQuoting( annJoin.columnDefinition() ) );
 			}
 			if ( !BinderHelper.isEmptyAnnotationValue( annJoin.name() ) ) {
 				setLogicalColumnName( annJoin.name() );

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

@@ -78,6 +78,7 @@ public JdbcEnvironmentImpl(ServiceRegistryImplementor serviceRegistry, Dialect d
 
 		final IdentifierHelperBuilder identifierHelperBuilder = IdentifierHelperBuilder.from( this );
 		identifierHelperBuilder.setGloballyQuoteIdentifiers( globalQuoting( cfgService ) );
+		identifierHelperBuilder.setSkipGlobalQuotingForColumnDefinitions( globalQuotingSkippedForColumnDefinitions( cfgService ) );
 		identifierHelperBuilder.setAutoQuoteKeywords( autoKeywordQuoting( cfgService ) );
 		identifierHelperBuilder.setNameQualifierSupport( nameQualifierSupport );
 
@@ -114,6 +115,15 @@ private static boolean globalQuoting(ConfigurationService cfgService) {
 		);
 	}
 
+	private boolean globalQuotingSkippedForColumnDefinitions(ConfigurationService cfgService) {
+		return cfgService.getSetting(
+				AvailableSettings.GLOBALLY_QUOTED_IDENTIFIERS_SKIP_COLUMN_DEFINITIONS,
+				StandardConverters.BOOLEAN,
+				// default is true for JPA compliance - DO NOT CHANGE!
+				true
+		);
+	}
+
 	private static boolean autoKeywordQuoting(ConfigurationService cfgService) {
 		return cfgService.getSetting(
 				AvailableSettings.KEYWORD_AUTO_QUOTING_ENABLED,
@@ -218,6 +228,7 @@ public JdbcEnvironmentImpl(
 
 		final IdentifierHelperBuilder identifierHelperBuilder = IdentifierHelperBuilder.from( this );
 		identifierHelperBuilder.setGloballyQuoteIdentifiers( globalQuoting( cfgService ) );
+		identifierHelperBuilder.setSkipGlobalQuotingForColumnDefinitions( globalQuotingSkippedForColumnDefinitions( cfgService ) );
 		identifierHelperBuilder.setAutoQuoteKeywords( autoKeywordQuoting( cfgService ) );
 		identifierHelperBuilder.setNameQualifierSupport( nameQualifierSupport );
 		IdentifierHelper identifierHelper = null;

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

@@ -29,6 +29,7 @@ public class NormalizingIdentifierHelperImpl implements IdentifierHelper {
 
 	private final NameQualifierSupport nameQualifierSupport;
 	private final boolean globallyQuoteIdentifiers;
+	private final boolean globallyQuoteIdentifiersSkipColumnDefinitions;
 	private final boolean autoQuoteKeywords;
 	private final Set<String> reservedWords = new TreeSet<String>( String.CASE_INSENSITIVE_ORDER );
 	private final IdentifierCaseStrategy unquotedCaseStrategy;
@@ -38,6 +39,7 @@ public NormalizingIdentifierHelperImpl(
 			JdbcEnvironment jdbcEnvironment,
 			NameQualifierSupport nameQualifierSupport,
 			boolean globallyQuoteIdentifiers,
+			boolean globallyQuoteIdentifiersSkipColumnDefinitions,
 			boolean autoQuoteKeywords,
 			Set<String> reservedWords,
 			IdentifierCaseStrategy unquotedCaseStrategy,
@@ -45,6 +47,7 @@ public NormalizingIdentifierHelperImpl(
 		this.jdbcEnvironment = jdbcEnvironment;
 		this.nameQualifierSupport = nameQualifierSupport;
 		this.globallyQuoteIdentifiers = globallyQuoteIdentifiers;
+		this.globallyQuoteIdentifiersSkipColumnDefinitions = globallyQuoteIdentifiersSkipColumnDefinitions;
 		this.autoQuoteKeywords = autoQuoteKeywords;
 		if ( reservedWords != null ) {
 			this.reservedWords.addAll( reservedWords );
@@ -90,7 +93,7 @@ public Identifier toIdentifier(String text, boolean quoted) {
 
 	@Override
 	public Identifier applyGlobalQuoting(String text) {
-		return Identifier.toIdentifier( text, globallyQuoteIdentifiers );
+		return Identifier.toIdentifier( text, globallyQuoteIdentifiers && globallyQuoteIdentifiersSkipColumnDefinitions );
 	}
 
 	@Override

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

@@ -7,6 +7,7 @@
 package org.hibernate.engine.jdbc.env.spi;
 
 import org.hibernate.boot.model.naming.Identifier;
+import org.hibernate.cfg.AvailableSettings;
 
 /**
  * Helper for handling {@link Identifier} instances.
@@ -53,13 +54,22 @@ public interface IdentifierHelper {
 	Identifier toIdentifier(String text, boolean quoted);
 
 	/**
-	 * Needed to account for certain fields ({@link javax.persistence.Column#columnDefinition()} comes to mind)
+	 * Intended only for use in handling quoting requirements for {@code column-definition}
-	 * that need to be quoted id global identifier quoting is requested, but only for spec compliance.  TBH, I can
+	 * as defined by {@link javax.persistence.Column#columnDefinition()},
-	 * not think of a argument why column-definitions should ever be *globally* quoted, but the spec is the spec.
+	 * {@link javax.persistence.JoinColumn#columnDefinition}, etc.  This method should not
+	 * be called in any other scenario.
+	 * <p/>
+	 * This method is needed to account for that fact that the JPA spec says that {@code column-definition}
+	 * should be quoted of global-identifier-quoting is requested.  Again, this is needed for spec
+	 * compliance.  TBH, I can not think of a argument why column-definitions should ever be *globally* quoted,
+	 * but the spec is the spec.  In fact the default implementation allows applications to opt-out of
+	 * global-identifier-quoting affecting column-definitions.
 	 *
 	 * @param text The text to be (possibly) quoted
 	 *
 	 * @return The identifier form
+	 *
+	 * @see AvailableSettings#GLOBALLY_QUOTED_IDENTIFIERS_SKIP_COLUMN_DEFINITIONS
 	 */
 	Identifier applyGlobalQuoting(String text);
 

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

@@ -34,6 +34,7 @@ public class IdentifierHelperBuilder {
 
 	private Set<String> reservedWords = new TreeSet<String>( String.CASE_INSENSITIVE_ORDER );
 	private boolean globallyQuoteIdentifiers = false;
+	private boolean skipGlobalQuotingForColumnDefinitions = false;
 	private boolean autoQuoteKeywords = true;
 	private IdentifierCaseStrategy unquotedCaseStrategy = IdentifierCaseStrategy.MIXED;
 	private IdentifierCaseStrategy quotedCaseStrategy = IdentifierCaseStrategy.MIXED;
@@ -136,6 +137,14 @@ public void setGloballyQuoteIdentifiers(boolean globallyQuoteIdentifiers) {
 		this.globallyQuoteIdentifiers = globallyQuoteIdentifiers;
 	}
 
+	public boolean isSkipGlobalQuotingForColumnDefinitions() {
+		return skipGlobalQuotingForColumnDefinitions;
+	}
+
+	public void setSkipGlobalQuotingForColumnDefinitions(boolean skipGlobalQuotingForColumnDefinitions) {
+		this.skipGlobalQuotingForColumnDefinitions = skipGlobalQuotingForColumnDefinitions;
+	}
+
 	public void setAutoQuoteKeywords(boolean autoQuoteKeywords) {
 		this.autoQuoteKeywords = autoQuoteKeywords;
 	}
@@ -191,6 +200,7 @@ public IdentifierHelper build() {
 				jdbcEnvironment,
 				nameQualifierSupport,
 				globallyQuoteIdentifiers,
+				skipGlobalQuotingForColumnDefinitions,
 				autoQuoteKeywords,
 				reservedWords,
 				unquotedCaseStrategy,

hibernate-core/src/test/java/org/hibernate/test/quote/ColumnDefinitionQuotingTest.java

@@ -0,0 +1,162 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.test.quote;
+
+import java.util.Iterator;
+import javax.persistence.Column;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.JoinColumn;
+import javax.persistence.ManyToOne;
+
+import org.hibernate.boot.MetadataSources;
+import org.hibernate.boot.registry.StandardServiceRegistry;
+import org.hibernate.boot.registry.StandardServiceRegistryBuilder;
+import org.hibernate.boot.spi.MetadataImplementor;
+import org.hibernate.cfg.AvailableSettings;
+import org.hibernate.dialect.Dialect;
+import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment;
+import org.hibernate.mapping.PersistentClass;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseUnitTestCase;
+import org.junit.Test;
+
+import static org.junit.Assert.assertTrue;
+
+/**
+ * @author Steve Ebersole
+ */
+public class ColumnDefinitionQuotingTest extends BaseUnitTestCase {
+	@Test
+	@TestForIssue( jiraKey = "HHH-9491" )
+	public void testExplicitQuoting() {
+		withStandardServiceRegistry(
+				false,
+				false,
+				new TestWork() {
+					@Override
+					public void doTestWork(StandardServiceRegistry ssr) {
+						MetadataImplementor metadata = (MetadataImplementor) new MetadataSources( ssr )
+								.addAnnotatedClass( E1.class )
+								.buildMetadata();
+						metadata.validate();
+
+						PersistentClass entityBinding = metadata.getEntityBinding( E1.class.getName() );
+
+						org.hibernate.mapping.Column idColumn = extractColumn( entityBinding.getIdentifier().getColumnIterator() );
+						assertTrue( isQuoted( idColumn.getSqlType(), ssr ) );
+
+						org.hibernate.mapping.Column otherColumn = extractColumn( entityBinding.getProperty( "other" ).getColumnIterator() );
+						assertTrue( isQuoted( otherColumn.getSqlType(), ssr ) );
+					}
+				}
+		);
+	}
+
+	private org.hibernate.mapping.Column extractColumn(Iterator columnIterator) {
+		return (org.hibernate.mapping.Column) columnIterator.next();
+	}
+
+	private boolean isQuoted(String sqlType, StandardServiceRegistry ssr) {
+		final Dialect dialect = ssr.getService( JdbcEnvironment.class ).getDialect();
+		return sqlType.charAt( 0  ) == dialect.openQuote()
+				&& sqlType.charAt( sqlType.length()-1 ) == dialect.closeQuote();
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-9491" )
+	public void testGlobalQuotingOptIn() {
+		withStandardServiceRegistry(
+				true,
+				true,
+				new TestWork() {
+					@Override
+					public void doTestWork(StandardServiceRegistry ssr) {
+						MetadataImplementor metadata = (MetadataImplementor) new MetadataSources( ssr )
+								.addAnnotatedClass( E2.class )
+								.buildMetadata();
+						metadata.validate();
+
+						PersistentClass entityBinding = metadata.getEntityBinding( E2.class.getName() );
+
+						org.hibernate.mapping.Column idColumn = extractColumn( entityBinding.getIdentifier().getColumnIterator() );
+						assertTrue( isQuoted( idColumn.getSqlType(), ssr ) );
+
+						org.hibernate.mapping.Column otherColumn = extractColumn( entityBinding.getProperty( "other" ).getColumnIterator() );
+						assertTrue( isQuoted( otherColumn.getSqlType(), ssr ) );
+					}
+				}
+		);
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-9491" )
+	public void testGlobalQuotingOptOut() {
+		withStandardServiceRegistry(
+				true,
+				false,
+				new TestWork() {
+					@Override
+					public void doTestWork(StandardServiceRegistry ssr) {
+						MetadataImplementor metadata = (MetadataImplementor) new MetadataSources( ssr )
+								.addAnnotatedClass( E2.class )
+								.buildMetadata();
+						metadata.validate();
+
+						PersistentClass entityBinding = metadata.getEntityBinding( E2.class.getName() );
+
+						org.hibernate.mapping.Column idColumn = extractColumn( entityBinding.getIdentifier().getColumnIterator() );
+						assertTrue( !isQuoted( idColumn.getSqlType(), ssr ) );
+
+						org.hibernate.mapping.Column otherColumn = extractColumn( entityBinding.getProperty( "other" ).getColumnIterator() );
+						assertTrue( !isQuoted( otherColumn.getSqlType(), ssr ) );
+					}
+				}
+		);
+	}
+
+	interface TestWork {
+		void doTestWork(StandardServiceRegistry ssr);
+	}
+
+	void withStandardServiceRegistry(boolean globalQuoting, boolean skipColumnDefinitions, TestWork work) {
+		final StandardServiceRegistry ssr = new StandardServiceRegistryBuilder()
+				.applySetting( AvailableSettings.GLOBALLY_QUOTED_IDENTIFIERS, globalQuoting )
+				.applySetting( AvailableSettings.GLOBALLY_QUOTED_IDENTIFIERS_SKIP_COLUMN_DEFINITIONS, skipColumnDefinitions )
+				.build();
+
+		try {
+			work.doTestWork( ssr );
+		}
+		finally {
+			StandardServiceRegistryBuilder.destroy( ssr );
+		}
+	}
+
+	@Entity
+	public static class E1 {
+		@Id
+		@Column( columnDefinition = "`explicitly quoted`" )
+		private Integer id;
+
+		@ManyToOne
+		@JoinColumn( columnDefinition = "`explicitly quoted`" )
+		private E1 other;
+	}
+
+	@Entity
+	public static class E2 {
+		@Id
+		@Column( columnDefinition = "not explicitly quoted" )
+		private Integer id;
+
+		@ManyToOne
+		@JoinColumn( columnDefinition = "not explicitly quoted" )
+		private E2 other;
+	}
+}