cleanups to binder code

- add two type checks + exceptions
- use static imports
- refactor out lots of little methods + remove some dupe code
- improve some Javadoc

hibernate-core/src/main/java/org/hibernate/AnnotationException.java

@@ -7,9 +7,7 @@
 package org.hibernate;
 
 /**
- * Annotation related exception.
+ * An exception that occurs while reading mapping annotations.
- *
- * The EJB3 EG will probably set a generic exception.  I'll then use this one.
  *
  * @author Emmanuel Bernard
  */

hibernate-core/src/main/java/org/hibernate/MappingException.java

@@ -7,8 +7,8 @@
 package org.hibernate;
 
 /**
- * An exception that occurs while reading mapping sources (xml/annotations), usually as a result of something
+ * An exception that occurs while reading mapping sources (xml/annotations),
- * screwy in the O-R mappings.
+ * usually as a result of something screwy in the O-R mappings.
  *
  * @author Gavin King
  */

hibernate-core/src/main/java/org/hibernate/annotations/LazyToOne.java

@@ -12,7 +12,8 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 /**
- * Define the laziness options available for a ToOne (ie OneToOne or ManyToOne) association.
+ * Define the laziness options available for a {@link jakarta.persistence.OneToOne}
+ * or {@link jakarta.persistence.ManyToOne}) association.
  *
  * @author Emmanuel Bernard
  */

hibernate-core/src/main/java/org/hibernate/annotations/Loader.java

@@ -15,8 +15,8 @@ import static java.lang.annotation.ElementType.TYPE;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 
 /**
- * Used to override how Hibernate performs load operations. naming a named query to use instead of
+ * Specifies that a named query should be used to load an entity,
- * its generated SELECT SQL.
+ * overriding the SQL that Hibernate generates by default.
  *
  * @author L<EFBFBD>szl<EFBFBD> Benke
  */
@@ -24,7 +24,7 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME;
 @Retention( RUNTIME )
 public @interface Loader {
 	/**
-	 * THe namedQuery to use for loading.
+	 * THe named query to use for loading the entity.
 	 */
 	String namedQuery() default "";
 }

hibernate-core/src/main/java/org/hibernate/annotations/Table.java

@@ -43,6 +43,8 @@ public @interface Table {
 
 	/**
 	 * A check constraint, written in native SQL.
+	 * <p>
+	 * <em>Useful for secondary tables, otherwise use {@link Check}.</em>
 	 *
 	 * @see Check
 	 */
@@ -50,6 +52,8 @@ public @interface Table {
 
 	/**
 	 * Specifies comment to add to the generated DDL for the table.
+	 * <p>
+	 * <em>Useful for secondary tables, otherwise use {@link Comment}.</em>
 	 *
 	 * @see Comment
 	 */

hibernate-core/src/main/java/org/hibernate/boot/internal/IdGeneratorInterpreterImpl.java

@@ -23,6 +23,8 @@ import jakarta.persistence.GenerationType;
 import jakarta.persistence.SequenceGenerator;
 import jakarta.persistence.TableGenerator;
 
+import static org.hibernate.cfg.BinderHelper.isEmptyAnnotationValue;
+
 /**
  * The root (composition) IdGenerationTypeInterpreter.
  *
@@ -128,31 +130,31 @@ public class IdGeneratorInterpreterImpl implements IdGeneratorStrategyInterprete
 			definitionBuilder.setStrategy( org.hibernate.id.enhanced.TableGenerator.class.getName() );
 			definitionBuilder.addParam( org.hibernate.id.enhanced.TableGenerator.CONFIG_PREFER_SEGMENT_PER_ENTITY, "true" );
 
-			if ( !BinderHelper.isEmptyAnnotationValue( tableGeneratorAnnotation.catalog() ) ) {
+			if ( !isEmptyAnnotationValue( tableGeneratorAnnotation.catalog() ) ) {
 				definitionBuilder.addParam( PersistentIdentifierGenerator.CATALOG, tableGeneratorAnnotation.catalog() );
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( tableGeneratorAnnotation.schema() ) ) {
+			if ( !isEmptyAnnotationValue( tableGeneratorAnnotation.schema() ) ) {
 				definitionBuilder.addParam( PersistentIdentifierGenerator.SCHEMA, tableGeneratorAnnotation.schema() );
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( tableGeneratorAnnotation.table() ) ) {
+			if ( !isEmptyAnnotationValue( tableGeneratorAnnotation.table() ) ) {
 				definitionBuilder.addParam(
 						org.hibernate.id.enhanced.TableGenerator.TABLE_PARAM,
 						tableGeneratorAnnotation.table()
 				);
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( tableGeneratorAnnotation.pkColumnName() ) ) {
+			if ( !isEmptyAnnotationValue( tableGeneratorAnnotation.pkColumnName() ) ) {
 				definitionBuilder.addParam(
 						org.hibernate.id.enhanced.TableGenerator.SEGMENT_COLUMN_PARAM,
 						tableGeneratorAnnotation.pkColumnName()
 				);
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( tableGeneratorAnnotation.pkColumnValue() ) ) {
+			if ( !isEmptyAnnotationValue( tableGeneratorAnnotation.pkColumnValue() ) ) {
 				definitionBuilder.addParam(
 						org.hibernate.id.enhanced.TableGenerator.SEGMENT_VALUE_PARAM,
 						tableGeneratorAnnotation.pkColumnValue()
 				);
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( tableGeneratorAnnotation.valueColumnName() ) ) {
+			if ( !isEmptyAnnotationValue( tableGeneratorAnnotation.valueColumnName() ) ) {
 				definitionBuilder.addParam(
 						org.hibernate.id.enhanced.TableGenerator.VALUE_COLUMN_PARAM,
 						tableGeneratorAnnotation.valueColumnName()
@@ -182,19 +184,19 @@ public class IdGeneratorInterpreterImpl implements IdGeneratorStrategyInterprete
 			definitionBuilder.setName( sequenceGeneratorAnnotation.name() );
 			definitionBuilder.setStrategy( SequenceStyleGenerator.class.getName() );
 
-			if ( !BinderHelper.isEmptyAnnotationValue( sequenceGeneratorAnnotation.catalog() ) ) {
+			if ( !isEmptyAnnotationValue( sequenceGeneratorAnnotation.catalog() ) ) {
 				definitionBuilder.addParam(
 						PersistentIdentifierGenerator.CATALOG,
 						sequenceGeneratorAnnotation.catalog()
 				);
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( sequenceGeneratorAnnotation.schema() ) ) {
+			if ( !isEmptyAnnotationValue( sequenceGeneratorAnnotation.schema() ) ) {
 				definitionBuilder.addParam(
 						PersistentIdentifierGenerator.SCHEMA,
 						sequenceGeneratorAnnotation.schema()
 				);
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( sequenceGeneratorAnnotation.sequenceName() ) ) {
+			if ( !isEmptyAnnotationValue( sequenceGeneratorAnnotation.sequenceName() ) ) {
 				definitionBuilder.addParam(
 						SequenceStyleGenerator.SEQUENCE_PARAM,
 						sequenceGeneratorAnnotation.sequenceName()

hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/annotations/AnnotationMetadataSourceProcessorImpl.java

@@ -19,6 +19,7 @@ import org.hibernate.boot.AttributeConverterInfo;
 import org.hibernate.boot.internal.MetadataBuildingContextRootImpl;
 import org.hibernate.boot.jaxb.mapping.JaxbEntityMappings;
 import org.hibernate.boot.jaxb.spi.Binding;
+import org.hibernate.boot.model.convert.internal.AttributeConverterManager;
 import org.hibernate.boot.model.convert.spi.ConverterDescriptor;
 import org.hibernate.boot.model.process.spi.ManagedResources;
 import org.hibernate.boot.model.source.spi.MetadataSourceProcessor;
@@ -169,42 +170,34 @@ public class AnnotationMetadataSourceProcessorImpl implements MetadataSourceProc
 
 	@Override
 	public void processTypeDefinitions() {
-
 	}
 
 	@Override
 	public void processQueryRenames() {
-
 	}
 
 	@Override
 	public void processNamedQueries() {
-
 	}
 
 	@Override
 	public void processAuxiliaryDatabaseObjectDefinitions() {
-
 	}
 
 	@Override
 	public void processIdentifierGenerators() {
-
 	}
 
 	@Override
 	public void processFilterDefinitions() {
-
 	}
 
 	@Override
 	public void processFetchProfiles() {
-
 	}
 
 	@Override
 	public void prepareForEntityHierarchyProcessing() {
-
 	}
 
 	@Override
@@ -215,16 +208,15 @@ public class AnnotationMetadataSourceProcessorImpl implements MetadataSourceProc
 				rootMetadataBuildingContext
 		);
 
-
 		for ( XClass clazz : orderedClasses ) {
 			if ( processedEntityNames.contains( clazz.getName() ) ) {
 				log.debugf( "Skipping annotated class processing of entity [%s], as it has already been processed", clazz );
-				continue;
 			}
-
+			else {
-			AnnotationBinder.bindClass( clazz, inheritanceStatePerClass, rootMetadataBuildingContext );
+				AnnotationBinder.bindClass( clazz, inheritanceStatePerClass, rootMetadataBuildingContext );
-			AnnotationBinder.bindFetchProfilesForClass( clazz, rootMetadataBuildingContext );
+				AnnotationBinder.bindFetchProfilesForClass( clazz, rootMetadataBuildingContext );
-			processedEntityNames.add( clazz.getName() );
+				processedEntityNames.add( clazz.getName() );
+			}
 		}
 	}
 
@@ -270,16 +262,15 @@ public class AnnotationMetadataSourceProcessorImpl implements MetadataSourceProc
 	}
 
 	private void orderHierarchy(List<XClass> copy, List<XClass> newList, List<XClass> original, XClass clazz) {
-		if ( clazz == null || reflectionManager.equals( clazz, Object.class ) ) {
+		if ( clazz != null && !reflectionManager.equals( clazz, Object.class ) ) {
-			return;
+			//process superclass first
-		}
+			orderHierarchy( copy, newList, original, clazz.getSuperclass() );
-		//process superclass first
+			if ( original.contains( clazz ) ) {
-		orderHierarchy( copy, newList, original, clazz.getSuperclass() );
+				if ( !newList.contains( clazz ) ) {
-		if ( original.contains( clazz ) ) {
+					newList.add( clazz );
-			if ( !newList.contains( clazz ) ) {
+				}
-				newList.add( clazz );
+				copy.remove( clazz );
 			}
-			copy.remove( clazz );
 		}
 	}
 
@@ -292,12 +283,10 @@ public class AnnotationMetadataSourceProcessorImpl implements MetadataSourceProc
 
 	@Override
 	public void processResultSetMappings() {
-
 	}
 
 	@Override
 	public void finishUp() {
-
 	}
 
 	private static class AttributeConverterManager implements AttributeConverterDefinitionCollector {

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

@@ -38,6 +38,8 @@ import org.hibernate.mapping.Table;
 import org.jboss.logging.Logger;
 
 import static org.hibernate.cfg.AnnotationBinder.getOverridableAnnotation;
+import static org.hibernate.cfg.BinderHelper.getRelativePath;
+import static org.hibernate.internal.util.StringHelper.isNotEmpty;
 
 /**
  * Wrap state of an EJB3 @Column annotation
@@ -107,7 +109,7 @@ public class AnnotatedColumn {
 	}
 
 	public boolean isFormula() {
-		return StringHelper.isNotEmpty( formulaString );
+		return isNotEmpty( formulaString );
 	}
 
 	@SuppressWarnings("UnusedDeclaration")
@@ -229,7 +231,7 @@ public class AnnotatedColumn {
 	}
 
 	public void bind() {
-		if ( StringHelper.isNotEmpty( formulaString ) ) {
+		if ( isNotEmpty( formulaString ) ) {
 			LOG.debugf( "Binding formula %s", formulaString );
 			formula = new Formula();
 			formula.setFormula( formulaString );
@@ -252,7 +254,7 @@ public class AnnotatedColumn {
 			if ( checkConstraint !=null ) {
 				mappingColumn.setCheckConstraint( checkConstraint );
 			}
-			if ( StringHelper.isNotEmpty( comment ) ) {
+			if ( isNotEmpty( comment ) ) {
 				mappingColumn.setComment( comment );
 			}
 			if ( generatedAs != null ) {
@@ -274,22 +276,22 @@ public class AnnotatedColumn {
 			String sqlType,
 			boolean unique,
 			boolean applyNamingStrategy) {
-		if ( StringHelper.isNotEmpty( formulaString ) ) {
+		if ( isNotEmpty( formulaString ) ) {
-			this.formula = new Formula();
+			formula = new Formula();
-			this.formula.setFormula( formulaString );
+			formula.setFormula( formulaString );
 		}
 		else {
-			this.mappingColumn = new Column();
+			mappingColumn = new Column();
 			redefineColumnName( columnName, propertyName, applyNamingStrategy );
-			this.mappingColumn.setLength( length );
+			mappingColumn.setLength( length );
 			if ( precision != null && precision > 0 ) {  //relevant precision
-				this.mappingColumn.setPrecision( precision );
+				mappingColumn.setPrecision( precision );
-				this.mappingColumn.setScale( scale );
+				mappingColumn.setScale( scale );
 			}
-			this.mappingColumn.setNullable( nullable );
+			mappingColumn.setNullable( nullable );
-			this.mappingColumn.setSqlType( sqlType );
+			mappingColumn.setSqlType( sqlType );
-			this.mappingColumn.setUnique( unique );
+			mappingColumn.setUnique( unique );
-			this.mappingColumn.setCheckConstraint( checkConstraint );
+			mappingColumn.setCheckConstraint( checkConstraint );
 
 			if ( writeExpression != null ) {
 				final int numberOfJdbcParams = StringHelper.count( writeExpression, '?' );
@@ -301,8 +303,8 @@ public class AnnotatedColumn {
 				}
 			}
 
-			this.mappingColumn.setResolvedCustomRead( readExpression );
+			mappingColumn.setResolvedCustomRead( readExpression );
-			this.mappingColumn.setCustomWrite( writeExpression );
+			mappingColumn.setCustomWrite( writeExpression );
 		}
 	}
 
@@ -311,7 +313,7 @@ public class AnnotatedColumn {
 	}
 
 	public void redefineColumnName(String columnName, String propertyName, boolean applyNamingStrategy) {
-		if ( StringHelper.isNotEmpty( columnName ) ) {
+		if ( isNotEmpty( columnName ) ) {
 			mappingColumn.setName( processColumnName( columnName, applyNamingStrategy ) );
 		}
 		else {
@@ -323,7 +325,7 @@ public class AnnotatedColumn {
 	}
 
 	private String processColumnName(String columnName, boolean applyNamingStrategy) {
-		if (applyNamingStrategy) {
+		if ( applyNamingStrategy ) {
 			Database database = context.getMetadataCollector().getDatabase();
 			return context.getBuildingOptions().getPhysicalNamingStrategy()
 					.toPhysicalColumnName( database.toIdentifier( columnName ), database.getJdbcEnvironment() )
@@ -368,8 +370,10 @@ public class AnnotatedColumn {
 
 		// HHH-6005 magic
 		if ( implicitName.getText().contains( "_collection&&element_" ) ) {
-			implicitName = Identifier.toIdentifier( implicitName.getText().replace( "_collection&&element_", "_" ),
+			implicitName = Identifier.toIdentifier(
-					implicitName.isQuoted() );
+					implicitName.getText().replace( "_collection&&element_", "_" ),
+					implicitName.isQuoted()
+			);
 		}
 
 		return context.getBuildingOptions().getPhysicalNamingStrategy()
@@ -433,7 +437,7 @@ public class AnnotatedColumn {
 
 	protected void addColumnBinding(SimpleValue value) {
 		final String logicalColumnName;
-		if ( StringHelper.isNotEmpty( this.logicalColumnName ) ) {
+		if ( isNotEmpty( this.logicalColumnName ) ) {
 			logicalColumnName = this.logicalColumnName;
 		}
 		else {
@@ -492,7 +496,7 @@ public class AnnotatedColumn {
 			throw new AssertionFailure( "Should not call getTable() on column w/o persistent class defined" );
 		}
 
-		return StringHelper.isNotEmpty( explicitTableName )
+		return isNotEmpty( explicitTableName )
 			&& !propertyHolder.getTable().getName().equals( explicitTableName );
 	}
 
@@ -652,7 +656,7 @@ public class AnnotatedColumn {
 	public static AnnotatedColumn[] buildColumnsOrFormulaFromAnnotation(
 			jakarta.persistence.Column[] columnAnns,
 			org.hibernate.annotations.Formula formulaAnn,
-			Comment commentAnn,
+			Comment comment,
 			Nullability nullability,
 			PropertyHolder propertyHolder,
 			PropertyData inferredData,
@@ -670,129 +674,153 @@ public class AnnotatedColumn {
 			return new AnnotatedColumn[] { formulaColumn };
 		}
 		else {
-			jakarta.persistence.Column[] actualCols = columnAnns;
+			jakarta.persistence.Column[]  actualCols = overrideColumns( columnAnns, propertyHolder, inferredData);
-			jakarta.persistence.Column[] overriddenCols = propertyHolder.getOverriddenColumn(
-					StringHelper.qualify( propertyHolder.getPath(), inferredData.getPropertyName() )
-			);
-			if ( overriddenCols != null ) {
-				//check for overridden first
-				if ( columnAnns != null && overriddenCols.length != columnAnns.length ) {
-					throw new AnnotationException( "AttributeOverride.column() should override all columns for now" );
-				}
-				actualCols = overriddenCols.length == 0 ? null : overriddenCols;
-				LOG.debugf( "Column(s) overridden for property %s", inferredData.getPropertyName() );
-			}
-
-			AnnotatedColumn[] columns;
 			if ( actualCols == null ) {
-				columns = buildImplicitColumn(
+				return buildImplicitColumn(
 						inferredData,
 						suffixForDefaultColumnName,
 						secondaryTables,
 						propertyHolder,
-						commentAnn,
+						comment,
 						nullability,
 						context
 				);
 			}
 			else {
-				final int length = actualCols.length;
+				return buildExplicitColumns(
-				columns = new AnnotatedColumn[length];
+						comment,
-				for (int index = 0; index < length; index++) {
+						propertyHolder,
+						inferredData,
+						suffixForDefaultColumnName,
+						secondaryTables,
+						context,
+						actualCols
+				);
+			}
+		}
+	}
 
-					final ObjectNameNormalizer normalizer = context.getObjectNameNormalizer();
+	private static jakarta.persistence.Column[] overrideColumns(
-					final Database database = context.getMetadataCollector().getDatabase();
+			jakarta.persistence.Column[] columnAnns,
+			PropertyHolder propertyHolder,
+			PropertyData inferredData ) {
+		final jakarta.persistence.Column[] overriddenCols = propertyHolder.getOverriddenColumn(
+				StringHelper.qualify( propertyHolder.getPath(), inferredData.getPropertyName() )
+		);
+		if ( overriddenCols != null ) {
+			//check for overridden first
+			if ( columnAnns != null && overriddenCols.length != columnAnns.length ) {
+				throw new AnnotationException( "AttributeOverride.column() should override all columns for now" );
+			}
+			LOG.debugf( "Column(s) overridden for property %s", inferredData.getPropertyName() );
+			return overriddenCols.length == 0 ? null : overriddenCols;
+		}
+		else {
+			return columnAnns;
+		}
+	}
 
-					jakarta.persistence.Column col = actualCols[index];
+	private static AnnotatedColumn[] buildExplicitColumns(
-
+			Comment comment,
-					final String sqlType;
+			PropertyHolder propertyHolder,
-					if ( col.columnDefinition().isEmpty() ) {
+			PropertyData inferredData,
-						sqlType = null;
+			String suffixForDefaultColumnName,
-					}
+			Map<String, Join> secondaryTables,
-					else {
+			MetadataBuildingContext context,
-						sqlType = normalizer.applyGlobalQuoting( col.columnDefinition() );
+			jakarta.persistence.Column[] actualCols) {
-					}
+		final int length = actualCols.length;
-
+		final AnnotatedColumn[] columns = new AnnotatedColumn[length];
-					final String tableName;
+		for (int index = 0; index < length; index++) {
-					if ( StringHelper.isEmpty( col.table() ) ) {
+			final jakarta.persistence.Column column = actualCols[index];
-						tableName = "";
+			final Database database = context.getMetadataCollector().getDatabase();
-					}
+			final String sqlType = column.columnDefinition().isEmpty() ? null
-					else {
+					: context.getObjectNameNormalizer().applyGlobalQuoting( column.columnDefinition() );
-						tableName = database.getJdbcEnvironment()
+			final String tableName = StringHelper.isEmpty( column.table() ) ? ""
-								.getIdentifierHelper()
+					: database.getJdbcEnvironment().getIdentifierHelper().toIdentifier( column.table() ).render();
-								.toIdentifier( col.table() )
-								.render();
 //						final Identifier logicalName = database.getJdbcEnvironment()
 //								.getIdentifierHelper()
-//								.toIdentifier( col.table() );
+//								.toIdentifier( column.table() );
 //						final Identifier physicalName = physicalNamingStrategy.toPhysicalTableName( logicalName );
 //						tableName = physicalName.render( database.getDialect() );
-					}
+			columns[index] = buildColumn(
-
+					comment,
-					final String columnName;
+					propertyHolder,
-					if ( col.name() != null && col.name().isEmpty() ) {
+					inferredData,
-						columnName = null;
+					suffixForDefaultColumnName,
-					}
+					secondaryTables,
-					else {
+					context,
-						// NOTE : this is the logical column name, not the physical!
+					length,
-						columnName = database.getJdbcEnvironment()
+					database,
-								.getIdentifierHelper()
+					column,
-								.toIdentifier( col.name() )
+					sqlType,
-								.render();
+					tableName
-					}
+			);
-
-					AnnotatedColumn column = new AnnotatedColumn();
-
-					column.setImplicit( false );
-					column.setSqlType( sqlType );
-					column.setLength( (long) col.length() );
-					column.setPrecision( col.precision() );
-					column.setScale( col.scale() );
-					if ( StringHelper.isEmpty( columnName ) && ! StringHelper.isEmpty( suffixForDefaultColumnName ) ) {
-						column.setLogicalColumnName( inferredData.getPropertyName() + suffixForDefaultColumnName );
-					}
-					else {
-						column.setLogicalColumnName( columnName );
-					}
-
-					column.setPropertyName(
-							BinderHelper.getRelativePath( propertyHolder, inferredData.getPropertyName() )
-					);
-					column.setNullable(
-						col.nullable()
-					); //TODO force to not null if available? This is a (bad) user choice.
-					if ( commentAnn != null ) {
-						column.setComment( commentAnn.value() );
-					}
-					column.setUnique( col.unique() );
-					column.setInsertable( col.insertable() );
-					column.setUpdatable( col.updatable() );
-					column.setExplicitTableName( tableName );
-					column.setPropertyHolder( propertyHolder );
-					column.setJoins( secondaryTables );
-					column.setBuildingContext( context );
-					column.applyColumnDefault( inferredData, length );
-					column.applyGeneratedAs( inferredData, length );
-					column.applyCheckConstraint( inferredData, length );
-					column.extractDataFromPropertyData(inferredData);
-					column.bind();
-					columns[index] = column;
-				}
-			}
-
-			return columns;
 		}
+		return columns;
+	}
+
+	private static AnnotatedColumn buildColumn(
+			Comment comment,
+			PropertyHolder propertyHolder,
+			PropertyData inferredData,
+			String suffixForDefaultColumnName,
+			Map<String, Join> secondaryTables,
+			MetadataBuildingContext context,
+			int length,
+			Database database,
+			jakarta.persistence.Column col,
+			String sqlType,
+			String tableName) {
+
+		final AnnotatedColumn column = new AnnotatedColumn();
+		column.setLogicalColumnName( getLogicalColumnName( inferredData, suffixForDefaultColumnName, database, col ) );
+		column.setImplicit( false );
+		column.setSqlType(sqlType);
+		column.setLength( (long) col.length() );
+		column.setPrecision( col.precision() );
+		column.setScale( col.scale() );
+		column.setPropertyName( getRelativePath( propertyHolder, inferredData.getPropertyName() ) );
+		column.setNullable( col.nullable() ); //TODO force to not null if available? This is a (bad) user choice.
+		if ( comment != null ) {
+			column.setComment( comment.value() );
+		}
+		column.setUnique( col.unique() );
+		column.setInsertable( col.insertable() );
+		column.setUpdatable( col.updatable() );
+		column.setExplicitTableName( tableName );
+		column.setPropertyHolder( propertyHolder );
+		column.setJoins( secondaryTables );
+		column.setBuildingContext( context );
+		column.applyColumnDefault( inferredData, length );
+		column.applyGeneratedAs( inferredData, length );
+		column.applyCheckConstraint( inferredData, length );
+		column.extractDataFromPropertyData( inferredData );
+		column.bind();
+		return column;
+	}
+
+	private static String getLogicalColumnName(
+			PropertyData inferredData,
+			String suffixForDefaultColumnName,
+			Database database,
+			jakarta.persistence.Column column) {
+		final String columnName = column.name() != null && column.name().isEmpty() ? null
+				: database.getJdbcEnvironment().getIdentifierHelper().toIdentifier( column.name() ).render();
+		// NOTE : this is the logical column name, not the physical!
+		return StringHelper.isEmpty( columnName ) && !StringHelper.isEmpty(suffixForDefaultColumnName)
+				? inferredData.getPropertyName() + suffixForDefaultColumnName
+				: columnName;
 	}
 
 	private void applyColumnDefault(PropertyData inferredData, int length) {
 		final XProperty xProperty = inferredData.getProperty();
 		if ( xProperty != null ) {
-			ColumnDefault columnDefaultAnn = getOverridableAnnotation( xProperty, ColumnDefault.class, context );
+			ColumnDefault columnDefault = getOverridableAnnotation( xProperty, ColumnDefault.class, context );
-			if ( columnDefaultAnn != null ) {
+			if ( columnDefault != null ) {
 				if (length!=1) {
 					throw new MappingException("@ColumnDefault may only be applied to single-column mappings");
 				}
-				setDefaultValue( columnDefaultAnn.value() );
+				setDefaultValue( columnDefault.value() );
 			}
 		}
 		else {
@@ -805,12 +833,12 @@ public class AnnotatedColumn {
 	private void applyGeneratedAs(PropertyData inferredData, int length) {
 		final XProperty xProperty = inferredData.getProperty();
 		if ( xProperty != null ) {
-			GeneratedColumn generatedAnn = getOverridableAnnotation( xProperty, GeneratedColumn.class, context );
+			GeneratedColumn generatedColumn = getOverridableAnnotation( xProperty, GeneratedColumn.class, context );
-			if ( generatedAnn != null ) {
+			if ( generatedColumn != null ) {
 				if (length!=1) {
 					throw new MappingException("@GeneratedColumn may only be applied to single-column mappings");
 				}
-				setGeneratedAs( generatedAnn.value() );
+				setGeneratedAs( generatedColumn.value() );
 			}
 		}
 		else {
@@ -823,12 +851,12 @@ public class AnnotatedColumn {
 	private void applyCheckConstraint(PropertyData inferredData, int length) {
 		final XProperty xProperty = inferredData.getProperty();
 		if ( xProperty != null ) {
-			Check columnDefaultAnn = AnnotationBinder.getOverridableAnnotation( xProperty, Check.class, context );
+			Check check = AnnotationBinder.getOverridableAnnotation( xProperty, Check.class, context );
-			if ( columnDefaultAnn != null ) {
+			if ( check != null ) {
 				if (length!=1) {
 					throw new MappingException("@Check may only be applied to single-column mappings (use a table-level @Check)");
 				}
-				setCheckConstraint( columnDefaultAnn.constraints() );
+				setCheckConstraint( check.constraints() );
 			}
 		}
 		else {
@@ -844,33 +872,27 @@ public class AnnotatedColumn {
 			XProperty property = inferredData.getProperty();
 			if ( property != null ) {
 				if ( propertyHolder.isComponent() ) {
-					processExpression( propertyHolder.getOverriddenColumnTransformer( logicalColumnName ) );
+					processColumnTransformerExpressions( propertyHolder.getOverriddenColumnTransformer( logicalColumnName ) );
 				}
-				processExpression( property.getAnnotation( ColumnTransformer.class ) );
+				processColumnTransformerExpressions( property.getAnnotation( ColumnTransformer.class ) );
 				ColumnTransformers annotations = property.getAnnotation( ColumnTransformers.class );
 				if (annotations != null) {
 					for ( ColumnTransformer annotation : annotations.value() ) {
-						processExpression( annotation );
+						processColumnTransformerExpressions( annotation );
 					}
 				}
 			}
 		}
 	}
 
-	private void processExpression(ColumnTransformer annotation) {
+	private void processColumnTransformerExpressions(ColumnTransformer annotation) {
-		if ( annotation == null ) {
+		if ( annotation != null ) {
-			return;
+			if ( StringHelper.isEmpty( annotation.forColumn() )
-		}
+					// "" is the default value for annotations
-
+					|| annotation.forColumn().equals( logicalColumnName != null ? logicalColumnName : "" ) ) {
-		final String nonNullLogicalColumnName = logicalColumnName != null
+				readExpression = StringHelper.nullIfEmpty( annotation.read() );
-				? logicalColumnName
+				writeExpression = StringHelper.nullIfEmpty( annotation.write() );
-				//use the default for annotations
+			}
-				: "";
-
-		if ( StringHelper.isEmpty( annotation.forColumn() )
-				|| annotation.forColumn().equals( nonNullLogicalColumnName ) ) {
-			readExpression = StringHelper.nullIfEmpty( annotation.read() );
-			writeExpression = StringHelper.nullIfEmpty( annotation.write() );
 		}
 	}
 
@@ -882,14 +904,31 @@ public class AnnotatedColumn {
 			Comment comment,
 			Nullability nullability,
 			MetadataBuildingContext context) {
-		AnnotatedColumn column = new AnnotatedColumn();
+		final AnnotatedColumn[] columns = new AnnotatedColumn[1];
-		AnnotatedColumn[] columns = new AnnotatedColumn[1];
+		columns[0] = bindImplicitColumn(
-		columns[0] = column;
+				inferredData,
+				suffixForDefaultColumnName,
+				secondaryTables,
+				propertyHolder,
+				comment,
+				nullability,
+				context
+		);
+		return columns;
+	}
 
+	private static AnnotatedColumn bindImplicitColumn(
+			PropertyData inferredData,
+			String suffixForDefaultColumnName,
+			Map<String, Join> secondaryTables,
+			PropertyHolder propertyHolder,
+			Comment comment,
+			Nullability nullability,
+			MetadataBuildingContext context) {
+		final AnnotatedColumn column = new AnnotatedColumn();
 		if ( comment != null ) {
 			column.setComment( comment.value() );
 		}
-
 		//not following the spec but more clean
 		if ( nullability != Nullability.FORCED_NULL
 				&& inferredData.getClassOrElement().isPrimitive()
@@ -897,13 +936,10 @@ public class AnnotatedColumn {
 			column.setNullable( false );
 		}
 		final String propertyName = inferredData.getPropertyName();
-		column.setPropertyName(
+		column.setPropertyName( getRelativePath( propertyHolder, propertyName ) );
-				BinderHelper.getRelativePath( propertyHolder, propertyName )
+		column.setPropertyHolder(propertyHolder);
-		);
+		column.setJoins(secondaryTables);
-		column.setPropertyHolder( propertyHolder );
+		column.setBuildingContext(context);
-		column.setJoins( secondaryTables );
-		column.setBuildingContext( context );
-
 		// property name + suffix is an "explicit" column name
 		boolean implicit = StringHelper.isEmpty( suffixForDefaultColumnName );
 		if ( !implicit ) {
@@ -915,49 +951,43 @@ public class AnnotatedColumn {
 		column.applyCheckConstraint( inferredData, 1 );
 		column.extractDataFromPropertyData( inferredData );
 		column.bind();
-
+		return column;
-		return columns;
 	}
 
 	public static void checkPropertyConsistency(AnnotatedColumn[] columns, String propertyName) {
 		int nbrOfColumns = columns.length;
-
 		if ( nbrOfColumns > 1 ) {
 			for (int currentIndex = 1; currentIndex < nbrOfColumns; currentIndex++) {
-
+				if ( !columns[currentIndex].isFormula() && !columns[currentIndex - 1].isFormula() ) {
-				if (columns[currentIndex].isFormula() || columns[currentIndex - 1].isFormula()) {
+					if ( columns[currentIndex].isInsertable() != columns[currentIndex - 1].isInsertable() ) {
-					continue;
+						throw new AnnotationException(
-				}
+								"Mixing insertable and non insertable columns in a property is not allowed: " + propertyName
-
+						);
-				if ( columns[currentIndex].isInsertable() != columns[currentIndex - 1].isInsertable() ) {
+					}
-					throw new AnnotationException(
+					if ( columns[currentIndex].isNullable() != columns[currentIndex - 1].isNullable() ) {
-							"Mixing insertable and non insertable columns in a property is not allowed: " + propertyName
+						throw new AnnotationException(
-					);
+								"Mixing nullable and non nullable columns in a property is not allowed: " + propertyName
-				}
+						);
-				if ( columns[currentIndex].isNullable() != columns[currentIndex - 1].isNullable() ) {
+					}
-					throw new AnnotationException(
+					if ( columns[currentIndex].isUpdatable() != columns[currentIndex - 1].isUpdatable() ) {
-							"Mixing nullable and non nullable columns in a property is not allowed: " + propertyName
+						throw new AnnotationException(
-					);
+								"Mixing updatable and non updatable columns in a property is not allowed: " + propertyName
-				}
+						);
-				if ( columns[currentIndex].isUpdatable() != columns[currentIndex - 1].isUpdatable() ) {
+					}
-					throw new AnnotationException(
+					if ( !columns[currentIndex].getTable().equals( columns[currentIndex - 1].getTable() ) ) {
-							"Mixing updatable and non updatable columns in a property is not allowed: " + propertyName
+						throw new AnnotationException(
-					);
+								"Mixing different tables in a property is not allowed: " + propertyName
-				}
+						);
-				if ( !columns[currentIndex].getTable().equals( columns[currentIndex - 1].getTable() ) ) {
+					}
-					throw new AnnotationException(
-							"Mixing different tables in a property is not allowed: " + propertyName
-					);
 				}
 			}
 		}
-
 	}
 
 	public void addIndex(Index index, boolean inSecondPass) {
-		if ( index == null ) return;
+		if ( index != null ) {
-		String indexName = index.name();
+			addIndex( index.name(), inSecondPass );
-		addIndex( indexName, inSecondPass );
+		}
 	}
 
 	void addIndex(String indexName, boolean inSecondPass) {

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

@@ -12,6 +12,8 @@ import org.hibernate.AssertionFailure;
 import org.hibernate.annotations.DiscriminatorFormula;
 import org.hibernate.boot.spi.MetadataBuildingContext;
 
+import static org.hibernate.cfg.BinderHelper.isEmptyAnnotationValue;
+
 /**
  * Discriminator column
  *
@@ -42,51 +44,69 @@ public class AnnotatedDiscriminatorColumn extends AnnotatedColumn {
 	}
 
 	public static AnnotatedDiscriminatorColumn buildDiscriminatorColumn(
-			DiscriminatorType type, DiscriminatorColumn discAnn,
+			DiscriminatorType type,
+			DiscriminatorColumn discAnn,
 			DiscriminatorFormula discFormulaAnn,
 			MetadataBuildingContext context) {
-		AnnotatedDiscriminatorColumn discriminatorColumn = new AnnotatedDiscriminatorColumn();
+		final AnnotatedDiscriminatorColumn discriminatorColumn = new AnnotatedDiscriminatorColumn();
 		discriminatorColumn.setBuildingContext( context );
-		discriminatorColumn.setImplicit( true );
 		if ( discFormulaAnn != null ) {
 			discriminatorColumn.setImplicit( false );
 			discriminatorColumn.setFormula( discFormulaAnn.value() );
 		}
 		else if ( discAnn != null ) {
 			discriminatorColumn.setImplicit( false );
-			if ( !BinderHelper.isEmptyAnnotationValue( discAnn.columnDefinition() ) ) {
+			if ( !isEmptyAnnotationValue( discAnn.columnDefinition() ) ) {
-				discriminatorColumn.setSqlType(
+				discriminatorColumn.setSqlType( discAnn.columnDefinition() );
-						discAnn.columnDefinition()
-				);
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( discAnn.name() ) ) {
+			if ( !isEmptyAnnotationValue( discAnn.name() ) ) {
 				discriminatorColumn.setLogicalColumnName( discAnn.name() );
 			}
 			discriminatorColumn.setNullable( false );
 		}
-		if ( DiscriminatorType.CHAR.equals( type ) ) {
-			discriminatorColumn.setDiscriminatorTypeName( "character" );
-			discriminatorColumn.setImplicit( false );
-		}
-		else if ( DiscriminatorType.INTEGER.equals( type ) ) {
-			discriminatorColumn.setDiscriminatorTypeName( "integer" );
-			discriminatorColumn.setImplicit( false );
-		}
-		else if ( DiscriminatorType.STRING.equals( type ) || type == null ) {
-			if ( discAnn != null ) discriminatorColumn.setLength( (long) discAnn.length() );
-			discriminatorColumn.setDiscriminatorTypeName( "string" );
-		}
 		else {
-			throw new AssertionFailure( "Unknown discriminator type: " + type );
+			discriminatorColumn.setImplicit( true );
 		}
+		setDiscriminatorType( type, discAnn, discriminatorColumn );
 		discriminatorColumn.bind();
 		return discriminatorColumn;
 	}
 
+	private static void setDiscriminatorType(
+			DiscriminatorType type,
+			DiscriminatorColumn discAnn,
+			AnnotatedDiscriminatorColumn discriminatorColumn) {
+		if ( type == null ) {
+			discriminatorColumn.setDiscriminatorTypeName( "string" );
+		}
+		else {
+			switch ( type ) {
+				case CHAR:
+					discriminatorColumn.setDiscriminatorTypeName( "character" );
+					discriminatorColumn.setImplicit( false );
+					break;
+				case INTEGER:
+					discriminatorColumn.setDiscriminatorTypeName( "integer" );
+					discriminatorColumn.setImplicit( false );
+					break;
+				case STRING:
+					discriminatorColumn.setDiscriminatorTypeName( "string" );
+					if ( discAnn != null ) {
+						discriminatorColumn.setLength( (long) discAnn.length() );
+					}
+					break;
+				default:
+					throw new AssertionFailure( "Unknown discriminator type: " + type );
+			}
+		}
+	}
+
 	@Override
 	public String toString() {
-		return String.format("DiscriminatorColumn{logicalColumnName'%s', discriminatorTypeName='%s'}",
+		return String.format(
-				getLogicalColumnName(), discriminatorTypeName
+				"DiscriminatorColumn{logicalColumnName'%s', discriminatorTypeName='%s'}",
+				getLogicalColumnName(),
+				discriminatorTypeName
 		);
 	}
 }

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

@@ -29,6 +29,7 @@ import org.hibernate.boot.model.naming.ObjectNameNormalizer;
 import org.hibernate.boot.model.naming.PhysicalNamingStrategy;
 import org.hibernate.boot.model.relational.Database;
 import org.hibernate.boot.model.source.spi.AttributePath;
+import org.hibernate.boot.spi.InFlightMetadataCollector;
 import org.hibernate.boot.spi.MetadataBuildingContext;
 import org.hibernate.internal.util.StringHelper;
 import org.hibernate.mapping.Column;
@@ -40,6 +41,10 @@ import org.hibernate.mapping.SimpleValue;
 import org.hibernate.mapping.Table;
 import org.hibernate.mapping.Value;
 
+import static org.hibernate.cfg.BinderHelper.findColumnOwner;
+import static org.hibernate.cfg.BinderHelper.getRelativePath;
+import static org.hibernate.cfg.BinderHelper.isEmptyAnnotationValue;
+
 /**
  * Wrap state of an EJB3 @JoinColumn annotation
  * and build the Hibernate column mapping element
@@ -123,7 +128,7 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 		setPropertyHolder( propertyHolder );
 		setJoins( joins );
 		setBuildingContext( buildingContext );
-		setPropertyName( BinderHelper.getRelativePath( propertyHolder, propertyName ) );
+		setPropertyName( getRelativePath( propertyHolder, propertyName ) );
 		bind();
 		this.referencedColumn = referencedColumn;
 		this.mappedBy = mappedBy;
@@ -175,7 +180,7 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 		formulaColumn.setBuildingContext( buildingContext );
 		formulaColumn.setPropertyHolder( propertyHolder );
 		formulaColumn.setJoins( joins );
-		formulaColumn.setPropertyName( BinderHelper.getRelativePath( propertyHolder, propertyName ) );
+		formulaColumn.setPropertyName( getRelativePath( propertyHolder, propertyName ) );
 		formulaColumn.bind();
 		return formulaColumn;
 	}
@@ -254,7 +259,7 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 			if ( !BinderHelper.isEmptyOrNullAnnotationValue( mappedBy ) ) {
 				throw new AnnotationException(
 						"Illegal attempt to define a @JoinColumn with a mappedBy association: "
-								+ BinderHelper.getRelativePath( propertyHolder, propertyName )
+								+ getRelativePath( propertyHolder, propertyName )
 				);
 			}
 			AnnotatedJoinColumn joinColumn = new AnnotatedJoinColumn();
@@ -267,7 +272,7 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 			}
 			joinColumn.setJoins( joins );
 			joinColumn.setPropertyHolder( propertyHolder );
-			joinColumn.setPropertyName( BinderHelper.getRelativePath( propertyHolder, propertyName ) );
+			joinColumn.setPropertyName( getRelativePath( propertyHolder, propertyName ) );
 			joinColumn.setImplicit( false );
 			joinColumn.bind();
 			return joinColumn;
@@ -277,9 +282,7 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 			joinColumn.setMappedBy( mappedBy );
 			joinColumn.setJoins( joins );
 			joinColumn.setPropertyHolder( propertyHolder );
-			joinColumn.setPropertyName(
+			joinColumn.setPropertyName( getRelativePath( propertyHolder, propertyName ) );
-					BinderHelper.getRelativePath( propertyHolder, propertyName )
-			);
 			// property name + suffix is an "explicit" column name
 			if ( !StringHelper.isEmpty( suffixForDefaultColumnName ) ) {
 				joinColumn.setLogicalColumnName( propertyName + suffixForDefaultColumnName );
@@ -302,10 +305,10 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 		}
 		else {
 			setImplicit( false );
-			if ( !BinderHelper.isEmptyAnnotationValue( annJoin.columnDefinition() ) ) {
+			if ( !isEmptyAnnotationValue( annJoin.columnDefinition() ) ) {
 				setSqlType( getBuildingContext().getObjectNameNormalizer().applyGlobalQuoting( annJoin.columnDefinition() ) );
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( annJoin.name() ) ) {
+			if ( !isEmptyAnnotationValue( annJoin.name() ) ) {
 				setLogicalColumnName( annJoin.name() );
 			}
 			setNullable( annJoin.nullable() );
@@ -314,7 +317,7 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 			setUpdatable( annJoin.updatable() );
 			setReferencedColumn( annJoin.referencedColumnName() );
 
-			if ( BinderHelper.isEmptyAnnotationValue( annJoin.table() ) ) {
+			if ( isEmptyAnnotationValue( annJoin.table() ) ) {
 				setExplicitTableName( "" );
 			}
 			else {
@@ -342,13 +345,11 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 			PropertyHolder propertyHolder,
 			MetadataBuildingContext context) {
 
-		final ObjectNameNormalizer normalizer = context.getObjectNameNormalizer();
+		final String defaultName = context.getMetadataCollector().getLogicalColumnName(
-
-		Column col = identifier.getColumns().get(0);
-		String defaultName = context.getMetadataCollector().getLogicalColumnName(
 				identifier.getTable(),
-				col.getQuotedName()
+				identifier.getColumns().get(0).getQuotedName()
 		);
+		final ObjectNameNormalizer normalizer = context.getObjectNameNormalizer();
 
 		if ( pkJoinAnn != null || joinAnn != null ) {
 			String colName;
@@ -364,25 +365,13 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 				columnDefinition = joinAnn.columnDefinition();
 				referencedColumnName = joinAnn.referencedColumnName();
 			}
-
-			final String sqlType;
-			if ( columnDefinition.isEmpty() ) {
-				sqlType = null;
-			}
-			else {
-				sqlType = normalizer.toDatabaseIdentifierText( columnDefinition );
-			}
-
-			final String name;
-			if ( colName != null && colName.isEmpty() ) {
-				name = normalizer.normalizeIdentifierQuotingAsString( defaultName );
-			}
-			else {
-				name = context.getObjectNameNormalizer().normalizeIdentifierQuotingAsString( colName );
-			}
 			return new AnnotatedJoinColumn(
-					sqlType,
+					columnDefinition.isEmpty()
-					name,
+							? null
+							: normalizer.toDatabaseIdentifierText( columnDefinition ),
+					colName != null && colName.isEmpty()
+							? normalizer.normalizeIdentifierQuotingAsString( defaultName )
+							: normalizer.normalizeIdentifierQuotingAsString( colName ),
 					null,
 					false,
 					false,
@@ -399,10 +388,9 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 			);
 		}
 		else {
-			defaultName = context.getObjectNameNormalizer().normalizeIdentifierQuotingAsString( defaultName );
 			return new AnnotatedJoinColumn(
 					null,
-					defaultName,
+					normalizer.normalizeIdentifierQuotingAsString( defaultName ),
 					null,
 					false,
 					false,
@@ -495,36 +483,26 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 	}
 
 	private String buildDefaultColumnName(final PersistentClass referencedEntity, final String logicalReferencedColumn) {
-		final Database database = getBuildingContext().getMetadataCollector().getDatabase();
+		final InFlightMetadataCollector metadataCollector = getBuildingContext().getMetadataCollector();
+		final Database database = metadataCollector.getDatabase();
 		final ImplicitNamingStrategy implicitNamingStrategy = getBuildingContext().getBuildingOptions().getImplicitNamingStrategy();
 		final PhysicalNamingStrategy physicalNamingStrategy = getBuildingContext().getBuildingOptions().getPhysicalNamingStrategy();
 
-		Identifier columnIdentifier;
 		boolean mappedBySide = mappedByTableName != null || mappedByPropertyName != null;
 		boolean ownerSide = getPropertyName() != null;
-
 		boolean isRefColumnQuoted = StringHelper.isQuoted( logicalReferencedColumn );
 
+		Identifier columnIdentifier;
 		if ( mappedBySide ) {
 			// NOTE : While it is completely misleading here to allow for the combination
 			//		of a "JPA ElementCollection" to be mappedBy, the code that uses this
 			// 		class relies on this behavior for handling the inverse side of
 			// 		many-to-many mappings
-
-			final AttributePath attributePath = AttributePath.parse( mappedByPropertyName );
-			final ImplicitJoinColumnNameSource.Nature implicitNamingNature;
-			if ( getPropertyHolder().isEntity() ) {
-				implicitNamingNature = ImplicitJoinColumnNameSource.Nature.ENTITY;
-			}
-			else if ( JPA2ElementCollection ) {
-				implicitNamingNature = ImplicitJoinColumnNameSource.Nature.ELEMENT_COLLECTION;
-			}
-			else {
-				implicitNamingNature = ImplicitJoinColumnNameSource.Nature.ENTITY_COLLECTION;
-			}
-
 			columnIdentifier = implicitNamingStrategy.determineJoinColumnName(
 					new ImplicitJoinColumnNameSource() {
+						final AttributePath attributePath = AttributePath.parse( mappedByPropertyName );
+						final ImplicitJoinColumnNameSource.Nature implicitNamingNature = getImplicitNature();
+
 						private final EntityNaming entityNaming = new EntityNaming() {
 							@Override
 							public String getClassName() {
@@ -542,9 +520,7 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 							}
 						};
 
-						private final Identifier referencedTableName = getBuildingContext().getMetadataCollector()
+						private final Identifier referencedTableName = database.toIdentifier( mappedByTableName );
-								.getDatabase()
-								.toIdentifier( mappedByTableName );
 
 						@Override
 						public Nature getNature() {
@@ -569,18 +545,15 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 						@Override
 						public Identifier getReferencedColumnName() {
 							if ( logicalReferencedColumn != null ) {
-								return getBuildingContext().getMetadataCollector()
+								return database.toIdentifier( logicalReferencedColumn );
-										.getDatabase()
-										.toIdentifier( logicalReferencedColumn );
 							}
 
 							if ( mappedByEntityName == null || mappedByPropertyName == null ) {
 								return null;
 							}
 
-							final PersistentClass mappedByEntityBinding = getBuildingContext().getMetadataCollector()
+							final Property mappedByProperty = metadataCollector.getEntityBinding( mappedByEntityName )
-									.getEntityBinding( mappedByEntityName );
+									.getProperty( mappedByPropertyName );
-							final Property mappedByProperty = mappedByEntityBinding.getProperty( mappedByPropertyName );
 							final SimpleValue value = (SimpleValue) mappedByProperty.getValue();
 							if ( value.getSelectables().isEmpty() ) {
 								throw new AnnotationException(
@@ -613,9 +586,7 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 										)
 								);
 							}
-							return getBuildingContext().getMetadataCollector()
+							return database.toIdentifier( ( (Column) selectable ).getQuotedName() );
-									.getDatabase()
-									.toIdentifier( ( (Column) selectable ).getQuotedName() );
 						}
 
 						@Override
@@ -631,23 +602,12 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 			}
 		}
 		else if ( ownerSide ) {
-			final String logicalTableName = getBuildingContext().getMetadataCollector().getLogicalTableName(
+			final String logicalTableName = metadataCollector.getLogicalTableName( referencedEntity.getTable() );
-					referencedEntity.getTable()
-			);
 
-			final ImplicitJoinColumnNameSource.Nature implicitNamingNature;
+			columnIdentifier =implicitNamingStrategy.determineJoinColumnName(
-			if ( JPA2ElementCollection ) {
-				implicitNamingNature = ImplicitJoinColumnNameSource.Nature.ELEMENT_COLLECTION;
-			}
-			else if ( getPropertyHolder().isEntity() ) {
-				implicitNamingNature = ImplicitJoinColumnNameSource.Nature.ENTITY;
-			}
-			else {
-				implicitNamingNature = ImplicitJoinColumnNameSource.Nature.ENTITY_COLLECTION;
-			}
-
-			columnIdentifier = getBuildingContext().getBuildingOptions().getImplicitNamingStrategy().determineJoinColumnName(
 					new ImplicitJoinColumnNameSource() {
+						final ImplicitJoinColumnNameSource.Nature implicitNamingNature = getImplicitNature();
+
 						private final EntityNaming entityNaming = new EntityNaming() {
 							@Override
 							public String getClassName() {
@@ -666,12 +626,8 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 						};
 
 						private final AttributePath attributePath = AttributePath.parse( getPropertyName() );
-						private final Identifier referencedTableName = getBuildingContext().getMetadataCollector()
+						private final Identifier referencedTableName = database.toIdentifier( logicalTableName );
-								.getDatabase()
+						private final Identifier referencedColumnName = database.toIdentifier( logicalReferencedColumn );
-								.toIdentifier( logicalTableName );
-						private final Identifier referencedColumnName = getBuildingContext().getMetadataCollector()
-								.getDatabase()
-								.toIdentifier( logicalReferencedColumn );
 
 						@Override
 						public Nature getNature() {
@@ -707,8 +663,10 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 
 			// HHH-11826 magic. See Ejb3Column and the HHH-6005 comments
 			if ( columnIdentifier.getText().contains( "_collection&&element_" ) ) {
-				columnIdentifier = Identifier.toIdentifier( columnIdentifier.getText().replace( "_collection&&element_", "_" ),
+				columnIdentifier = Identifier.toIdentifier(
-														columnIdentifier.isQuoted() );
+						columnIdentifier.getText().replace( "_collection&&element_", "_" ),
+						columnIdentifier.isQuoted()
+				);
 			}
 
 			//one element was quoted so we quote
@@ -718,7 +676,7 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 		}
 		else {
 			final Identifier logicalTableName = database.toIdentifier(
-					getBuildingContext().getMetadataCollector().getLogicalTableName( referencedEntity.getTable() )
+					metadataCollector.getLogicalTableName( referencedEntity.getTable() )
 			);
 
 			// is an intra-entity hierarchy table join so copy the name by default
@@ -750,6 +708,18 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 				.render( database.getJdbcEnvironment().getDialect() );
 	}
 
+	private ImplicitJoinColumnNameSource.Nature getImplicitNature() {
+		if ( getPropertyHolder().isEntity() ) {
+			return ImplicitJoinColumnNameSource.Nature.ENTITY;
+		}
+		else if ( JPA2ElementCollection ) {
+			return ImplicitJoinColumnNameSource.Nature.ELEMENT_COLLECTION;
+		}
+		else {
+			return ImplicitJoinColumnNameSource.Nature.ENTITY_COLLECTION;
+		}
+	}
+
 	/**
 	 * used for mappedBy cases
 	 */
@@ -820,17 +790,14 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 		boolean noReferencedColumn = true;
 		//build the list of potential tables
 		if ( columns.length == 0 ) return NO_REFERENCE; //shortcut
-		Object columnOwner = BinderHelper.findColumnOwner(
+		Object columnOwner = findColumnOwner( referencedEntity, columns[0].getReferencedColumn(), context );
-				referencedEntity,
-				columns[0].getReferencedColumn(),
-				context
-		);
 		if ( columnOwner == null ) {
 			try {
 				throw new MappingException(
 						"Unable to find column with logical name: "
-								+ columns[0].getReferencedColumn() + " in " + referencedEntity.getTable() + " and its related "
+								+ columns[0].getReferencedColumn()
-								+ "supertables and secondary tables"
+								+ " in " + referencedEntity.getTable()
+								+ " and its related supertables and secondary tables"
 				);
 			}
 			catch (MappingException e) {
@@ -889,7 +856,7 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 	 */
 	public void overrideFromReferencedColumnIfNecessary(Column column) {
 		Column mappingColumn = getMappingColumn();
-		if (mappingColumn != null) {
+		if ( mappingColumn != null ) {
 			// columnDefinition can also be specified using @JoinColumn, hence we have to check
 			// whether it is set or not
 			if ( StringHelper.isEmpty( sqlType ) ) {
@@ -924,16 +891,10 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 			currentJoinColumn.setPropertyHolder( propertyHolder );
 			currentJoinColumn.setJoins( secondaryTables );
 			currentJoinColumn.setBuildingContext( buildingContext );
-			currentJoinColumn.setPropertyName(
+			currentJoinColumn.setPropertyName( getRelativePath( propertyHolder, propertyName ) );
-					BinderHelper.getRelativePath( propertyHolder, propertyName )
-			);
 			currentJoinColumn.setMappedBy( mappedBy );
 			currentJoinColumn.bind();
-
+			joinColumns = new AnnotatedJoinColumn[] { currentJoinColumn };
-			joinColumns = new AnnotatedJoinColumn[] {
-					currentJoinColumn
-
-			};
 		}
 		else {
 			joinColumns = new AnnotatedJoinColumn[annJoins.length];
@@ -946,7 +907,7 @@ public class AnnotatedJoinColumn extends AnnotatedColumn {
 				currentJoinColumn.setPropertyHolder( propertyHolder );
 				currentJoinColumn.setJoins( secondaryTables );
 				currentJoinColumn.setBuildingContext( buildingContext );
-				currentJoinColumn.setPropertyName( BinderHelper.getRelativePath( propertyHolder, propertyName ) );
+				currentJoinColumn.setPropertyName( getRelativePath( propertyHolder, propertyName ) );
 				currentJoinColumn.setMappedBy( mappedBy );
 				currentJoinColumn.setJoinAnnotation( annJoin, propertyName );
 				currentJoinColumn.setNullable( false ); //I break the spec, but it's for good

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

@@ -26,6 +26,8 @@ import java.util.Map;
 import java.util.Set;
 import java.util.stream.Stream;
 
+import jakarta.persistence.SecondaryTable;
+import jakarta.persistence.SecondaryTables;
 import org.hibernate.AnnotationException;
 import org.hibernate.AssertionFailure;
 import org.hibernate.FetchMode;
@@ -229,9 +231,7 @@ import static org.hibernate.cfg.AnnotatedColumn.buildFormulaFromAnnotation;
 import static org.hibernate.cfg.AnnotatedDiscriminatorColumn.buildDiscriminatorColumn;
 import static org.hibernate.cfg.AnnotatedJoinColumn.buildJoinColumnsWithDefaultColumnSuffix;
 import static org.hibernate.cfg.AnnotatedJoinColumn.buildJoinTableJoinColumns;
-import static org.hibernate.cfg.BinderHelper.getMappedSuperclassOrNull;
+import static org.hibernate.cfg.BinderHelper.*;
-import static org.hibernate.cfg.BinderHelper.getPropertyOverriddenByMapperOrMapsId;
-import static org.hibernate.cfg.BinderHelper.makeIdGenerator;
 import static org.hibernate.cfg.InheritanceState.getInheritanceStateOfSuperEntity;
 import static org.hibernate.cfg.InheritanceState.getSuperclassInheritanceState;
 import static org.hibernate.cfg.PropertyHolderBuilder.buildPropertyHolder;
@@ -584,7 +584,7 @@ public final class AnnotationBinder {
 	 * Bind a class having JSR175 annotations. Subclasses <b>have to</b> be bound after its parent class.
 	 *
 	 * @param clazzToProcess entity to bind as {@code XClass} instance
-	 * @param inheritanceStatePerClass Meta data about the inheritance relationships for all mapped classes
+	 * @param inheritanceStatePerClass Metadata about the inheritance relationships for all mapped classes
 	 *
 	 * @throws MappingException in case there is a configuration error
 	 */
@@ -802,13 +802,17 @@ public final class AnnotationBinder {
 	}
 
 	private static void handleSecondaryTables(XClass clazzToProcess, EntityBinder entityBinder) {
-		jakarta.persistence.SecondaryTable secTabAnn = clazzToProcess.getAnnotation(
+		SecondaryTable secTable = clazzToProcess.getAnnotation( SecondaryTable.class );
-				jakarta.persistence.SecondaryTable.class
+		SecondaryTables secTables = clazzToProcess.getAnnotation( SecondaryTables.class );
-		);
+		if ( secTables != null ) {
-		jakarta.persistence.SecondaryTables secTabsAnn = clazzToProcess.getAnnotation(
+			//loop through it
-				jakarta.persistence.SecondaryTables.class
+			for ( SecondaryTable tab : secTables.value() ) {
-		);
+				entityBinder.addJoin( tab, null, false );
-		entityBinder.firstLevelSecondaryTablesBinding( secTabAnn, secTabsAnn );
+			}
+		}
+		else if ( secTable != null ) {
+			entityBinder.addJoin( secTable, null, false );
+		}
 	}
 
 	private static jakarta.persistence.Table handleClassTable(
@@ -958,7 +962,7 @@ public final class AnnotationBinder {
 
 	private static void handleForeignKeys(XClass clazzToProcess, MetadataBuildingContext context, DependantValue key) {
 		ForeignKey foreignKey = clazzToProcess.getAnnotation( ForeignKey.class );
-		if ( foreignKey != null && !BinderHelper.isEmptyAnnotationValue( foreignKey.name() ) ) {
+		if ( foreignKey != null && !isEmptyAnnotationValue( foreignKey.name() ) ) {
 			key.setForeignKeyName( foreignKey.name() );
 		}
 		else {
@@ -972,7 +976,7 @@ public final class AnnotationBinder {
 			}
 			else if ( pkJoinColumns != null && !StringHelper.isEmpty( pkJoinColumns.foreignKey().name() ) ) {
 				key.setForeignKeyName( pkJoinColumns.foreignKey().name() );
-				if ( !BinderHelper.isEmptyAnnotationValue( pkJoinColumns.foreignKey().foreignKeyDefinition() ) ) {
+				if ( !isEmptyAnnotationValue( pkJoinColumns.foreignKey().foreignKeyDefinition() ) ) {
 					key.setForeignKeyDefinition( pkJoinColumns.foreignKey().foreignKeyDefinition() );
 				}
 			}
@@ -983,7 +987,7 @@ public final class AnnotationBinder {
 			}
 			else if ( pkJoinColumn != null && !StringHelper.isEmpty( pkJoinColumn.foreignKey().name() ) ) {
 				key.setForeignKeyName( pkJoinColumn.foreignKey().name() );
-				if ( !BinderHelper.isEmptyAnnotationValue( pkJoinColumn.foreignKey().foreignKeyDefinition() ) ) {
+				if ( !isEmptyAnnotationValue( pkJoinColumn.foreignKey().foreignKeyDefinition() ) ) {
 					key.setForeignKeyDefinition( pkJoinColumn.foreignKey().foreignKeyDefinition() );
 				}
 			}
@@ -1173,8 +1177,8 @@ public final class AnnotationBinder {
 		final ConverterRegistrations plural = container.getAnnotation( ConverterRegistrations.class );
 		if ( plural != null ) {
 			final ConverterRegistration[] registrations = plural.value();
-			for ( int i = 0; i < registrations.length; i++ ) {
+			for (ConverterRegistration registration : registrations) {
-				handleConverterRegistration( registrations[i], context );
+				handleConverterRegistration(registration, context);
 			}
 		}
 	}
@@ -2085,7 +2089,7 @@ public final class AnnotationBinder {
 			else {
 				throw new AnnotationException(
 						"@Parent cannot be applied outside an embeddable object: "
-								+ BinderHelper.getPath( propertyHolder, inferredData )
+								+ getPath( propertyHolder, inferredData )
 				);
 			}
 			return;
@@ -2474,7 +2478,7 @@ public final class AnnotationBinder {
 				&& isToManyAssociationWithinEmbeddableCollection(propertyHolder) ) {
 			throw new AnnotationException(
 					"@OneToMany, @ManyToMany or @ElementCollection cannot be used inside an @Embeddable that is also contained within an @ElementCollection: "
-							+ BinderHelper.getPath(propertyHolder, inferredData)
+							+ getPath(propertyHolder, inferredData)
 			);
 		}
 
@@ -2482,7 +2486,7 @@ public final class AnnotationBinder {
 				&& manyToManyAnn != null && !manyToManyAnn.mappedBy().isEmpty() ) {
 			throw new AnnotationException(
 					"Explicit @OrderColumn on inverse side of @ManyToMany is illegal: "
-							+ BinderHelper.getPath(propertyHolder, inferredData)
+							+ getPath(propertyHolder, inferredData)
 			);
 		}
 
@@ -2516,7 +2520,7 @@ public final class AnnotationBinder {
 		if ( notFound != null ) {
 			if ( manyToManyAnn == null ) {
 				throw new AnnotationException("collection annotated @NotFound is not a @ManyToMany association: "
-						+ BinderHelper.getPath(propertyHolder, inferredData) );
+						+ getPath(propertyHolder, inferredData) );
 			}
 			collectionBinder.setNotFoundAction( notFound.action() );
 		}
@@ -2711,7 +2715,7 @@ public final class AnnotationBinder {
 							Locale.ROOT,
 							"@Columns not allowed on a @Any property [%s]; @Column or @Formula is used to map the discriminator" +
 									"and only one is allowed",
-							BinderHelper.getPath(propertyHolder, inferredData)
+							getPath(propertyHolder, inferredData)
 					)
 			);
 		}
@@ -2747,7 +2751,7 @@ public final class AnnotationBinder {
 				|| property.isAnnotationPresent( Columns.class ) ) {
 			throw new AnnotationException(
 					"@Column(s) not allowed on a @OneToOne property: "
-							+ BinderHelper.getPath(propertyHolder, inferredData)
+							+ getPath(propertyHolder, inferredData)
 			);
 		}
 
@@ -2817,7 +2821,7 @@ public final class AnnotationBinder {
 				|| property.isAnnotationPresent( Columns.class ) ) {
 			throw new AnnotationException(
 					"@Column(s) not allowed on a @ManyToOne property: "
-							+ BinderHelper.getPath(propertyHolder, inferredData)
+							+ getPath(propertyHolder, inferredData)
 			);
 		}
 
@@ -2896,7 +2900,7 @@ public final class AnnotationBinder {
 			if ( property.isAnnotationPresent( MapKeyJoinColumn.class ) ) {
 				throw new AnnotationException(
 						"@MapKeyJoinColumn and @MapKeyJoinColumns used on the same property: "
-								+ BinderHelper.getPath(propertyHolder, inferredData)
+								+ getPath(propertyHolder, inferredData)
 				);
 			}
 		}
@@ -3077,7 +3081,7 @@ public final class AnnotationBinder {
 		if ( isIdentifierMapper ) {
 			throw new AnnotationException(
 					"@IdClass class should not have @Id nor @EmbeddedId properties: "
-							+ BinderHelper.getPath( propertyHolder, inferredData )
+							+ getPath( propertyHolder, inferredData )
 			);
 		}
 		XClass entityXClass = inferredData.getClassOrElement();
@@ -3216,13 +3220,13 @@ public final class AnnotationBinder {
 			if ( jpaIndexes != null && jpaIndexes.length > 0 ) {
 				associationTableBinder.setJpaIndex( jpaIndexes );
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( schema ) ) {
+			if ( !isEmptyAnnotationValue( schema ) ) {
 				associationTableBinder.setSchema( schema );
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( catalog ) ) {
+			if ( !isEmptyAnnotationValue( catalog ) ) {
 				associationTableBinder.setCatalog( catalog );
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( tableName ) ) {
+			if ( !isEmptyAnnotationValue( tableName ) ) {
 				associationTableBinder.setName( tableName );
 			}
 			associationTableBinder.setUniqueConstraints( uniqueConstraints );
@@ -3311,14 +3315,14 @@ public final class AnnotationBinder {
 				throw new AnnotationException(
 						comp.getComponentClassName()
 								+ " must not have @Id properties when used as an @EmbeddedId: "
-								+ BinderHelper.getPath( propertyHolder, inferredData )
+								+ getPath( propertyHolder, inferredData )
 				);
 			}
 			if ( referencedEntityName == null && comp.getPropertySpan() == 0 ) {
 				throw new AnnotationException(
 						comp.getComponentClassName()
 								+ " has no persistent id property: "
-								+ BinderHelper.getPath( propertyHolder, inferredData )
+								+ getPath( propertyHolder, inferredData )
 				);
 			}
 		}
@@ -3396,7 +3400,7 @@ public final class AnnotationBinder {
 				buildingContext
 		);
 
-		String subpath = BinderHelper.getPath( propertyHolder, inferredData );
+		String subpath = getPath( propertyHolder, inferredData );
 		LOG.tracev( "Binding component with path: {0}", subpath );
 		PropertyHolder subHolder = buildPropertyHolder(
 				comp,
@@ -3738,7 +3742,7 @@ public final class AnnotationBinder {
 				}
 
 				if ( property.isAnnotationPresent( ManyToOne.class ) && joinColumn != null
-						&& ! BinderHelper.isEmptyAnnotationValue( joinColumn.name() )
+						&& ! isEmptyAnnotationValue( joinColumn.name() )
 						&& joinColumn.name().equals( columnName )
 						&& !property.isAnnotationPresent( MapsId.class ) ) {
 					hasSpecjManyToOne = true;
@@ -3823,31 +3827,16 @@ public final class AnnotationBinder {
 	}
 
 	static void defineFetchingStrategy(ToOne toOne, XProperty property) {
+		final FetchType fetchType = getJpaFetchType( property );
+
 		LazyToOne lazy = property.getAnnotation( LazyToOne.class );
-		Fetch fetch = property.getAnnotation( Fetch.class );
-		ManyToOne manyToOne = property.getAnnotation( ManyToOne.class );
-		OneToOne oneToOne = property.getAnnotation( OneToOne.class );
 		NotFound notFound = property.getAnnotation( NotFound.class );
-
-		FetchType fetchType;
-		if ( manyToOne != null ) {
-			fetchType = manyToOne.fetch();
-		}
-		else if ( oneToOne != null ) {
-			fetchType = oneToOne.fetch();
-		}
-		else {
-			throw new AssertionFailure(
-					"Define fetch strategy on a property not annotated with @OneToMany nor @OneToOne"
-			);
-		}
-
 		if ( notFound != null ) {
 			toOne.setLazy( false );
 			toOne.setUnwrapProxy( true );
 		}
 		else if ( lazy != null ) {
-			toOne.setLazy( !( lazy.value() == LazyToOneOption.FALSE ) );
+			toOne.setLazy( lazy.value() != LazyToOneOption.FALSE );
 			toOne.setUnwrapProxy( ( lazy.value() == LazyToOneOption.NO_PROXY ) );
 		}
 		else {
@@ -3856,7 +3845,9 @@ public final class AnnotationBinder {
 			toOne.setUnwrapProxyImplicit( true );
 		}
 
+		Fetch fetch = property.getAnnotation( Fetch.class );
 		if ( fetch != null ) {
+			// Hibernate @Fetch annotation takes precedence
 			if ( fetch.value() == org.hibernate.annotations.FetchMode.JOIN ) {
 				toOne.setFetchMode( FetchMode.JOIN );
 				toOne.setLazy( false );
@@ -3866,7 +3857,8 @@ public final class AnnotationBinder {
 				toOne.setFetchMode( FetchMode.SELECT );
 			}
 			else if ( fetch.value() == org.hibernate.annotations.FetchMode.SUBSELECT ) {
-				throw new AnnotationException( "Use of FetchMode.SUBSELECT not allowed on ToOne associations" );
+				throw new AnnotationException( "Use of FetchMode.SUBSELECT not allowed for to-one associations: "
+						+ property.getName() );
 			}
 			else {
 				throw new AssertionFailure( "Unknown FetchMode: " + fetch.value() );
@@ -3877,6 +3869,20 @@ public final class AnnotationBinder {
 		}
 	}
 
+	private static FetchType getJpaFetchType(XProperty property) {
+		ManyToOne manyToOne = property.getAnnotation( ManyToOne.class );
+		OneToOne oneToOne = property.getAnnotation( OneToOne.class );
+		if ( manyToOne != null ) {
+			return manyToOne.fetch();
+		}
+		else if ( oneToOne != null ) {
+			return oneToOne.fetch();
+		}
+		else {
+			throw new AssertionFailure("Define fetch strategy on a property not annotated with @OneToMany nor @OneToOne");
+		}
+	}
+
 	private static void bindOneToOne(
 			String cascadeStrategy,
 			AnnotatedJoinColumn[] joinColumns,
@@ -3896,34 +3902,7 @@ public final class AnnotationBinder {
 		//column.getTable() => persistentClass.getTable()
 		final String propertyName = inferredData.getPropertyName();
 		LOG.tracev( "Fetching {0} with {1}", propertyName, fetchMode );
-		boolean mapToPK = true;
+		if ( isMapToPK( joinColumns, propertyHolder, trueOneToOne ) || !isEmptyAnnotationValue( mappedBy ) ) {
-		if ( !trueOneToOne ) {
-			//try to find a hidden true one to one (FK == PK columns)
-			KeyValue identifier = propertyHolder.getIdentifier();
-			if ( identifier == null ) {
-				//this is a @OneToOne in an @EmbeddedId (the persistentClass.identifier is not set yet, it's being built)
-				//by definition the PK cannot refer to itself so it cannot map to itself
-				mapToPK = false;
-			}
-			else {
-				List<String> idColumnNames = new ArrayList<>();
-				if ( identifier.getColumnSpan() != joinColumns.length ) {
-					mapToPK = false;
-				}
-				else {
-					for ( org.hibernate.mapping.Column currentColumn : identifier.getColumns() ) {
-						idColumnNames.add( currentColumn.getName() );
-					}
-					for ( AnnotatedJoinColumn col : joinColumns ) {
-						if ( !idColumnNames.contains( col.getMappingColumn().getName() ) ) {
-							mapToPK = false;
-							break;
-						}
-					}
-				}
-			}
-		}
-		if ( trueOneToOne || mapToPK || !BinderHelper.isEmptyAnnotationValue( mappedBy ) ) {
 			//is a true one-to-one
 			//FIXME referencedColumnName ignored => ordering may fail.
 			OneToOneSecondPass secondPass = new OneToOneSecondPass(
@@ -3944,10 +3923,7 @@ public final class AnnotationBinder {
 				secondPass.doSecondPass( context.getMetadataCollector().getEntityBindingMap() );
 			}
 			else {
-				context.getMetadataCollector().addSecondPass(
+				context.getMetadataCollector().addSecondPass( secondPass, isEmptyAnnotationValue( mappedBy ) );
-						secondPass,
-						BinderHelper.isEmptyAnnotationValue( mappedBy )
-				);
 			}
 		}
 		else {
@@ -3961,6 +3937,38 @@ public final class AnnotationBinder {
 		}
 	}
 
+	private static boolean isMapToPK(AnnotatedJoinColumn[] joinColumns, PropertyHolder propertyHolder, boolean trueOneToOne) {
+		if ( trueOneToOne ) {
+			return true;
+		}
+		else {
+			//try to find a hidden true one to one (FK == PK columns)
+			KeyValue identifier = propertyHolder.getIdentifier();
+			if ( identifier == null ) {
+				//this is a @OneToOne in an @EmbeddedId (the persistentClass.identifier is not set yet, it's being built)
+				//by definition the PK cannot refer to itself so it cannot map to itself
+				return false;
+			}
+			else {
+				List<String> idColumnNames = new ArrayList<>();
+				if ( identifier.getColumnSpan() != joinColumns.length ) {
+					return false;
+				}
+				else {
+					for ( org.hibernate.mapping.Column currentColumn: identifier.getColumns() ) {
+						idColumnNames.add( currentColumn.getName() );
+					}
+					for ( AnnotatedJoinColumn col: joinColumns) {
+						if ( !idColumnNames.contains( col.getMappingColumn().getName() ) ) {
+							return false;
+						}
+					}
+					return true;
+				}
+			}
+		}
+	}
+
 	private static void bindAny(
 			String cascadeStrategy,
 			AnnotatedJoinColumn[] columns,
@@ -3971,23 +3979,16 @@ public final class AnnotationBinder {
 			EntityBinder entityBinder,
 			boolean isIdentifierMapper,
 			MetadataBuildingContext buildingContext) {
-		XProperty property = inferredData.getProperty();
+		final XProperty property = inferredData.getProperty();
-		org.hibernate.annotations.Any anyAnn = property
+		final org.hibernate.annotations.Any any = property.getAnnotation( org.hibernate.annotations.Any.class );
-				.getAnnotation( org.hibernate.annotations.Any.class );
+		if ( any == null ) {
-		if ( anyAnn == null ) {
+			throw new AssertionFailure( "Missing @Any annotation: " + getPath( propertyHolder, inferredData ) );
-			throw new AssertionFailure(
-					"Missing @Any annotation: "
-							+ BinderHelper.getPath( propertyHolder, inferredData )
-			);
 		}
 
-		final Column discriminatorColumnAnn = property.getAnnotation( Column.class );
+		final boolean lazy = any.fetch() == FetchType.LAZY;
-		final Formula discriminatorFormulaAnn = getOverridableAnnotation( property, Formula.class, buildingContext );
+		final Any value = BinderHelper.buildAnyValue(
-
+				property.getAnnotation( Column.class ),
-		boolean lazy = ( anyAnn.fetch() == FetchType.LAZY );
+				getOverridableAnnotation( property, Formula.class, buildingContext ),
-		Any value = BinderHelper.buildAnyValue(
-				discriminatorColumnAnn,
-				discriminatorFormulaAnn,
 				columns,
 				inferredData,
 				cascadeOnDelete,
@@ -3995,11 +3996,11 @@ public final class AnnotationBinder {
 				nullability,
 				propertyHolder,
 				entityBinder,
-				anyAnn.optional(),
+				any.optional(),
 				buildingContext
 		);
 
-		PropertyBinder binder = new PropertyBinder();
+		final PropertyBinder binder = new PropertyBinder();
 		binder.setName( inferredData.getPropertyName() );
 		binder.setValue( value );
 
@@ -4021,33 +4022,32 @@ public final class AnnotationBinder {
 	}
 
 	private static EnumSet<CascadeType> convertToHibernateCascadeType(jakarta.persistence.CascadeType[] ejbCascades) {
-		EnumSet<CascadeType> hibernateCascadeSet = EnumSet.noneOf( CascadeType.class );
+		final EnumSet<CascadeType> cascadeTypes = EnumSet.noneOf( CascadeType.class );
 		if ( ejbCascades != null && ejbCascades.length > 0 ) {
-			for ( jakarta.persistence.CascadeType cascade : ejbCascades ) {
+			for ( jakarta.persistence.CascadeType cascade: ejbCascades ) {
-				switch ( cascade ) {
+				cascadeTypes.add( convertCascadeType( cascade ) );
-					case ALL:
-						hibernateCascadeSet.add( CascadeType.ALL );
-						break;
-					case PERSIST:
-						hibernateCascadeSet.add( CascadeType.PERSIST );
-						break;
-					case MERGE:
-						hibernateCascadeSet.add( CascadeType.MERGE );
-						break;
-					case REMOVE:
-						hibernateCascadeSet.add( CascadeType.REMOVE );
-						break;
-					case REFRESH:
-						hibernateCascadeSet.add( CascadeType.REFRESH );
-						break;
-					case DETACH:
-						hibernateCascadeSet.add( CascadeType.DETACH );
-						break;
-				}
 			}
 		}
+		return cascadeTypes;
+	}
 
-		return hibernateCascadeSet;
+	private static CascadeType convertCascadeType(jakarta.persistence.CascadeType cascade) {
+		switch (cascade) {
+			case ALL:
+				return CascadeType.ALL;
+			case PERSIST:
+				return CascadeType.PERSIST;
+			case MERGE:
+				return CascadeType.MERGE;
+			case REMOVE:
+				return CascadeType.REMOVE;
+			case REFRESH:
+				return CascadeType.REFRESH;
+			case DETACH:
+				return CascadeType.DETACH;
+			default:
+				throw new AssertionFailure("unknown cascade type: " + cascade);
+		}
 	}
 
 	private static String getCascadeStrategy(
@@ -4055,26 +4055,25 @@ public final class AnnotationBinder {
 			Cascade hibernateCascadeAnnotation,
 			boolean orphanRemoval,
 			boolean forcePersist) {
-		EnumSet<CascadeType> hibernateCascadeSet = convertToHibernateCascadeType( ejbCascades );
+		EnumSet<CascadeType> cascadeTypes = convertToHibernateCascadeType( ejbCascades );
-		CascadeType[] hibernateCascades = hibernateCascadeAnnotation == null ?
+		CascadeType[] hibernateCascades = hibernateCascadeAnnotation == null ? null : hibernateCascadeAnnotation.value();
-				null :
-				hibernateCascadeAnnotation.value();
-
 		if ( hibernateCascades != null && hibernateCascades.length > 0 ) {
-			hibernateCascadeSet.addAll( Arrays.asList( hibernateCascades ) );
+			cascadeTypes.addAll( Arrays.asList( hibernateCascades ) );
 		}
-
 		if ( orphanRemoval ) {
-			hibernateCascadeSet.add( CascadeType.DELETE_ORPHAN );
+			cascadeTypes.add( CascadeType.DELETE_ORPHAN );
-			hibernateCascadeSet.add( CascadeType.REMOVE );
+			cascadeTypes.add( CascadeType.REMOVE );
 		}
 		if ( forcePersist ) {
-			hibernateCascadeSet.add( CascadeType.PERSIST );
+			cascadeTypes.add( CascadeType.PERSIST );
 		}
+		return renderCascadeTypeList( cascadeTypes );
+	}
 
+	private static String renderCascadeTypeList(EnumSet<CascadeType> cascadeTypes) {
 		StringBuilder cascade = new StringBuilder();
-		for ( CascadeType aHibernateCascadeSet : hibernateCascadeSet ) {
+		for ( CascadeType cascadeType : cascadeTypes) {
-			switch ( aHibernateCascadeSet ) {
+			switch ( cascadeType ) {
 				case ALL:
 					cascade.append( "," ).append( "all" );
 					break;
@@ -4108,9 +4107,7 @@ public final class AnnotationBinder {
 					break;
 			}
 		}
-		return cascade.length() > 0 ?
+		return cascade.length() > 0 ? cascade.substring( 1 ) : "none";
-				cascade.substring( 1 ) :
-				"none";
 	}
 
 	public static FetchMode getFetchMode(FetchType fetch) {
@@ -4124,55 +4121,54 @@ public final class AnnotationBinder {
 			JoinColumn joinColumn,
 			JoinColumns joinColumns,
 			MetadataBuildingContext context) {
-		final boolean noConstraintByDefault = context.getBuildingOptions().isNoConstraintByDefault();
+		if ( property.getAnnotation( NotFound.class ) != null ) {
-		final NotFound notFoundAnn= property.getAnnotation( NotFound.class );
-
-		if ( notFoundAnn != null ) {
 			// supersedes all others
 			value.disableForeignKey();
 		}
-		else if ( joinColumn != null && (
-				joinColumn.foreignKey().value() == ConstraintMode.NO_CONSTRAINT
-						|| ( joinColumn.foreignKey().value() == ConstraintMode.PROVIDER_DEFAULT && noConstraintByDefault ) ) ) {
-			value.disableForeignKey();
-		}
-		else if ( joinColumns != null && (
-				joinColumns.foreignKey().value() == ConstraintMode.NO_CONSTRAINT
-						|| ( joinColumns.foreignKey().value() == ConstraintMode.PROVIDER_DEFAULT && noConstraintByDefault ) ) ) {
-			value.disableForeignKey();
-		}
 		else {
-			final ForeignKey fk = property.getAnnotation( ForeignKey.class );
+			if ( joinColumn!=null && noConstraint( joinColumn.foreignKey(), context )
-			if ( fk != null && StringHelper.isNotEmpty( fk.name() ) ) {
+					|| joinColumns!=null && noConstraint( joinColumns.foreignKey(), context ) ) {
-				value.setForeignKeyName( fk.name() );
+				value.disableForeignKey();
 			}
 			else {
-				if ( fkOverride != null && ( fkOverride.value() == ConstraintMode.NO_CONSTRAINT
+				final ForeignKey fk = property.getAnnotation( ForeignKey.class );
-						|| fkOverride.value() == ConstraintMode.PROVIDER_DEFAULT && noConstraintByDefault ) ) {
+				if ( fk != null && StringHelper.isNotEmpty( fk.name() ) ) {
-					value.disableForeignKey();
+					value.setForeignKeyName( fk.name() );
 				}
-				else if ( fkOverride != null ) {
+				else {
-					value.setForeignKeyName( nullIfEmpty( fkOverride.name() ) );
+					if ( noConstraint( fkOverride, context) ) {
-					value.setForeignKeyDefinition( nullIfEmpty( fkOverride.foreignKeyDefinition() ) );
+						value.disableForeignKey();
-				}
+					}
-				else if ( joinColumns != null ) {
+					else if ( fkOverride != null ) {
-					value.setForeignKeyName( nullIfEmpty( joinColumns.foreignKey().name() ) );
+						value.setForeignKeyName( nullIfEmpty( fkOverride.name() ) );
-					value.setForeignKeyDefinition( nullIfEmpty( joinColumns.foreignKey().foreignKeyDefinition() ) );
+						value.setForeignKeyDefinition( nullIfEmpty( fkOverride.foreignKeyDefinition() ) );
-				}
+					}
-				else if ( joinColumn != null ) {
+					else if ( joinColumns != null ) {
-					value.setForeignKeyName( nullIfEmpty( joinColumn.foreignKey().name() ) );
+						value.setForeignKeyName( nullIfEmpty( joinColumns.foreignKey().name() ) );
-					value.setForeignKeyDefinition( nullIfEmpty( joinColumn.foreignKey().foreignKeyDefinition() ) );
+						value.setForeignKeyDefinition( nullIfEmpty( joinColumns.foreignKey().foreignKeyDefinition() ) );
+					}
+					else if ( joinColumn != null ) {
+						value.setForeignKeyName( nullIfEmpty( joinColumn.foreignKey().name() ) );
+						value.setForeignKeyDefinition( nullIfEmpty( joinColumn.foreignKey().foreignKeyDefinition() ) );
+					}
 				}
 			}
 		}
 	}
 
+	private static boolean noConstraint(jakarta.persistence.ForeignKey joinColumns, MetadataBuildingContext context) {
+		return joinColumns != null
+			&& ( joinColumns.value() == ConstraintMode.NO_CONSTRAINT
+				|| joinColumns.value() == ConstraintMode.PROVIDER_DEFAULT
+						&& context.getBuildingOptions().isNoConstraintByDefault() );
+	}
+
 	private static HashMap<String, IdentifierGeneratorDefinition> buildGenerators(
 			XAnnotatedElement annElt,
 			MetadataBuildingContext context) {
 
-		InFlightMetadataCollector metadataCollector = context.getMetadataCollector();
+		final InFlightMetadataCollector metadataCollector = context.getMetadataCollector();
-		HashMap<String, IdentifierGeneratorDefinition> generators = new HashMap<>();
+		final HashMap<String, IdentifierGeneratorDefinition> generators = new HashMap<>();
 
 		TableGenerators tableGenerators = annElt.getAnnotation( TableGenerators.class );
 		if ( tableGenerators != null ) {
@@ -4205,24 +4201,27 @@ public final class AnnotationBinder {
 		}
 
 		TableGenerator tabGen = annElt.getAnnotation( TableGenerator.class );
-		SequenceGenerator seqGen = annElt.getAnnotation( SequenceGenerator.class );
-		GenericGenerator genGen = annElt.getAnnotation( GenericGenerator.class );
 		if ( tabGen != null ) {
 			IdentifierGeneratorDefinition idGen = buildIdGenerator( tabGen, context );
 			generators.put( idGen.getName(), idGen );
 			metadataCollector.addIdentifierGenerator( idGen );
 
 		}
+
+		SequenceGenerator seqGen = annElt.getAnnotation( SequenceGenerator.class );
 		if ( seqGen != null ) {
 			IdentifierGeneratorDefinition idGen = buildIdGenerator( seqGen, context );
 			generators.put( idGen.getName(), idGen );
 			metadataCollector.addIdentifierGenerator( idGen );
 		}
+
+		GenericGenerator genGen = annElt.getAnnotation( GenericGenerator.class );
 		if ( genGen != null ) {
 			IdentifierGeneratorDefinition idGen = buildIdGenerator( genGen, context );
 			generators.put( idGen.getName(), idGen );
 			metadataCollector.addIdentifierGenerator( idGen );
 		}
+
 		return generators;
 	}
 
@@ -4243,8 +4242,8 @@ public final class AnnotationBinder {
 			MetadataBuildingContext buildingContext) {
 		Map<XClass, InheritanceState> inheritanceStatePerClass = new HashMap<>( orderedClasses.size() );
 		for ( XClass clazz : orderedClasses ) {
-			InheritanceState superclassState = getSuperclassInheritanceState( clazz, inheritanceStatePerClass );
+			final InheritanceState superclassState = getSuperclassInheritanceState( clazz, inheritanceStatePerClass );
-			InheritanceState state = new InheritanceState( clazz, inheritanceStatePerClass, buildingContext );
+			final InheritanceState state = new InheritanceState( clazz, inheritanceStatePerClass, buildingContext );
 			if ( superclassState != null ) {
 				//the classes are ordered thus preventing an NPE
 				//FIXME if an entity has subclasses annotated @MappedSuperclass wo sub @Entity this is wrong
@@ -4272,9 +4271,6 @@ public final class AnnotationBinder {
 	}
 
 	private static boolean hasAnnotationsOnIdClass(XClass idClass) {
-//		if(idClass.getAnnotation(Embeddable.class) != null)
-//			return true;
-
 		for ( XProperty property : idClass.getDeclaredProperties( XClass.ACCESS_FIELD ) ) {
 			if ( hasTriggeringAnnotation(property) ) {
 				return true;

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

@@ -152,7 +152,7 @@ public class BinderHelper {
 				embeddedComp.setEmbedded( true );
 				embeddedComp.setComponentClassName( embeddedComp.getOwner().getClassName() );
 				for (Property property : properties) {
-					Property clone = BinderHelper.shallowCopy( property );
+					Property clone = shallowCopy( property );
 					clone.setInsertable( false );
 					clone.setUpdateable( false );
 					clone.setNaturalIdentifier( false );

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

@@ -30,12 +30,12 @@ public abstract class CollectionSecondPass implements SecondPass {
 
 	private static final CoreMessageLogger LOG = Logger.getMessageLogger(CoreMessageLogger.class, CollectionSecondPass.class.getName());
 
-	MetadataBuildingContext buildingContext;
+//	MetadataBuildingContext buildingContext;
-	Collection collection;
+	private final Collection collection;
 
 	public CollectionSecondPass(MetadataBuildingContext buildingContext, Collection collection) {
 		this.collection = collection;
-		this.buildingContext = buildingContext;
+//		this.buildingContext = buildingContext;
 	}
 
 	public void doSecondPass(Map<String, PersistentClass> persistentClasses)
@@ -49,8 +49,9 @@ public abstract class CollectionSecondPass implements SecondPass {
 
 		if ( LOG.isDebugEnabled() ) {
 			String msg = "Mapped collection key: " + columns( collection.getKey() );
-			if ( collection.isIndexed() )
+			if ( collection.isIndexed() ) {
 				msg += ", index: " + columns( ( (IndexedCollection) collection ).getIndex() );
+			}
 			if ( collection.isOneToMany() ) {
 				msg += ", one-to-many: "
 					+ ( (OneToMany) collection.getElement() ).getReferencedEntityName();

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

@@ -33,6 +33,8 @@ import static org.hibernate.cfg.AnnotatedColumn.buildColumnFromNoAnnotation;
 import static org.hibernate.cfg.AnnotatedColumn.buildColumnsFromAnnotations;
 import static org.hibernate.cfg.AnnotatedColumn.buildFormulaFromAnnotation;
 import static org.hibernate.cfg.AnnotationBinder.getOverridableAnnotation;
+import static org.hibernate.cfg.BinderHelper.getPath;
+import static org.hibernate.cfg.BinderHelper.getPropertyOverriddenByMapperOrMapsId;
 
 /**
  * Do the initial discovery of columns metadata and apply defaults.
@@ -139,7 +141,7 @@ class ColumnsBuilder {
 		}
 		else if ( joinColumns == null && property.isAnnotationPresent( org.hibernate.annotations.Any.class ) ) {
 			throw new AnnotationException( "@Any requires an explicit @JoinColumn(s): "
-					+ BinderHelper.getPath( propertyHolder, inferredData ) );
+					+ getPath( propertyHolder, inferredData ) );
 		}
 		if ( columns == null && !property.isAnnotationPresent( ManyToMany.class ) ) {
 			//useful for collection of embedded elements
@@ -179,7 +181,7 @@ class ColumnsBuilder {
 			if ( StringHelper.isEmpty( joinTableAnn.name() ) ) {
 				throw new AnnotationException(
 						"JoinTable.name() on a @ToOne association has to be explicit: "
-								+ BinderHelper.getPath( propertyHolder, inferredData )
+								+ getPath( propertyHolder, inferredData )
 				);
 			}
 		}
@@ -275,7 +277,7 @@ class ColumnsBuilder {
 
 	AnnotatedColumn[] overrideColumnFromMapperOrMapsIdProperty(boolean isId) {
 		AnnotatedColumn[] result = columns;
-		final PropertyData overridingProperty = BinderHelper.getPropertyOverriddenByMapperOrMapsId(
+		final PropertyData overridingProperty = getPropertyOverriddenByMapperOrMapsId(
 				isId,
 				propertyHolder,
 				property.getName(),

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

@@ -28,6 +28,9 @@ import org.hibernate.mapping.SimpleValue;
 
 import org.jboss.logging.Logger;
 
+import static org.hibernate.cfg.BinderHelper.isEmptyAnnotationValue;
+import static org.hibernate.internal.util.collections.CollectionHelper.mapOfSize;
+
 /**
  * @author Emmanuel Bernard
  */
@@ -79,13 +82,13 @@ public class CopyIdentifierComponentSecondPass extends FkSecondPass {
 
 		//prepare column name structure
 		boolean isExplicitReference = true;
-		Map<String, AnnotatedJoinColumn> columnByReferencedName = CollectionHelper.mapOfSize( joinColumns.length);
+		Map<String, AnnotatedJoinColumn> columnByReferencedName = mapOfSize( joinColumns.length);
 		for (AnnotatedJoinColumn joinColumn : joinColumns) {
 			final String referencedColumnName = joinColumn.getReferencedColumn();
-			if ( referencedColumnName == null || BinderHelper.isEmptyAnnotationValue( referencedColumnName ) ) {
+			if ( referencedColumnName == null || isEmptyAnnotationValue( referencedColumnName ) ) {
 				break;
 			}
-			//JPA 2 requires referencedColumnNames to be case insensitive
+			//JPA 2 requires referencedColumnNames to be case-insensitive
 			columnByReferencedName.put( referencedColumnName.toLowerCase(Locale.ROOT), joinColumn );
 		}
 		//try default column orientation

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

@@ -15,6 +15,8 @@ import org.hibernate.boot.spi.MetadataBuildingContext;
 import org.hibernate.mapping.PersistentClass;
 import org.hibernate.mapping.SimpleValue;
 
+import static org.hibernate.cfg.BinderHelper.makeIdGenerator;
+
 /**
  * @author Andrea Boriero
  */
@@ -52,6 +54,6 @@ public class IdGeneratorResolverSecondPass implements SecondPass {
 
 	@Override
 	public void doSecondPass(Map<String, PersistentClass> idGeneratorDefinitionMap) throws MappingException {
-		BinderHelper.makeIdGenerator( id, idXProperty, generatorType, generatorName, buildingContext, localIdentifierGeneratorDefinition );
+		makeIdGenerator( id, idXProperty, generatorType, generatorName, buildingContext, localIdentifierGeneratorDefinition );
 	}
 }

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

@@ -12,6 +12,8 @@ import org.hibernate.annotations.ListIndexBase;
 import org.hibernate.boot.spi.MetadataBuildingContext;
 import org.hibernate.mapping.Join;
 
+import static org.hibernate.cfg.BinderHelper.isEmptyAnnotationValue;
+
 /**
  * index column
  *
@@ -134,8 +136,8 @@ public class IndexColumn extends AnnotatedColumn {
 			MetadataBuildingContext buildingContext) {
 		final IndexColumn column;
 		if ( ann != null ) {
-			final String sqlType = BinderHelper.isEmptyAnnotationValue( ann.columnDefinition() ) ? null : ann.columnDefinition();
+			final String sqlType = isEmptyAnnotationValue( ann.columnDefinition() ) ? null : ann.columnDefinition();
-			final String name = BinderHelper.isEmptyAnnotationValue( ann.name() ) ? inferredData.getPropertyName() + "_ORDER" : ann.name();
+			final String name = isEmptyAnnotationValue( ann.name() ) ? inferredData.getPropertyName() + "_ORDER" : ann.name();
 			//TODO move it to a getter based system and remove the constructor
 // The JPA OrderColumn annotation defines no table element...
 //			column = new IndexColumn(
@@ -198,8 +200,8 @@ public class IndexColumn extends AnnotatedColumn {
 			MetadataBuildingContext buildingContext) {
 		final IndexColumn column;
 		if ( ann != null ) {
-			final String sqlType = BinderHelper.isEmptyAnnotationValue( ann.columnDefinition() ) ? null : ann.columnDefinition();
+			final String sqlType = isEmptyAnnotationValue( ann.columnDefinition() ) ? null : ann.columnDefinition();
-			final String name = BinderHelper.isEmptyAnnotationValue( ann.name() ) ? inferredData.getPropertyName() : ann.name();
+			final String name = isEmptyAnnotationValue( ann.name() ) ? inferredData.getPropertyName() : ann.name();
 			//TODO move it to a getter based system and remove the constructor
 			column = new IndexColumn(
 					false,

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

@@ -30,6 +30,10 @@ import org.hibernate.mapping.Property;
 import org.hibernate.mapping.SortableValue;
 import org.hibernate.type.ForeignKeyDirection;
 
+import static org.hibernate.cfg.BinderHelper.findPropertyByName;
+import static org.hibernate.cfg.BinderHelper.isEmptyAnnotationValue;
+import static org.hibernate.internal.util.StringHelper.qualify;
+
 /**
  * We have to handle OneToOne in a second pass because:
  * -
@@ -93,7 +97,7 @@ public class OneToOneSecondPass implements SecondPass {
 		//value.setLazy( fetchMode != FetchMode.JOIN );
 
 		value.setConstrained( !optional );
-		final ForeignKeyDirection foreignKeyDirection = !BinderHelper.isEmptyAnnotationValue( mappedBy )
+		final ForeignKeyDirection foreignKeyDirection = !isEmptyAnnotationValue( mappedBy )
 				? ForeignKeyDirection.TO_PARENT
 				: ForeignKeyDirection.FROM_PARENT;
 		value.setForeignKeyType(foreignKeyDirection);
@@ -120,7 +124,7 @@ public class OneToOneSecondPass implements SecondPass {
 
 		Property prop = binder.makeProperty();
 		prop.setOptional( optional );
-		if ( BinderHelper.isEmptyAnnotationValue( mappedBy ) ) {
+		if ( isEmptyAnnotationValue( mappedBy ) ) {
 			/*
 			 * we need to check if the columns are in the right order
 			 * if not, then we need to create a many to one and formula
@@ -130,7 +134,7 @@ public class OneToOneSecondPass implements SecondPass {
 			boolean rightOrder = true;
 
 			if ( rightOrder ) {
-				String path = StringHelper.qualify( propertyHolder.getPath(), propertyName );
+				String path = qualify( propertyHolder.getPath(), propertyName );
 				final ToOneFkSecondPass secondPass = new ToOneFkSecondPass(
 						value,
 						joinColumns,
@@ -155,20 +159,20 @@ public class OneToOneSecondPass implements SecondPass {
 				if ( otherSide == null ) {
 					throw new MappingException( "Unable to find entity: " + value.getReferencedEntityName() );
 				}
-				otherSideProperty = BinderHelper.findPropertyByName( otherSide, mappedBy );
+				otherSideProperty = findPropertyByName( otherSide, mappedBy );
 			}
 			catch (MappingException e) {
 				throw new AnnotationException(
-						"Unknown mappedBy in: " + StringHelper.qualify( ownerEntity, ownerProperty )
+						"Unknown mappedBy in: " + qualify( ownerEntity, ownerProperty )
 								+ ", referenced property unknown: "
-								+ StringHelper.qualify( value.getReferencedEntityName(), mappedBy )
+								+ qualify( value.getReferencedEntityName(), mappedBy )
 				);
 			}
 			if ( otherSideProperty == null ) {
 				throw new AnnotationException(
-						"Unknown mappedBy in: " + StringHelper.qualify( ownerEntity, ownerProperty )
+						"Unknown mappedBy in: " + qualify( ownerEntity, ownerProperty )
 								+ ", referenced property unknown: "
-								+ StringHelper.qualify( value.getReferencedEntityName(), mappedBy )
+								+ qualify( value.getReferencedEntityName(), mappedBy )
 				);
 			}
 			if ( otherSideProperty.getValue() instanceof OneToOne ) {
@@ -238,11 +242,11 @@ public class OneToOneSecondPass implements SecondPass {
 			else {
 				throw new AnnotationException(
 						"Referenced property not a (One|Many)ToOne: "
-								+ StringHelper.qualify(
+								+ qualify(
 								otherSide.getEntityName(), mappedBy
 						)
 								+ " in mappedBy of "
-								+ StringHelper.qualify( ownerEntity, ownerProperty )
+								+ qualify( ownerEntity, ownerProperty )
 				);
 			}
 		}

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

@@ -20,6 +20,8 @@ import org.hibernate.mapping.PersistentClass;
 import org.hibernate.mapping.Property;
 import org.hibernate.mapping.ToOne;
 
+import static org.hibernate.cfg.BinderHelper.createSyntheticPropertyReference;
+
 /**
  * Enable a proper set of the FK columns in respect with the id column order
  * Allow the correct implementation of the default EJB3 values which needs both
@@ -99,7 +101,7 @@ public class ToOneFkSecondPass extends FkSecondPass {
 				);
 			}
 			manyToOne.setPropertyName( path );
-			BinderHelper.createSyntheticPropertyReference( columns, ref, null, manyToOne, false, buildingContext );
+			createSyntheticPropertyReference( columns, ref, null, manyToOne, false, buildingContext );
 			TableBinder.bindFk( ref, null, columns, manyToOne, unique, buildingContext );
 			/*
 			 * HbmMetadataSourceProcessorImpl does this only when property-ref != null, but IMO, it makes sense event if it is null

hibernate-core/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java

@@ -19,7 +19,6 @@ import jakarta.persistence.AttributeOverrides;
 import jakarta.persistence.CollectionTable;
 import jakarta.persistence.ConstraintMode;
 import jakarta.persistence.ElementCollection;
-import jakarta.persistence.Embeddable;
 import jakarta.persistence.FetchType;
 import jakarta.persistence.JoinColumn;
 import jakarta.persistence.JoinColumns;
@@ -90,7 +89,6 @@ import org.hibernate.cfg.AnnotatedColumn;
 import org.hibernate.cfg.AnnotatedJoinColumn;
 import org.hibernate.cfg.AnnotationBinder;
 import org.hibernate.cfg.AvailableSettings;
-import org.hibernate.cfg.BinderHelper;
 import org.hibernate.cfg.CollectionPropertyHolder;
 import org.hibernate.cfg.CollectionSecondPass;
 import org.hibernate.cfg.IndexColumn;
@@ -104,7 +102,6 @@ import org.hibernate.cfg.SecondPass;
 import org.hibernate.engine.config.spi.ConfigurationService;
 import org.hibernate.engine.spi.FilterDefinition;
 import org.hibernate.internal.CoreMessageLogger;
-import org.hibernate.internal.util.StringHelper;
 import org.hibernate.internal.util.collections.CollectionHelper;
 import org.hibernate.internal.util.config.ConfigurationHelper;
 import org.hibernate.mapping.Any;
@@ -136,10 +133,18 @@ import static jakarta.persistence.AccessType.PROPERTY;
 import static org.hibernate.cfg.AnnotatedColumn.checkPropertyConsistency;
 import static org.hibernate.cfg.AnnotationBinder.fillComponent;
 import static org.hibernate.cfg.AnnotationBinder.getOverridableAnnotation;
+import static org.hibernate.cfg.BinderHelper.PRIMITIVE_NAMES;
+import static org.hibernate.cfg.BinderHelper.buildAnyValue;
+import static org.hibernate.cfg.BinderHelper.createSyntheticPropertyReference;
 import static org.hibernate.cfg.BinderHelper.isEmptyAnnotationValue;
 import static org.hibernate.cfg.BinderHelper.toAliasEntityMap;
 import static org.hibernate.cfg.BinderHelper.toAliasTableMap;
 import static org.hibernate.engine.spi.ExecuteUpdateResultCheckStyle.fromExternalName;
+import static org.hibernate.internal.util.StringHelper.getNonEmptyOrConjunctionIfBothNonEmpty;
+import static org.hibernate.internal.util.StringHelper.isEmpty;
+import static org.hibernate.internal.util.StringHelper.isNotEmpty;
+import static org.hibernate.internal.util.StringHelper.nullIfEmpty;
+import static org.hibernate.internal.util.StringHelper.qualify;
 
 /**
  * Base class for binding different types of collections to Hibernate configuration objects.
@@ -295,12 +300,11 @@ public abstract class CollectionBinder {
 			XProperty property,
 			boolean isHibernateExtensionMapping,
 			MetadataBuildingContext buildingContext) {
-		final CollectionType typeAnnotation = HCANNHelper.findAnnotation( property, CollectionType.class );
 
 		final CollectionBinder binder;
+		final CollectionType typeAnnotation = HCANNHelper.findAnnotation( property, CollectionType.class );
 		if ( typeAnnotation != null ) {
 			binder = createBinderFromCustomTypeAnnotation( property, typeAnnotation, buildingContext );
-
 			// todo (6.0) - technically, these should no longer be needed
 			binder.explicitType = typeAnnotation.type().getName();
 			for ( Parameter param : typeAnnotation.parameters() ) {
@@ -308,23 +312,21 @@ public abstract class CollectionBinder {
 			}
 		}
 		else {
-			final CollectionClassification classification = determineCollectionClassification( property, buildingContext );
+			binder = createBinderAutomatically( property, buildingContext );
-			final CollectionTypeRegistrationDescriptor typeRegistration = buildingContext
-					.getMetadataCollector()
-					.findCollectionTypeRegistration( classification );
-			if ( typeRegistration != null ) {
-				binder = createBinderFromTypeRegistration( property, classification, typeRegistration, buildingContext );
-			}
-			else {
-				binder = createBinderFromProperty( property, buildingContext );
-			}
 		}
-
 		binder.setIsHibernateExtensionMapping( isHibernateExtensionMapping );
-
 		return binder;
 	}
 
+	private static CollectionBinder createBinderAutomatically(XProperty property, MetadataBuildingContext buildingContext) {
+		final CollectionClassification classification = determineCollectionClassification( property, buildingContext );
+		final CollectionTypeRegistrationDescriptor typeRegistration = buildingContext.getMetadataCollector()
+				.findCollectionTypeRegistration( classification );
+		return typeRegistration != null
+				? createBinderFromTypeRegistration( property, classification, typeRegistration, buildingContext )
+				: createBinderFromProperty( property, buildingContext );
+	}
+
 	private static CollectionBinder createBinderFromTypeRegistration(
 			XProperty property,
 			CollectionClassification classification,
@@ -390,8 +392,8 @@ public abstract class CollectionBinder {
 			CollectionType typeAnnotation,
 			MetadataBuildingContext buildingContext) {
 		determineSemanticJavaType( property );
-
+		final ManagedBean<? extends UserCollectionType> customTypeBean
-		final ManagedBean<? extends UserCollectionType> customTypeBean = resolveCustomType( property, typeAnnotation, buildingContext );
+				= resolveCustomType( property, typeAnnotation, buildingContext );
 		return createBinder(
 				property,
 				() -> customTypeBean,
@@ -455,48 +457,37 @@ public abstract class CollectionBinder {
 			CollectionClassification classification,
 			MetadataBuildingContext buildingContext) {
 		switch ( classification ) {
-			case ARRAY: {
+			case ARRAY:
-				if ( property.getElementClass().isPrimitive() ) {
+				return property.getElementClass().isPrimitive()
-					return new PrimitiveArrayBinder( customTypeBeanAccess, buildingContext );
+						? new PrimitiveArrayBinder( customTypeBeanAccess, buildingContext )
-				}
+						: new ArrayBinder( customTypeBeanAccess, buildingContext );
-				return new ArrayBinder( customTypeBeanAccess, buildingContext );
+			case BAG:
-			}
-			case BAG: {
 				return new BagBinder( customTypeBeanAccess, buildingContext );
-			}
+			case ID_BAG:
-			case ID_BAG: {
 				return new IdBagBinder( customTypeBeanAccess, buildingContext );
-			}
+			case LIST:
-			case LIST: {
 				return new ListBinder( customTypeBeanAccess, buildingContext );
-			}
 			case MAP:
-			case ORDERED_MAP: {
+			case ORDERED_MAP:
 				return new MapBinder( customTypeBeanAccess, false, buildingContext );
-			}
+			case SORTED_MAP:
-			case SORTED_MAP: {
 				return new MapBinder( customTypeBeanAccess, true, buildingContext );
-			}
 			case SET:
-			case ORDERED_SET: {
+			case ORDERED_SET:
 				return new SetBinder( customTypeBeanAccess, false, buildingContext );
-			}
+			case SORTED_SET:
-			case SORTED_SET: {
 				return new SetBinder( customTypeBeanAccess, true, buildingContext );
-			}
+			default:
+				throw new AnnotationException(
+						String.format(
+								Locale.ROOT,
+								"Unable to determine proper CollectionBinder (`%s) : %s.%s",
+								classification,
+								property.getDeclaringClass().getName(),
+								property.getName()
+						)
+				);
 		}
-
-		final XClass declaringClass = property.getDeclaringClass();
-
-		throw new AnnotationException(
-				String.format(
-						Locale.ROOT,
-						"Unable to determine proper CollectionBinder (`%s) : %s.%s",
-						classification,
-						declaringClass.getName(),
-						property.getName()
-				)
-		);
 	}
 
 	private static CollectionClassification determineCollectionClassification(
@@ -505,31 +496,29 @@ public abstract class CollectionBinder {
 		if ( property.isArray() ) {
 			return CollectionClassification.ARRAY;
 		}
-
+		else if ( HCANNHelper.findAnnotation( property, Bag.class ) == null ) {
-		final Bag bagAnnotation = HCANNHelper.findAnnotation( property, Bag.class );
+			return determineCollectionClassification( determineSemanticJavaType( property ), property, buildingContext );
-		if ( bagAnnotation != null ) {
+		}
+		else {
 			final Class<?> collectionJavaType = property.getCollectionClass();
-			if ( java.util.List.class.equals( collectionJavaType ) || java.util.Collection.class.equals( collectionJavaType ) ) {
+			if ( java.util.List.class.equals( collectionJavaType )
+					|| java.util.Collection.class.equals( collectionJavaType ) ) {
 				return CollectionClassification.BAG;
 			}
-			throw new MappingException(
+			else {
-					String.format(
+				throw new MappingException(
-							Locale.ROOT,
+						String.format(
-							"@Bag annotation encountered on an attribute `%s#%s` of type `%s`; only `%s` and `%s` are supported",
+								Locale.ROOT,
-							property.getDeclaringClass().getName(),
+								"@Bag annotation encountered on an attribute `%s#%s` of type `%s`; only `%s` and `%s` are supported",
-							property.getName(),
+								property.getDeclaringClass().getName(),
-							collectionJavaType.getName(),
+								property.getName(),
-							java.util.List.class.getName(),
+								collectionJavaType.getName(),
-							java.util.Collection.class.getName()
+								java.util.List.class.getName(),
-					)
+								java.util.Collection.class.getName()
-			);
+						)
+				);
+			}
 		}
-
-		return determineCollectionClassification(
-				determineSemanticJavaType( property ),
-				property,
-				buildingContext
-		);
 	}
 
 	private static CollectionClassification determineCollectionClassification(
@@ -561,12 +550,12 @@ public abstract class CollectionBinder {
 				return CollectionClassification.BAG;
 			}
 			ManyToMany manyToMany = property.getAnnotation( ManyToMany.class );
-			if ( manyToMany != null && ! StringHelper.isEmpty( manyToMany.mappedBy() ) ) {
+			if ( manyToMany != null && ! isEmpty( manyToMany.mappedBy() ) ) {
 				// We don't support @OrderColumn on the non-owning side of a many-to-many association.
 				return CollectionClassification.BAG;
 			}
 			OneToMany oneToMany = property.getAnnotation( OneToMany.class );
-			if ( oneToMany != null && ! StringHelper.isEmpty( oneToMany.mappedBy() ) ) {
+			if ( oneToMany != null && ! isEmpty( oneToMany.mappedBy() ) ) {
 				// Unowned to-many mappings are always considered BAG by default
 				return CollectionClassification.BAG;
 			}
@@ -599,8 +588,12 @@ public abstract class CollectionBinder {
 	}
 
 	private static Class<?> determineSemanticJavaType(XProperty property) {
-		final Class<?> returnedJavaType = property.getCollectionClass();
+		@SuppressWarnings("rawtypes")
-		if ( returnedJavaType == null ) {
+		Class<? extends java.util.Collection> collectionClass = property.getCollectionClass();
+		if ( collectionClass != null ) {
+			return inferCollectionClassFromSubclass( collectionClass );
+		}
+		else {
 			throw new AnnotationException(
 					String.format(
 							Locale.ROOT,
@@ -610,8 +603,6 @@ public abstract class CollectionBinder {
 					)
 			);
 		}
-
-		return inferCollectionClassFromSubclass( returnedJavaType );
 	}
 
 	private static Class<?> inferCollectionClassFromSubclass(Class<?> clazz) {
@@ -658,7 +649,7 @@ public abstract class CollectionBinder {
 
 	public void bind() {
 		this.collection = createCollection( propertyHolder.getPersistentClass() );
-		String role = StringHelper.qualify( propertyHolder.getPath(), propertyName );
+		String role = qualify( propertyHolder.getPath(), propertyName );
 		LOG.debugf( "Collection role: %s", role );
 		collection.setRole( role );
 		collection.setMappedByProperty( mappedBy );
@@ -667,7 +658,7 @@ public abstract class CollectionBinder {
 			&& mapKeyPropertyName != null ) {
 			throw new AnnotationException(
 					"Cannot mix @jakarta.persistence.MapKey and @MapKeyColumn or @org.hibernate.annotations.MapKey "
-							+ "on the same collection: " + StringHelper.qualify(
+							+ "on the same collection: " + qualify(
 							propertyHolder.getPath(), propertyName
 					)
 			);
@@ -700,54 +691,52 @@ public abstract class CollectionBinder {
 
 		final InFlightMetadataCollector metadataCollector = buildingContext.getMetadataCollector();
 
-		//many to many may need some second pass information
-		if ( !oneToMany && isMappedBy ) {
-			metadataCollector.addMappedBy( getCollectionType().getName(), mappedBy, propertyName );
-		}
 		//TODO reduce tableBinder != null and oneToMany
-		XClass collectionType = getCollectionType();
+		scheduleSecondPass( isMappedBy, metadataCollector );
-		if ( inheritanceStatePerClass == null) {
-			throw new AssertionFailure( "inheritanceStatePerClass not set" );
-		}
-		SecondPass sp = getSecondPass(
-				fkJoinColumns,
-				joinColumns,
-				inverseJoinColumns,
-				elementColumns,
-				mapKeyColumns,
-				mapKeyManyToManyColumns,
-				isEmbedded,
-				property,
-				collectionType,
-				notFoundAction,
-				oneToMany,
-				tableBinder,
-				buildingContext
-		);
-		if ( collectionType.isAnnotationPresent( Embeddable.class )
-				|| property.isAnnotationPresent( ElementCollection.class ) //JPA 2
-				) {
-			// do it right away, otherwise @ManyToOne on composite element call addSecondPass
-			// and raise a ConcurrentModificationException
-			//sp.doSecondPass( CollectionHelper.EMPTY_MAP );
-			metadataCollector.addSecondPass( sp, !isMappedBy );
-		}
-		else {
-			metadataCollector.addSecondPass( sp, !isMappedBy );
-		}
 
 		metadataCollector.addCollectionBinding( collection );
 
 		bindProperty();
 	}
 
+	private void scheduleSecondPass(boolean isMappedBy, InFlightMetadataCollector metadataCollector) {
+		//many to many may need some second pass information
+		if ( !oneToMany && isMappedBy ) {
+			metadataCollector.addMappedBy( getCollectionType().getName(), mappedBy, propertyName );
+		}
+
+		if ( inheritanceStatePerClass == null) {
+			throw new AssertionFailure( "inheritanceStatePerClass not set" );
+		}
+		metadataCollector.addSecondPass(
+				getSecondPass(
+						fkJoinColumns,
+						joinColumns,
+						inverseJoinColumns,
+						elementColumns,
+						mapKeyColumns,
+						mapKeyManyToManyColumns,
+						isEmbedded,
+						property,
+						getCollectionType(),
+						notFoundAction,
+						oneToMany,
+						tableBinder,
+						buildingContext
+				),
+				!isMappedBy
+		);
+	}
+
+	@SuppressWarnings({"rawtypes", "unchecked"})
 	private void bindCustomPersister() {
 		Persister persisterAnn = property.getAnnotation( Persister.class );
 		if ( persisterAnn != null ) {
-			//noinspection unchecked
+			Class clazz = persisterAnn.impl();
-			Class<? extends CollectionPersister> persister =
+			if ( !CollectionPersister.class.isAssignableFrom(clazz) ) {
-					(Class<? extends CollectionPersister>) persisterAnn.impl();
+				throw new AnnotationException( "persister class does not implement CollectionPersister: " + clazz.getName() );
-			collection.setCollectionPersisterClass( persister );
+			}
+			collection.setCollectionPersisterClass( clazz );
 		}
 	}
 
@@ -759,7 +748,7 @@ public abstract class CollectionBinder {
 
 	private void bindCache() {
 		//set cache
-		if ( StringHelper.isNotEmpty( cacheConcurrencyStrategy ) ) {
+		if ( isNotEmpty( cacheConcurrencyStrategy ) ) {
 			collection.setCacheConcurrencyStrategy( cacheConcurrencyStrategy );
 			collection.setCacheRegionName( cacheRegionName );
 		}
@@ -787,7 +776,7 @@ public abstract class CollectionBinder {
 					|| property.isAnnotationPresent( JoinColumns.class )
 					|| propertyHolder.getJoinTable( property ) != null ) ) {
 			String message = "Associations marked as mappedBy must not define database mappings like @JoinTable or @JoinColumn: ";
-			message += StringHelper.qualify( propertyHolder.getPath(), propertyName );
+			message += qualify( propertyHolder.getPath(), propertyName );
 			throw new AnnotationException( message );
 		}
 
@@ -796,7 +785,7 @@ public abstract class CollectionBinder {
 				&& property.isAnnotationPresent( OnDelete.class )
 				&& !property.isAnnotationPresent( JoinColumn.class )) {
 			String message = "Unidirectional one-to-many associations annotated with @OnDelete must define @JoinColumn: ";
-			message += StringHelper.qualify( propertyHolder.getPath(), propertyName );
+			message += qualify( propertyHolder.getPath(), propertyName );
 			throw new AnnotationException( message );
 		}
 	}
@@ -829,11 +818,8 @@ public abstract class CollectionBinder {
 
 	private void bindLoader() {
 		//SQL overriding
+
 		SQLInsert sqlInsert = property.getAnnotation( SQLInsert.class );
-		SQLUpdate sqlUpdate = property.getAnnotation( SQLUpdate.class );
-		SQLDelete sqlDelete = property.getAnnotation( SQLDelete.class );
-		SQLDeleteAll sqlDeleteAll = property.getAnnotation( SQLDeleteAll.class );
-		Loader loader = property.getAnnotation( Loader.class );
 		if ( sqlInsert != null ) {
 			collection.setCustomSQLInsert(
 					sqlInsert.sql().trim(),
@@ -842,6 +828,7 @@ public abstract class CollectionBinder {
 			);
 
 		}
+		SQLUpdate sqlUpdate = property.getAnnotation( SQLUpdate.class );
 		if ( sqlUpdate != null ) {
 			collection.setCustomSQLUpdate(
 					sqlUpdate.sql(),
@@ -849,6 +836,8 @@ public abstract class CollectionBinder {
 					fromExternalName( sqlUpdate.check().toString().toLowerCase(Locale.ROOT) )
 			);
 		}
+
+		SQLDelete sqlDelete = property.getAnnotation( SQLDelete.class );
 		if ( sqlDelete != null ) {
 			collection.setCustomSQLDelete(
 					sqlDelete.sql(),
@@ -856,6 +845,8 @@ public abstract class CollectionBinder {
 					fromExternalName( sqlDelete.check().toString().toLowerCase(Locale.ROOT) )
 			);
 		}
+
+		SQLDeleteAll sqlDeleteAll = property.getAnnotation( SQLDeleteAll.class );
 		if ( sqlDeleteAll != null ) {
 			collection.setCustomSQLDeleteAll(
 					sqlDeleteAll.sql(),
@@ -863,53 +854,50 @@ public abstract class CollectionBinder {
 					fromExternalName( sqlDeleteAll.check().toString().toLowerCase(Locale.ROOT) )
 			);
 		}
+
+		Loader loader = property.getAnnotation( Loader.class );
 		if ( loader != null ) {
 			collection.setLoaderName( loader.namedQuery() );
 		}
 	}
 
 	private void applySortingAndOrdering(Collection collection) {
-		final boolean hadExplicitSort;
-		final Class<? extends Comparator<?>> comparatorClass;
 
+		if ( naturalSort != null && comparatorSort != null ) {
+			throw buildIllegalSortCombination();
+		}
+		final boolean sorted = naturalSort != null || comparatorSort != null;
+		final Class<? extends Comparator<?>> comparatorClass;
 		if ( naturalSort != null ) {
-			if ( comparatorSort != null ) {
-				throw buildIllegalSortCombination();
-			}
-			hadExplicitSort = true;
 			comparatorClass = null;
 		}
 		else if ( comparatorSort != null ) {
-			hadExplicitSort = true;
 			comparatorClass = comparatorSort.value();
 		}
 		else {
-			hadExplicitSort = false;
 			comparatorClass = null;
 		}
 
-		boolean hadOrderBy = false;
+		if ( jpaOrderBy != null && sqlOrderBy != null ) {
-		if ( jpaOrderBy != null || sqlOrderBy != null ) {
+			throw buildIllegalOrderCombination();
-			if ( jpaOrderBy != null && sqlOrderBy != null ) {
+		}
-				throw buildIllegalOrderCombination();
+		boolean ordered = jpaOrderBy != null || sqlOrderBy != null;
-			}
+		if ( ordered ) {
-
-			hadOrderBy = true;
-
 			// we can only apply the sql-based order by up front.  The jpa order by has to wait for second pass
 			if ( sqlOrderBy != null ) {
 				collection.setOrderBy( sqlOrderBy.clause() );
 			}
 		}
 
-		final boolean isSorted = isSortedCollection || hadExplicitSort;
+		final boolean isSorted = isSortedCollection || sorted;
-
+		if ( isSorted && ordered ) {
-		if ( isSorted && hadOrderBy ) {
 			throw buildIllegalOrderAndSortCombination();
 		}
-
 		collection.setSorted( isSorted );
+		instantiateComparator( collection, comparatorClass );
+	}
 
+	private void instantiateComparator(Collection collection, Class<? extends Comparator<?>> comparatorClass) {
 		if ( comparatorClass != null ) {
 			try {
 				collection.setComparator( comparatorClass.newInstance() );
@@ -964,42 +952,21 @@ public abstract class CollectionBinder {
 	}
 
 	private void defineFetchingStrategy() {
+		final FetchType jpaFetchType = getJpaFetchType();
+
 		LazyCollection lazy = property.getAnnotation( LazyCollection.class );
-		Fetch fetch = property.getAnnotation( Fetch.class );
-		OneToMany oneToMany = property.getAnnotation( OneToMany.class );
-		ManyToMany manyToMany = property.getAnnotation( ManyToMany.class );
-		ElementCollection elementCollection = property.getAnnotation( ElementCollection.class );
-		ManyToAny manyToAny = property.getAnnotation( ManyToAny.class );
-
-		FetchType fetchType;
-		if ( oneToMany != null ) {
-			fetchType = oneToMany.fetch();
-		}
-		else if ( manyToMany != null ) {
-			fetchType = manyToMany.fetch();
-		}
-		else if ( elementCollection != null ) {
-			fetchType = elementCollection.fetch();
-		}
-		else if ( manyToAny != null ) {
-			fetchType = FetchType.LAZY;
-		}
-		else {
-			throw new AssertionFailure(
-					"Define fetch strategy on a property not annotated with @ManyToOne nor @OneToMany nor @CollectionOfElements"
-			);
-		}
-
 		if ( lazy != null ) {
-			collection.setLazy( !( lazy.value() == LazyCollectionOption.FALSE ) );
+			collection.setLazy( lazy.value() != LazyCollectionOption.FALSE );
 			collection.setExtraLazy( lazy.value() == LazyCollectionOption.EXTRA );
 		}
 		else {
-			collection.setLazy( fetchType == FetchType.LAZY );
+			collection.setLazy( jpaFetchType == FetchType.LAZY );
 			collection.setExtraLazy( false );
 		}
 
+		Fetch fetch = property.getAnnotation( Fetch.class );
 		if ( fetch != null ) {
+			// Hibernate @Fetch annotation takes precedence
 			if ( fetch.value() == org.hibernate.annotations.FetchMode.JOIN ) {
 				collection.setFetchMode( FetchMode.JOIN );
 				collection.setLazy( false );
@@ -1017,7 +984,31 @@ public abstract class CollectionBinder {
 			}
 		}
 		else {
-			collection.setFetchMode( AnnotationBinder.getFetchMode( fetchType ) );
+			collection.setFetchMode( AnnotationBinder.getFetchMode( jpaFetchType ) );
+		}
+	}
+
+	private FetchType getJpaFetchType() {
+		OneToMany oneToMany = property.getAnnotation( OneToMany.class );
+		ManyToMany manyToMany = property.getAnnotation( ManyToMany.class );
+		ElementCollection elementCollection = property.getAnnotation( ElementCollection.class );
+		ManyToAny manyToAny = property.getAnnotation( ManyToAny.class );
+		if ( oneToMany != null ) {
+			return oneToMany.fetch();
+		}
+		else if ( manyToMany != null ) {
+			return manyToMany.fetch();
+		}
+		else if ( elementCollection != null ) {
+			return elementCollection.fetch();
+		}
+		else if ( manyToAny != null ) {
+			return FetchType.LAZY;
+		}
+		else {
+			throw new AssertionFailure(
+					"Define fetch strategy on a property not annotated with @ManyToOne nor @OneToMany nor @CollectionOfElements"
+			);
 		}
 	}
 
@@ -1090,7 +1081,7 @@ public abstract class CollectionBinder {
 			MetadataBuildingContext buildingContext) {
 		PersistentClass persistentClass = persistentClasses.get( collType.getName() );
 		boolean reversePropertyInJoin = false;
-		if ( persistentClass != null && StringHelper.isNotEmpty( mappedBy ) ) {
+		if ( persistentClass != null && isNotEmpty( mappedBy ) ) {
 			try {
 				reversePropertyInJoin = 0 != persistentClass.getJoinNumber(
 						persistentClass.getRecursiveProperty( mappedBy )
@@ -1224,7 +1215,7 @@ public abstract class CollectionBinder {
 	private void handleJpaOrderBy(Collection collection, PersistentClass associatedClass) {
 		if ( jpaOrderBy != null ) {
 			final String orderByFragment = buildOrderByClauseFromHql( jpaOrderBy.value(), associatedClass );
-			if ( StringHelper.isNotEmpty( orderByFragment ) ) {
+			if ( isNotEmpty( orderByFragment ) ) {
 				collection.setOrderBy( orderByFragment );
 			}
 		}
@@ -1307,7 +1298,7 @@ public abstract class CollectionBinder {
 		if ( whereOnCollection != null ) {
 			whereOnCollectionClause = whereOnCollection.clause();
 		}
-		final String whereClause = StringHelper.getNonEmptyOrConjunctionIfBothNonEmpty(
+		final String whereClause = getNonEmptyOrConjunctionIfBothNonEmpty(
 				whereOnClassClause,
 				whereOnCollectionClause
 		);
@@ -1326,7 +1317,7 @@ public abstract class CollectionBinder {
 
 		WhereJoinTable whereJoinTable = property.getAnnotation( WhereJoinTable.class );
 		String whereJoinTableClause = whereJoinTable == null ? null : whereJoinTable.clause();
-		if ( StringHelper.isNotEmpty( whereJoinTableClause ) ) {
+		if ( isNotEmpty( whereJoinTableClause ) ) {
 			if (hasAssociationTable) {
 				// This is a many-to-many association.
 				// Collection#setWhere is used to set the "where" clause that applies to the collection table
@@ -1336,7 +1327,7 @@ public abstract class CollectionBinder {
 			else {
 				throw new AnnotationException(
 						"Illegal use of @WhereJoinTable on an association without join table: "
-								+ StringHelper.qualify( propertyHolder.getPath(), propertyName )
+								+ qualify( propertyHolder.getPath(), propertyName )
 				);
 			}
 		}
@@ -1345,13 +1336,13 @@ public abstract class CollectionBinder {
 	private void addFilter(boolean hasAssociationTable, FilterJoinTable filter) {
 		if ( hasAssociationTable ) {
 			final String condition;
-			if ( StringHelper.isEmpty( filter.condition() ) ) {
+			if ( isEmpty( filter.condition() ) ) {
 				final FilterDefinition filterDefinition = buildingContext.getMetadataCollector()
 						.getFilterDefinition( filter.name() );
 				if ( filterDefinition == null ) {
 					throw new AnnotationException(
 							"@FilterJoinTable on an association without condition attribute and without an any @FilterDef with a default condition"
-									+ StringHelper.qualify( propertyHolder.getPath(), propertyName )
+									+ qualify( propertyHolder.getPath(), propertyName )
 					);
 				}
 				condition = filterDefinition.getDefaultFilterCondition();
@@ -1370,7 +1361,7 @@ public abstract class CollectionBinder {
 		else {
 			throw new AnnotationException(
 					"Illegal use of @FilterJoinTable on an association without join table: "
-							+ StringHelper.qualify( propertyHolder.getPath(), propertyName )
+							+ qualify( propertyHolder.getPath(), propertyName )
 			);
 		}
 	}
@@ -1385,10 +1376,10 @@ public abstract class CollectionBinder {
 	private String getCondition(String cond, String name) {
 		if ( isEmptyAnnotationValue( cond ) ) {
 			cond = buildingContext.getMetadataCollector().getFilterDefinition( name ).getDefaultFilterCondition();
-			if ( StringHelper.isEmpty( cond ) ) {
+			if ( isEmpty( cond ) ) {
 				throw new AnnotationException(
 						"no filter condition found for filter " + name + " in "
-								+ StringHelper.qualify( propertyHolder.getPath(), propertyName )
+								+ qualify( propertyHolder.getPath(), propertyName )
 				);
 			}
 		}
@@ -1480,7 +1471,7 @@ public abstract class CollectionBinder {
 
 		//give a chance to override the referenced property name
 		//has to do that here because the referencedProperty creation happens in a FKSecondPass for Many to one yuk!
-		if ( joinColumns.length > 0 && StringHelper.isNotEmpty( joinColumns[0].getMappedBy() ) ) {
+		if ( joinColumns.length > 0 && isNotEmpty( joinColumns[0].getMappedBy() ) ) {
 			String entityName = joinColumns[0].getManyToManyOwnerSideEntityName() != null ?
 					"inverse__" + joinColumns[0].getManyToManyOwnerSideEntityName() :
 					joinColumns[0].getPropertyHolder().getEntityName();
@@ -1522,14 +1513,14 @@ public abstract class CollectionBinder {
 						key.disableForeignKey();
 					}
 					else {
-						key.setForeignKeyName( StringHelper.nullIfEmpty( collectionTableAnn.foreignKey().name() ) );
+						key.setForeignKeyName( nullIfEmpty( collectionTableAnn.foreignKey().name() ) );
-						key.setForeignKeyDefinition( StringHelper.nullIfEmpty( collectionTableAnn.foreignKey().foreignKeyDefinition() ) );
+						key.setForeignKeyDefinition( nullIfEmpty( collectionTableAnn.foreignKey().foreignKeyDefinition() ) );
 						if ( key.getForeignKeyName() == null &&
 							key.getForeignKeyDefinition() == null &&
 							collectionTableAnn.joinColumns().length == 1 ) {
 							JoinColumn joinColumn = collectionTableAnn.joinColumns()[0];
-							key.setForeignKeyName( StringHelper.nullIfEmpty( joinColumn.foreignKey().name() ) );
+							key.setForeignKeyName( nullIfEmpty( joinColumn.foreignKey().name() ) );
-							key.setForeignKeyDefinition( StringHelper.nullIfEmpty( joinColumn.foreignKey().foreignKeyDefinition() ) );
+							key.setForeignKeyDefinition( nullIfEmpty( joinColumn.foreignKey().foreignKeyDefinition() ) );
 						}
 					}
 				}
@@ -1554,21 +1545,21 @@ public abstract class CollectionBinder {
 							key.disableForeignKey();
 						}
 						else {
-							key.setForeignKeyName( StringHelper.nullIfEmpty( foreignKeyName ) );
+							key.setForeignKeyName( nullIfEmpty( foreignKeyName ) );
-							key.setForeignKeyDefinition( StringHelper.nullIfEmpty( foreignKeyDefinition ) );
+							key.setForeignKeyDefinition( nullIfEmpty( foreignKeyDefinition ) );
 						}
 					}
 					else {
 						final jakarta.persistence.ForeignKey fkOverride = propertyHolder.getOverriddenForeignKey(
-								StringHelper.qualify( propertyHolder.getPath(), property.getName() )
+								qualify( propertyHolder.getPath(), property.getName() )
 						);
 						if ( fkOverride != null && ( fkOverride.value() == ConstraintMode.NO_CONSTRAINT ||
 								fkOverride.value() == ConstraintMode.PROVIDER_DEFAULT && noConstraintByDefault ) ) {
 							key.disableForeignKey();
 						}
 						else if ( fkOverride != null ) {
-							key.setForeignKeyName( StringHelper.nullIfEmpty( fkOverride.name() ) );
+							key.setForeignKeyName( nullIfEmpty( fkOverride.name() ) );
-							key.setForeignKeyDefinition( StringHelper.nullIfEmpty( fkOverride.foreignKeyDefinition() ) );
+							key.setForeignKeyDefinition( nullIfEmpty( fkOverride.foreignKeyDefinition() ) );
 						}
 						else {
 							final OneToMany oneToManyAnn = property.getAnnotation( OneToMany.class );
@@ -1587,8 +1578,8 @@ public abstract class CollectionBinder {
 										key.disableForeignKey();
 									}
 									else {
-										key.setForeignKeyName( StringHelper.nullIfEmpty( joinColumnAnn.foreignKey().name() ) );
+										key.setForeignKeyName( nullIfEmpty( joinColumnAnn.foreignKey().name() ) );
-										key.setForeignKeyDefinition( StringHelper.nullIfEmpty( joinColumnAnn.foreignKey().foreignKeyDefinition() ) );
+										key.setForeignKeyDefinition( nullIfEmpty( joinColumnAnn.foreignKey().foreignKeyDefinition() ) );
 									}
 								}
 							}
@@ -1720,7 +1711,7 @@ public abstract class CollectionBinder {
 		AnnotatedClassType classType;
 
 		CollectionPropertyHolder holder;
-		if ( BinderHelper.PRIMITIVE_NAMES.contains( collType.getName() ) ) {
+		if ( PRIMITIVE_NAMES.contains( collType.getName() ) ) {
 			classType = AnnotatedClassType.NONE;
 			elementClass = null;
 
@@ -1815,7 +1806,7 @@ public abstract class CollectionBinder {
 
 			collValue.setElement( component );
 
-			if ( StringHelper.isNotEmpty(hqlOrderBy) ) {
+			if ( isNotEmpty(hqlOrderBy) ) {
 				String orderBy = adjustUserSuppliedValueCollectionOrderingFragment(hqlOrderBy);
 				if ( orderBy != null ) {
 					collValue.setOrderBy( orderBy );
@@ -1909,8 +1900,8 @@ public abstract class CollectionBinder {
 					element.disableForeignKey();
 				}
 				else {
-					element.setForeignKeyName( StringHelper.nullIfEmpty( foreignKeyName ) );
+					element.setForeignKeyName( nullIfEmpty( foreignKeyName ) );
-					element.setForeignKeyDefinition( StringHelper.nullIfEmpty( foreignKeyDefinition ) );
+					element.setForeignKeyDefinition( nullIfEmpty( foreignKeyDefinition ) );
 				}
 			}
 		}
@@ -1937,7 +1928,7 @@ public abstract class CollectionBinder {
 		}
 
 		ManyToAny anyAnn = property.getAnnotation( ManyToAny.class );
-		final Any any = BinderHelper.buildAnyValue(
+		final Any any = buildAnyValue(
 				discriminatorColumnAnn,
 				discriminatorFormulaAnn,
 				inverseJoinColumns,
@@ -1998,13 +1989,14 @@ public abstract class CollectionBinder {
 //				String header = ( mappedByProperty == null ) ? mappings.getLogicalTableName( ownerTable ) : mappedByProperty;
 //				column.setDefaultColumnHeader( header );
 		}
-		if ( StringHelper.isEmpty( associationTableBinder.getName() ) ) {
+		if ( isEmpty( associationTableBinder.getName() ) ) {
 			//default value
+			PersistentClass owner = collValue.getOwner();
 			associationTableBinder.setDefaultName(
-					collValue.getOwner().getClassName(),
+					owner.getClassName(),
-					collValue.getOwner().getEntityName(),
+					owner.getEntityName(),
-					collValue.getOwner().getJpaEntityName(),
+					owner.getJpaEntityName(),
-					buildingContext.getMetadataCollector().getLogicalTableName( collValue.getOwner().getTable() ),
+					buildingContext.getMetadataCollector().getLogicalTableName( owner.getTable() ),
 					collectionEntity != null ? collectionEntity.getClassName() : null,
 					collectionEntity != null ? collectionEntity.getEntityName() : null,
 					collectionEntity != null ? collectionEntity.getJpaEntityName() : null,
@@ -2047,15 +2039,11 @@ public abstract class CollectionBinder {
 							+ collValue.getOwnerEntityName() + "." + joinColumns[0].getPropertyName()
 			);
 		}
-		Table table;
+		Table table = otherSideProperty.getValue() instanceof Collection
-		if ( otherSideProperty.getValue() instanceof Collection ) {
+				? ((Collection) otherSideProperty.getValue()).getCollectionTable()
-			//this is a collection on the other side
+				: otherSideProperty.getValue().getTable();
-			table = ( (Collection) otherSideProperty.getValue() ).getCollectionTable();
+		//this is a collection on the other side
-		}
+		//This is a ToOne with a @JoinTable or a regular property
-		else {
-			//This is a ToOne with a @JoinTable or a regular property
-			table = otherSideProperty.getValue().getTable();
-		}
 		collValue.setCollectionTable( table );
 		String entityName = collectionEntity.getEntityName();
 		for (AnnotatedJoinColumn column : joinColumns) {
@@ -2128,12 +2116,14 @@ public abstract class CollectionBinder {
 			XProperty property,
 			XClass propertyClass,
 			MetadataBuildingContext context) {
-		final org.hibernate.annotations.EmbeddableInstantiator propertyAnnotation = property.getAnnotation( org.hibernate.annotations.EmbeddableInstantiator.class );
+		final org.hibernate.annotations.EmbeddableInstantiator propertyAnnotation
+				= property.getAnnotation( org.hibernate.annotations.EmbeddableInstantiator.class );
 		if ( propertyAnnotation != null ) {
 			return propertyAnnotation.value();
 		}
 
-		final org.hibernate.annotations.EmbeddableInstantiator classAnnotation = propertyClass.getAnnotation( org.hibernate.annotations.EmbeddableInstantiator.class );
+		final org.hibernate.annotations.EmbeddableInstantiator classAnnotation
+				= propertyClass.getAnnotation( org.hibernate.annotations.EmbeddableInstantiator.class );
 		if ( classAnnotation != null ) {
 			return classAnnotation.value();
 		}
@@ -2176,7 +2166,7 @@ public abstract class CollectionBinder {
 
 	private static void checkFilterConditions(Collection collValue) {
 		//for now it can't happen, but sometime soon...
-		if ( ( collValue.getFilters().size() != 0 || StringHelper.isNotEmpty( collValue.getWhere() ) ) &&
+		if ( ( collValue.getFilters().size() != 0 || isNotEmpty( collValue.getWhere() ) ) &&
 				collValue.getFetchMode() == FetchMode.JOIN &&
 				!( collValue.getElement() instanceof SimpleValue ) && //SimpleValue (CollectionOfElements) are always SELECT but it does not matter
 				collValue.getElement().getFetchMode() != FetchMode.JOIN ) {
@@ -2196,7 +2186,7 @@ public abstract class CollectionBinder {
 			PropertyHolder propertyHolder,
 			MetadataBuildingContext buildingContext) {
 		try {
-			BinderHelper.createSyntheticPropertyReference(
+			createSyntheticPropertyReference(
 					joinColumns,
 					collValue.getOwner(),
 					collectionEntity,
@@ -2239,7 +2229,7 @@ public abstract class CollectionBinder {
 			boolean unique,
 			MetadataBuildingContext buildingContext) {
 		final String mappedBy = columns[0].getMappedBy();
-		if ( StringHelper.isNotEmpty( mappedBy ) ) {
+		if ( isNotEmpty( mappedBy ) ) {
 			final Property property = referencedEntity.getRecursiveProperty( mappedBy );
 			List<Selectable> mappedByColumns;
 			if ( property.getValue() instanceof Collection ) {
@@ -2279,7 +2269,7 @@ public abstract class CollectionBinder {
 			value.createForeignKey();
 		}
 		else {
-			BinderHelper.createSyntheticPropertyReference( columns, referencedEntity, null, value, true, buildingContext );
+			createSyntheticPropertyReference( columns, referencedEntity, null, value, true, buildingContext );
 			if ( notFoundAction == NotFoundAction.IGNORE ) {
 				value.disableForeignKey();
 			}

hibernate-core/src/main/java/org/hibernate/cfg/annotations/EntityBinder.java

@@ -69,16 +69,13 @@ import org.hibernate.boot.spi.InFlightMetadataCollector;
 import org.hibernate.boot.spi.MetadataBuildingContext;
 import org.hibernate.cfg.AccessType;
 import org.hibernate.cfg.AnnotationBinder;
-import org.hibernate.cfg.BinderHelper;
 import org.hibernate.cfg.AnnotatedJoinColumn;
 import org.hibernate.cfg.PropertyHolder;
 import org.hibernate.cfg.UniqueConstraintHolder;
 import org.hibernate.engine.OptimisticLockStyle;
 import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment;
-import org.hibernate.engine.spi.ExecuteUpdateResultCheckStyle;
 import org.hibernate.engine.spi.FilterDefinition;
 import org.hibernate.internal.CoreMessageLogger;
-import org.hibernate.internal.util.StringHelper;
 import org.hibernate.mapping.DependantValue;
 import org.hibernate.mapping.Join;
 import org.hibernate.mapping.PersistentClass;
@@ -92,8 +89,15 @@ import org.hibernate.mapping.Value;
 import org.hibernate.persister.entity.EntityPersister;
 import org.jboss.logging.Logger;
 
+import static org.hibernate.cfg.AnnotatedJoinColumn.buildJoinColumn;
+import static org.hibernate.cfg.BinderHelper.isEmptyAnnotationValue;
 import static org.hibernate.cfg.BinderHelper.toAliasEntityMap;
 import static org.hibernate.cfg.BinderHelper.toAliasTableMap;
+import static org.hibernate.engine.spi.ExecuteUpdateResultCheckStyle.fromExternalName;
+import static org.hibernate.internal.util.StringHelper.isEmpty;
+import static org.hibernate.internal.util.StringHelper.isNotEmpty;
+import static org.hibernate.internal.util.StringHelper.nullIfEmpty;
+import static org.hibernate.internal.util.StringHelper.unqualify;
 
 
 /**
@@ -159,7 +163,7 @@ public class EntityBinder {
 		this.context = context;
 		this.persistentClass = persistentClass;
 		this.annotatedClass = annotatedClass;
-		bindEntityAnnotation( annotatedClass.getAnnotation( Entity.class ) );
+		bindEntityAnnotation();
 		bindHibernateAnnotation();
 	}
 
@@ -174,54 +178,28 @@ public class EntityBinder {
 	public boolean isPropertyDefinedInSuperHierarchy(String name) {
 		// Yes, yes... persistentClass can be null because EntityBinder can be used
 		// to bind components as well, of course...
-		return  persistentClass != null &&  persistentClass.isPropertyDefinedInSuperHierarchy( name );
+		return  persistentClass != null && persistentClass.isPropertyDefinedInSuperHierarchy( name );
 	}
 
-	@SuppressWarnings("SimplifiableConditionalExpression")
 	private void bindHibernateAnnotation() {
-		{
+		final DynamicInsert dynamicInsertAnn = annotatedClass.getAnnotation( DynamicInsert.class );
-			final DynamicInsert dynamicInsertAnn = annotatedClass.getAnnotation( DynamicInsert.class );
+		dynamicInsert = dynamicInsertAnn != null && dynamicInsertAnn.value();
-			this.dynamicInsert = dynamicInsertAnn == null
+		final DynamicUpdate dynamicUpdateAnn = annotatedClass.getAnnotation( DynamicUpdate.class );
-					? false
+		dynamicUpdate = dynamicUpdateAnn != null && dynamicUpdateAnn.value();
-					: dynamicInsertAnn.value();
+		final SelectBeforeUpdate selectBeforeUpdateAnn = annotatedClass.getAnnotation( SelectBeforeUpdate.class );
-		}
+		selectBeforeUpdate = selectBeforeUpdateAnn != null && selectBeforeUpdateAnn.value();
-
+		final OptimisticLocking optimisticLockingAnn = annotatedClass.getAnnotation( OptimisticLocking.class );
-		{
+		optimisticLockType = optimisticLockingAnn == null ? OptimisticLockType.VERSION : optimisticLockingAnn.type();
-			final DynamicUpdate dynamicUpdateAnn = annotatedClass.getAnnotation( DynamicUpdate.class );
+		final Polymorphism polymorphismAnn = annotatedClass.getAnnotation( Polymorphism.class );
-			this.dynamicUpdate = dynamicUpdateAnn == null
+		polymorphismType = polymorphismAnn == null ? PolymorphismType.IMPLICIT : polymorphismAnn.type();
-					? false
-					: dynamicUpdateAnn.value();
-		}
-
-		{
-			final SelectBeforeUpdate selectBeforeUpdateAnn = annotatedClass.getAnnotation( SelectBeforeUpdate.class );
-			this.selectBeforeUpdate = selectBeforeUpdateAnn == null
-					? false
-					: selectBeforeUpdateAnn.value();
-		}
-
-		{
-			final OptimisticLocking optimisticLockingAnn = annotatedClass.getAnnotation( OptimisticLocking.class );
-			this.optimisticLockType = optimisticLockingAnn == null
-					? OptimisticLockType.VERSION
-					: optimisticLockingAnn.type();
-		}
-
-		{
-			final Polymorphism polymorphismAnn = annotatedClass.getAnnotation( Polymorphism.class );
-			this.polymorphismType = polymorphismAnn == null
-					? PolymorphismType.IMPLICIT
-					: polymorphismAnn.type();
-		}
 	}
 
-	private void bindEntityAnnotation(Entity ejb3Ann) {
+	private void bindEntityAnnotation() {
-		if ( ejb3Ann == null ) {
+		Entity entity = annotatedClass.getAnnotation( Entity.class );
+		if ( entity == null ) {
 			throw new AssertionFailure( "@Entity should never be missing" );
 		}
-		name = BinderHelper.isEmptyAnnotationValue( ejb3Ann.name() )
+		name = isEmptyAnnotationValue( entity.name() ) ? unqualify( annotatedClass.getName() ) : entity.name();
-				? StringHelper.unqualify(annotatedClass.getName())
-				: ejb3Ann.name();
 	}
 
 	public boolean isRootEntity() {
@@ -258,12 +236,12 @@ public class EntityBinder {
 
 		if ( persistentClass instanceof RootClass ) {
 			RootClass rootClass = (RootClass) persistentClass;
-			boolean mutable = !annotatedClass.isAnnotationPresent( Immutable.class );
 
+			boolean mutable = !annotatedClass.isAnnotationPresent( Immutable.class );
 			rootClass.setMutable( mutable );
 			rootClass.setExplicitPolymorphism( isExplicitPolymorphism( polymorphismType ) );
 
-			if ( StringHelper.isNotEmpty( where ) ) {
+			if ( isNotEmpty( where ) ) {
 				rootClass.setWhere( where );
 			}
 
@@ -286,8 +264,8 @@ public class EntityBinder {
 			}
 		}
 		else {
-			if (annotatedClass.isAnnotationPresent(Immutable.class)) {
+			if ( annotatedClass.isAnnotationPresent(Immutable.class) ) {
-				LOG.immutableAnnotationOnNonRoot(annotatedClass.getName());
+				LOG.immutableAnnotationOnNonRoot( annotatedClass.getName() );
 			}
 		}
 
@@ -296,83 +274,14 @@ public class EntityBinder {
 		persistentClass.setOptimisticLockStyle( getVersioning( optimisticLockType ) );
 		persistentClass.setSelectBeforeUpdate( selectBeforeUpdate );
 
-		//set persister if needed
+		bindCustomPersister();
-		Persister persisterAnn = annotatedClass.getAnnotation( Persister.class );
-		if ( persisterAnn != null ) {
-			//TODO: throw an error if the class doesn't inherit EntityPersister
-			Class<? extends EntityPersister> persister = (Class<? extends EntityPersister>) persisterAnn.impl();
-			persistentClass.setEntityPersisterClass( persister );
-		}
 
 		persistentClass.setBatchSize( batchSize );
 
-		//SQL overriding
+		bindCustomSql();
-		SQLInsert sqlInsert = annotatedClass.getAnnotation( SQLInsert.class );
+		bindSynchronize();
-		SQLUpdate sqlUpdate = annotatedClass.getAnnotation( SQLUpdate.class );
+		bindhandleFilters();
-		SQLDelete sqlDelete = annotatedClass.getAnnotation( SQLDelete.class );
-		SQLDeleteAll sqlDeleteAll = annotatedClass.getAnnotation( SQLDeleteAll.class );
-		Loader loader = annotatedClass.getAnnotation( Loader.class );
 
-		if ( sqlInsert != null ) {
-			persistentClass.setCustomSQLInsert( sqlInsert.sql().trim(), sqlInsert.callable(),
-					ExecuteUpdateResultCheckStyle.fromExternalName( sqlInsert.check().toString().toLowerCase(Locale.ROOT) )
-			);
-
-		}
-		if ( sqlUpdate != null ) {
-			persistentClass.setCustomSQLUpdate( sqlUpdate.sql(), sqlUpdate.callable(),
-					ExecuteUpdateResultCheckStyle.fromExternalName( sqlUpdate.check().toString().toLowerCase(Locale.ROOT) )
-			);
-		}
-		if ( sqlDelete != null ) {
-			persistentClass.setCustomSQLDelete( sqlDelete.sql(), sqlDelete.callable(),
-					ExecuteUpdateResultCheckStyle.fromExternalName( sqlDelete.check().toString().toLowerCase(Locale.ROOT) )
-			);
-		}
-		if ( sqlDeleteAll != null ) {
-			persistentClass.setCustomSQLDelete( sqlDeleteAll.sql(), sqlDeleteAll.callable(),
-					ExecuteUpdateResultCheckStyle.fromExternalName( sqlDeleteAll.check().toString().toLowerCase(Locale.ROOT) )
-			);
-		}
-		if ( loader != null ) {
-			persistentClass.setLoaderName( loader.namedQuery() );
-		}
-
-		final JdbcEnvironment jdbcEnvironment = context.getMetadataCollector().getDatabase().getJdbcEnvironment();
-		if ( annotatedClass.isAnnotationPresent( Synchronize.class )) {
-			Synchronize synchronizedWith = annotatedClass.getAnnotation(Synchronize.class);
-
-			String [] tables = synchronizedWith.value();
-			for (String table : tables) {
-				persistentClass.addSynchronizedTable(
-						context.getBuildingOptions().getPhysicalNamingStrategy().toPhysicalTableName(
-								jdbcEnvironment.getIdentifierHelper().toIdentifier( table ),
-								jdbcEnvironment
-						).render( jdbcEnvironment.getDialect() )
-				);
-			}
-		}
-
-		if ( annotatedClass.isAnnotationPresent(Subselect.class )) {
-			Subselect subselect = annotatedClass.getAnnotation(Subselect.class);
-			this.subselect = subselect.value();
-		}
-
-		for ( Filter filter : filters ) {
-			String filterName = filter.name();
-			String cond = filter.condition();
-			if ( BinderHelper.isEmptyAnnotationValue( cond ) ) {
-				FilterDefinition definition = context.getMetadataCollector().getFilterDefinition( filterName );
-				cond = definition == null ? null : definition.getDefaultFilterCondition();
-				if ( StringHelper.isEmpty( cond ) ) {
-					throw new AnnotationException(
-							"no filter condition found for filter " + filterName + " in " + this.name
-					);
-				}
-			}
-			persistentClass.addFilter(filterName, cond, filter.deduceAliasInjectionPoints(),
-					toAliasTableMap(filter.aliases()), toAliasEntityMap(filter.aliases()));
-		}
 		LOG.debugf( "Import with entity name %s", name );
 		try {
 			context.getMetadataCollector().addImport( name, persistentClass.getEntityName() );
@@ -388,6 +297,105 @@ public class EntityBinder {
 		processNamedEntityGraphs();
 	}
 
+	private void bindCustomSql() {
+		//SQL overriding
+		SQLInsert sqlInsert = annotatedClass.getAnnotation( SQLInsert.class );
+		if ( sqlInsert != null ) {
+			persistentClass.setCustomSQLInsert(
+					sqlInsert.sql().trim(),
+					sqlInsert.callable(),
+					fromExternalName( sqlInsert.check().toString().toLowerCase(Locale.ROOT) )
+			);
+
+		}
+
+		SQLUpdate sqlUpdate = annotatedClass.getAnnotation( SQLUpdate.class );
+		if ( sqlUpdate != null ) {
+			persistentClass.setCustomSQLUpdate(
+					sqlUpdate.sql().trim(),
+					sqlUpdate.callable(),
+					fromExternalName( sqlUpdate.check().toString().toLowerCase(Locale.ROOT) )
+			);
+		}
+
+		SQLDelete sqlDelete = annotatedClass.getAnnotation( SQLDelete.class );
+		if ( sqlDelete != null ) {
+			persistentClass.setCustomSQLDelete(
+					sqlDelete.sql().trim(),
+					sqlDelete.callable(),
+					fromExternalName( sqlDelete.check().toString().toLowerCase(Locale.ROOT) )
+			);
+		}
+
+		SQLDeleteAll sqlDeleteAll = annotatedClass.getAnnotation( SQLDeleteAll.class );
+		if ( sqlDeleteAll != null ) {
+			persistentClass.setCustomSQLDelete(
+					sqlDeleteAll.sql().trim(),
+					sqlDeleteAll.callable(),
+					fromExternalName( sqlDeleteAll.check().toString().toLowerCase(Locale.ROOT) )
+			);
+		}
+
+		Loader loader = annotatedClass.getAnnotation( Loader.class );
+		if ( loader != null ) {
+			persistentClass.setLoaderName( loader.namedQuery() );
+		}
+
+		Subselect subselect = annotatedClass.getAnnotation( Subselect.class );
+		if ( subselect != null ) {
+			this.subselect = subselect.value();
+		}
+	}
+
+	private void bindhandleFilters() {
+		for ( Filter filter : filters ) {
+			final String filterName = filter.name();
+			String condition = filter.condition();
+			if ( isEmptyAnnotationValue( condition ) ) {
+				final FilterDefinition definition = context.getMetadataCollector().getFilterDefinition( filterName );
+				condition = definition == null ? null : definition.getDefaultFilterCondition();
+				if ( isEmpty( condition ) ) {
+					throw new AnnotationException( "no filter condition found for filter "
+							+ filterName + " in " + this.name );
+				}
+			}
+			persistentClass.addFilter(
+					filterName,
+					condition,
+					filter.deduceAliasInjectionPoints(),
+					toAliasTableMap( filter.aliases() ),
+					toAliasEntityMap( filter.aliases() )
+			);
+		}
+	}
+
+	private void bindSynchronize() {
+		if ( annotatedClass.isAnnotationPresent( Synchronize.class ) ) {
+			final JdbcEnvironment jdbcEnvironment = context.getMetadataCollector().getDatabase().getJdbcEnvironment();
+			for ( String table : annotatedClass.getAnnotation(Synchronize.class).value() ) {
+				persistentClass.addSynchronizedTable(
+						context.getBuildingOptions().getPhysicalNamingStrategy().toPhysicalTableName(
+								jdbcEnvironment.getIdentifierHelper().toIdentifier( table ),
+								jdbcEnvironment
+						).render( jdbcEnvironment.getDialect() )
+				);
+			}
+		}
+	}
+
+	@SuppressWarnings({"rawtypes", "unchecked"})
+	private void bindCustomPersister() {
+		//set persister if needed
+		Persister persisterAnn = annotatedClass.getAnnotation( Persister.class );
+		if ( persisterAnn != null ) {
+			Class clazz = persisterAnn.impl();
+			if ( !EntityPersister.class.isAssignableFrom(clazz) ) {
+				throw new AnnotationException( "persister class does not implement EntityPersister: " + clazz.getName() );
+			}
+			persistentClass.setEntityPersisterClass( clazz );
+		}
+	}
+
 	public PersistentClass getPersistentClass() {
 		return persistentClass;
 	}
@@ -412,7 +420,7 @@ public class EntityBinder {
 	}
 
 	public void bindDiscriminatorValue() {
-		if ( StringHelper.isEmpty( discriminatorValue ) ) {
+		if ( isEmpty( discriminatorValue ) ) {
 			Value discriminator = persistentClass.getDiscriminator();
 			if ( discriminator == null ) {
 				persistentClass.setDiscriminatorValue( name );
@@ -506,122 +514,19 @@ public class EntityBinder {
 			XClass clazzToProcess,
 			SharedCacheMode sharedCacheMode,
 			MetadataBuildingContext context) {
+		bindCache( clazzToProcess, sharedCacheMode, context );
+		bindNaturalIdCache( clazzToProcess );
+	}
 
-		final Cache explicitCacheAnn = clazzToProcess.getAnnotation( Cache.class );
+	private void bindNaturalIdCache(XClass clazzToProcess) {
-		final Cacheable explicitCacheableAnn = clazzToProcess.getAnnotation( Cacheable.class );
-
-		isCached = false;
-		cacheConcurrentStrategy = null;
-		cacheRegion = null;
-		cacheLazyProperty = true;
-
-		if ( persistentClass instanceof RootClass ) {
-			Cache effectiveCacheAnn = explicitCacheAnn;
-
-			if ( explicitCacheAnn != null ) {
-				// preserve legacy behavior of circumventing SharedCacheMode when Hibernate's @Cache is used.
-				isCached = true;
-			}
-			else {
-				effectiveCacheAnn = buildCacheMock( clazzToProcess.getName(), context );
-
-				switch ( sharedCacheMode ) {
-					case ALL: {
-						// all entities should be cached
-						isCached = true;
-						break;
-					}
-					case ENABLE_SELECTIVE: {
-						if ( explicitCacheableAnn != null && explicitCacheableAnn.value() ) {
-							isCached = true;
-						}
-						break;
-					}
-					case DISABLE_SELECTIVE: {
-						if ( explicitCacheableAnn == null || explicitCacheableAnn.value() ) {
-							isCached = true;
-						}
-						break;
-					}
-					default: {
-						// treat both NONE and UNSPECIFIED the same
-						isCached = false;
-						break;
-					}
-				}
-			}
-
-			cacheConcurrentStrategy = resolveCacheConcurrencyStrategy( effectiveCacheAnn.usage() );
-			cacheRegion = effectiveCacheAnn.region();
-			switch ( effectiveCacheAnn.include().toLowerCase( Locale.ROOT ) ) {
-				case "all": {
-					cacheLazyProperty = true;
-					break;
-				}
-				case "non-lazy": {
-					cacheLazyProperty = false;
-					break;
-				}
-				default: {
-					throw new AnnotationException(
-							"Unknown @Cache.include value [" + effectiveCacheAnn.include() + "] : "
-									+ annotatedClass.getName()
-					);
-				}
-			}
-		}
-		else {
-			if ( explicitCacheAnn != null ) {
-				LOG.cacheOrCacheableAnnotationOnNonRoot(
-						persistentClass.getClassName() == null
-								? annotatedClass.getName()
-								: persistentClass.getClassName()
-				);
-			}
-			else if ( explicitCacheableAnn == null && persistentClass.getSuperclass() != null ) {
-				// we should inherit our super's caching config
-				isCached = persistentClass.getSuperclass().isCached();
-			}
-			else {
-				switch ( sharedCacheMode ) {
-					case ALL: {
-						// all entities should be cached
-						isCached = true;
-						break;
-					}
-					case ENABLE_SELECTIVE: {
-						// only entities with @Cacheable(true) should be cached
-						if ( explicitCacheableAnn != null && explicitCacheableAnn.value() ) {
-							isCached = true;
-						}
-						break;
-					}
-					case DISABLE_SELECTIVE: {
-						if ( explicitCacheableAnn == null || !explicitCacheableAnn.value() ) {
-							isCached = true;
-						}
-						break;
-					}
-					default: {
-						// treat both NONE and UNSPECIFIED the same
-						isCached = false;
-						break;
-					}
-				}
-			}
-		}
-
 		naturalIdCacheRegion = null;
-
 		final NaturalIdCache naturalIdCacheAnn = clazzToProcess.getAnnotation( NaturalIdCache.class );
 		if ( naturalIdCacheAnn != null ) {
-			if ( BinderHelper.isEmptyAnnotationValue( naturalIdCacheAnn.region() ) ) {
+			if ( isEmptyAnnotationValue( naturalIdCacheAnn.region() ) ) {
-				if ( explicitCacheAnn != null && StringHelper.isNotEmpty( explicitCacheAnn.region() ) ) {
+				final Cache explicitCacheAnn = clazzToProcess.getAnnotation( Cache.class );
-					naturalIdCacheRegion = explicitCacheAnn.region() + NATURAL_ID_CACHE_SUFFIX;
+				naturalIdCacheRegion = explicitCacheAnn != null && isNotEmpty( explicitCacheAnn.region() )
-				}
+						? explicitCacheAnn.region() + NATURAL_ID_CACHE_SUFFIX
-				else {
+						: clazzToProcess.getName() + NATURAL_ID_CACHE_SUFFIX;
-					naturalIdCacheRegion = clazzToProcess.getName() + NATURAL_ID_CACHE_SUFFIX;
-				}
 			}
 			else {
 				naturalIdCacheRegion = naturalIdCacheAnn.region();
@@ -629,6 +534,86 @@ public class EntityBinder {
 		}
 	}
 
+	private void bindCache(XClass clazzToProcess, SharedCacheMode sharedCacheMode, MetadataBuildingContext context) {
+		isCached = false;
+		cacheConcurrentStrategy = null;
+		cacheRegion = null;
+		cacheLazyProperty = true;
+		if ( persistentClass instanceof RootClass ) {
+			bindRootClassCache( clazzToProcess, sharedCacheMode, context );
+		}
+		else {
+			bindSubclassCache( clazzToProcess, sharedCacheMode );
+		}
+	}
+
+	private void bindSubclassCache(XClass clazzToProcess, SharedCacheMode sharedCacheMode) {
+		final Cache cache = clazzToProcess.getAnnotation( Cache.class );
+		final Cacheable cacheable = clazzToProcess.getAnnotation( Cacheable.class );
+		if ( cache != null ) {
+			LOG.cacheOrCacheableAnnotationOnNonRoot(
+					persistentClass.getClassName() == null
+							? annotatedClass.getName()
+							: persistentClass.getClassName()
+			);
+		}
+		else if ( cacheable == null && persistentClass.getSuperclass() != null ) {
+			// we should inherit our super's caching config
+			isCached = persistentClass.getSuperclass().isCached();
+		}
+		else {
+			isCached = isCacheable( sharedCacheMode, cacheable );
+		}
+	}
+
+	private void bindRootClassCache(XClass clazzToProcess, SharedCacheMode sharedCacheMode, MetadataBuildingContext context) {
+		final Cache cache = clazzToProcess.getAnnotation( Cache.class );
+		final Cacheable cacheable = clazzToProcess.getAnnotation( Cacheable.class );
+		final Cache effectiveCache;
+		if ( cache != null ) {
+			// preserve legacy behavior of circumventing SharedCacheMode when Hibernate's @Cache is used.
+			isCached = true;
+			effectiveCache = cache;
+		}
+		else {
+			effectiveCache = buildCacheMock( clazzToProcess.getName(), context );
+			isCached = isCacheable( sharedCacheMode, cacheable );
+		}
+		cacheConcurrentStrategy = resolveCacheConcurrencyStrategy( effectiveCache.usage() );
+		cacheRegion = effectiveCache.region();
+		cacheLazyProperty = isCacheLazy( effectiveCache, annotatedClass );
+	}
+
+	private static boolean isCacheLazy(Cache effectiveCache, XClass annotatedClass) {
+		switch ( effectiveCache.include().toLowerCase( Locale.ROOT ) ) {
+			case "all":
+				return true;
+			case "non-lazy":
+				return false;
+			default:
+				throw new AnnotationException( "Unknown @Cache.include value [" + effectiveCache.include() + "] : "
+						+ annotatedClass.getName()
+				);
+		}
+	}
+
+	private static boolean isCacheable(SharedCacheMode sharedCacheMode, Cacheable explicitCacheableAnn) {
+		switch (sharedCacheMode) {
+			case ALL:
+				// all entities should be cached
+				return true;
+			case ENABLE_SELECTIVE:
+				// only entities with @Cacheable(true) should be cached
+				return explicitCacheableAnn != null && explicitCacheableAnn.value();
+			case DISABLE_SELECTIVE:
+				// only entities with @Cacheable(false) should not be cached
+				return explicitCacheableAnn == null || explicitCacheableAnn.value();
+			default:
+				// treat both NONE and UNSPECIFIED the same
+				return false;
+		}
+	}
+
 	private static String resolveCacheConcurrencyStrategy(CacheConcurrencyStrategy strategy) {
 		final org.hibernate.cache.spi.access.AccessType accessType = strategy.toAccessType();
 		return accessType == null ? null : accessType.getExternalName();
@@ -638,7 +623,7 @@ public class EntityBinder {
 		return new LocalCacheAnnotationStub( region, determineCacheConcurrencyStrategy( context ) );
 	}
 
-	@SuppressWarnings({ "ClassExplicitlyAnnotation" })
+	@SuppressWarnings("ClassExplicitlyAnnotation")
 	private static class LocalCacheAnnotationStub implements Cache {
 		private final String region;
 		private final CacheConcurrencyStrategy usage;
@@ -666,9 +651,7 @@ public class EntityBinder {
 	}
 
 	private static CacheConcurrencyStrategy determineCacheConcurrencyStrategy(MetadataBuildingContext context) {
-		return CacheConcurrencyStrategy.fromAccessType(
+		return CacheConcurrencyStrategy.fromAccessType( context.getBuildingOptions().getImplicitCacheAccessType() );
-				context.getBuildingOptions().getImplicitCacheAccessType()
-		);
 	}
 
 	private static class EntityTableNamingStrategyHelper implements NamingStrategyHelper {
@@ -745,9 +728,7 @@ public class EntityBinder {
 		context.getMetadataCollector().addEntityTableXref(
 				persistentClass.getEntityName(),
 				context.getMetadataCollector().getDatabase().toIdentifier(
-						context.getMetadataCollector().getLogicalTableName(
+						context.getMetadataCollector().getLogicalTableName( superTableXref.getPrimaryTable() )
-								superTableXref.getPrimaryTable()
-						)
 				),
 				superTableXref.getPrimaryTable(),
 				superTableXref
@@ -761,19 +742,15 @@ public class EntityBinder {
 			List<UniqueConstraintHolder> uniqueConstraints,
 			String constraints,
 			InFlightMetadataCollector.EntityTableXref denormalizedSuperTableXref) {
-		EntityTableNamingStrategyHelper namingStrategyHelper = new EntityTableNamingStrategyHelper(
+
+		final EntityTableNamingStrategyHelper namingStrategyHelper = new EntityTableNamingStrategyHelper(
 				persistentClass.getClassName(),
 				persistentClass.getEntityName(),
 				name
 		);
-
+		final Identifier logicalName = isNotEmpty( tableName )
-		final Identifier logicalName;
+				? namingStrategyHelper.handleExplicitName( tableName, context )
-		if ( StringHelper.isNotEmpty( tableName ) ) {
+				: namingStrategyHelper.determineImplicitName( context );
-			logicalName = namingStrategyHelper.handleExplicitName( tableName, context );
-		}
-		else {
-			logicalName = namingStrategyHelper.determineImplicitName( context );
-		}
 
 		final Table table = TableBinder.buildAndFillTable(
 				schema,
@@ -784,14 +761,13 @@ public class EntityBinder {
 				null,
 				constraints,
 				context,
-				this.subselect,
+				subselect,
 				denormalizedSuperTableXref
 		);
 		final RowId rowId = annotatedClass.getAnnotation( RowId.class );
 		if ( rowId != null ) {
 			table.setRowId( rowId.value() );
 		}
-
 		final Comment comment = annotatedClass.getAnnotation( Comment.class );
 		if ( comment != null ) {
 			table.setComment( comment.value() );
@@ -814,104 +790,82 @@ public class EntityBinder {
 	}
 
 	public void finalSecondaryTableBinding(PropertyHolder propertyHolder) {
-		/*
+		 // This operation has to be done after the id definition of the persistence class.
-		 * Those operations has to be done after the id definition of the persistence class.
+		 // ie after the properties parsing
-		 * ie after the properties parsing
-		 */
 		Iterator<Object> joinColumns = secondaryTableJoins.values().iterator();
-
 		for ( Map.Entry<String, Join> entrySet : secondaryTables.entrySet() ) {
 			if ( !secondaryTablesFromAnnotation.containsKey( entrySet.getKey() ) ) {
-				Object uncastedColumn = joinColumns.next();
+				createPrimaryColumnsToSecondaryTable( joinColumns.next(), propertyHolder, entrySet.getValue() );
-				createPrimaryColumnsToSecondaryTable( uncastedColumn, propertyHolder, entrySet.getValue() );
 			}
 		}
 	}
 
 	public void finalSecondaryTableFromAnnotationBinding(PropertyHolder propertyHolder) {
-		/*
+		 // This operation has to be done before the end of the FK second pass processing in order
-		 * Those operations have to be done before the end of the FK second pass processing in order
+		 // to find the join columns belonging to secondary tables
-		 * to find the join columns belonging to secondary tables
-		 */
 		Iterator<Object> joinColumns = secondaryTableFromAnnotationJoins.values().iterator();
-
 		for ( Map.Entry<String, Join> entrySet : secondaryTables.entrySet() ) {
 			if ( secondaryTablesFromAnnotation.containsKey( entrySet.getKey() ) ) {
-				Object uncastedColumn = joinColumns.next();
+				createPrimaryColumnsToSecondaryTable( joinColumns.next(), propertyHolder, entrySet.getValue() );
-				createPrimaryColumnsToSecondaryTable( uncastedColumn, propertyHolder, entrySet.getValue() );
 			}
 		}
 	}
 
-	private void createPrimaryColumnsToSecondaryTable(Object uncastedColumn, PropertyHolder propertyHolder, Join join) {
+	private void createPrimaryColumnsToSecondaryTable(Object column, PropertyHolder propertyHolder, Join join) {
-		AnnotatedJoinColumn[] annotatedJoinColumns;
+		final AnnotatedJoinColumn[] annotatedJoinColumns;
-		PrimaryKeyJoinColumn[] pkColumnsAnn = null;
+		final PrimaryKeyJoinColumn[] pkColumnsAnn = column instanceof PrimaryKeyJoinColumn[]
-		JoinColumn[] joinColumnsAnn = null;
+				? (PrimaryKeyJoinColumn[]) column
-		if ( uncastedColumn instanceof PrimaryKeyJoinColumn[] ) {
+				: null;
-			pkColumnsAnn = (PrimaryKeyJoinColumn[]) uncastedColumn;
+		final JoinColumn[] joinColumnsAnn = column instanceof JoinColumn[]
+				? (JoinColumn[]) column
+				: null;
+		annotatedJoinColumns = pkColumnsAnn == null && joinColumnsAnn == null
+				? createDefaultJoinColumn( propertyHolder )
+				: createJoinColumns( propertyHolder, pkColumnsAnn, joinColumnsAnn );
+
+		for (AnnotatedJoinColumn joinColumn : annotatedJoinColumns) {
+			joinColumn.forceNotNull();
 		}
-		if ( uncastedColumn instanceof JoinColumn[] ) {
+		bindJoinToPersistentClass( join, annotatedJoinColumns, context );
-			joinColumnsAnn = (JoinColumn[]) uncastedColumn;
+	}
-		}
+
-		if ( pkColumnsAnn == null && joinColumnsAnn == null ) {
+	private AnnotatedJoinColumn[] createDefaultJoinColumn(PropertyHolder propertyHolder) {
-			annotatedJoinColumns = new AnnotatedJoinColumn[1];
+		final AnnotatedJoinColumn[] annotatedJoinColumns = new AnnotatedJoinColumn[1];
-			annotatedJoinColumns[0] = AnnotatedJoinColumn.buildJoinColumn(
+		annotatedJoinColumns[0] = buildJoinColumn(
-					null,
+				null,
-					null,
+				null,
-					persistentClass.getIdentifier(),
+				persistentClass.getIdentifier(),
-					secondaryTables,
+				secondaryTables,
-					propertyHolder,
+				propertyHolder,
-					context
+				context
-			);
+		);
+		return annotatedJoinColumns;
+	}
+
+	private AnnotatedJoinColumn[] createJoinColumns(
+			PropertyHolder propertyHolder,
+			PrimaryKeyJoinColumn[] pkColumnsAnn,
+			JoinColumn[] joinColumnsAnn) {
+		final int joinColumnCount = pkColumnsAnn != null ? pkColumnsAnn.length : joinColumnsAnn.length;
+		if ( joinColumnCount == 0 ) {
+			return createDefaultJoinColumn( propertyHolder );
 		}
 		else {
-			int nbrOfJoinColumns = pkColumnsAnn != null ?
+			final AnnotatedJoinColumn[] annotatedJoinColumns = new AnnotatedJoinColumn[joinColumnCount];
-					pkColumnsAnn.length :
+			for (int colIndex = 0; colIndex < joinColumnCount; colIndex++) {
-					joinColumnsAnn.length;
+				PrimaryKeyJoinColumn pkJoinAnn = pkColumnsAnn != null ? pkColumnsAnn[colIndex] : null;
-			if ( nbrOfJoinColumns == 0 ) {
+				JoinColumn joinAnn = joinColumnsAnn != null ? joinColumnsAnn[colIndex] : null;
-				annotatedJoinColumns = new AnnotatedJoinColumn[1];
+				annotatedJoinColumns[colIndex] = buildJoinColumn(
-				annotatedJoinColumns[0] = AnnotatedJoinColumn.buildJoinColumn(
+						pkJoinAnn,
-						null,
+						joinAnn,
-						null,
 						persistentClass.getIdentifier(),
 						secondaryTables,
 						propertyHolder,
 						context
 				);
 			}
-			else {
+			return annotatedJoinColumns;
-				annotatedJoinColumns = new AnnotatedJoinColumn[nbrOfJoinColumns];
-				if ( pkColumnsAnn != null ) {
-					for (int colIndex = 0; colIndex < nbrOfJoinColumns; colIndex++) {
-						annotatedJoinColumns[colIndex] = AnnotatedJoinColumn.buildJoinColumn(
-								pkColumnsAnn[colIndex],
-								null,
-								persistentClass.getIdentifier(),
-								secondaryTables,
-								propertyHolder,
-								context
-						);
-					}
-				}
-				else {
-					for (int colIndex = 0; colIndex < nbrOfJoinColumns; colIndex++) {
-						annotatedJoinColumns[colIndex] = AnnotatedJoinColumn.buildJoinColumn(
-								null,
-								joinColumnsAnn[colIndex],
-								persistentClass.getIdentifier(),
-								secondaryTables,
-								propertyHolder,
-								context
-						);
-					}
-				}
-			}
 		}
-
-		for (AnnotatedJoinColumn joinColumn : annotatedJoinColumns) {
-			joinColumn.forceNotNull();
-		}
-		bindJoinToPersistentClass( join, annotatedJoinColumns, context );
 	}
 
 	private void bindJoinToPersistentClass(Join join, AnnotatedJoinColumn[] annotatedJoinColumns, MetadataBuildingContext buildingContext) {
@@ -928,22 +882,22 @@ public class EntityBinder {
 
 	private void setFKNameIfDefined(Join join) {
 		// just awful..
-
+		org.hibernate.annotations.Table matchingTable = findMatchingComplementaryTableAnnotation( join );
-		org.hibernate.annotations.Table matchingTable = findMatchingComplimentTableAnnotation( join );
+		final SimpleValue key = (SimpleValue) join.getKey();
-		if ( matchingTable != null && !BinderHelper.isEmptyAnnotationValue( matchingTable.foreignKey().name() ) ) {
+		if ( matchingTable != null && !isEmptyAnnotationValue( matchingTable.foreignKey().name() ) ) {
-			( (SimpleValue) join.getKey() ).setForeignKeyName( matchingTable.foreignKey().name() );
+			key.setForeignKeyName( matchingTable.foreignKey().name() );
 		}
 		else {
-			jakarta.persistence.SecondaryTable jpaSecondaryTable = findMatchingSecondaryTable( join );
+			SecondaryTable jpaSecondaryTable = findMatchingSecondaryTable( join );
 			if ( jpaSecondaryTable != null ) {
 				final boolean noConstraintByDefault = context.getBuildingOptions().isNoConstraintByDefault();
 				if ( jpaSecondaryTable.foreignKey().value() == ConstraintMode.NO_CONSTRAINT
 						|| jpaSecondaryTable.foreignKey().value() == ConstraintMode.PROVIDER_DEFAULT && noConstraintByDefault ) {
-					( (SimpleValue) join.getKey() ).disableForeignKey();
+					key.disableForeignKey();
 				}
 				else {
-					( (SimpleValue) join.getKey() ).setForeignKeyName( StringHelper.nullIfEmpty( jpaSecondaryTable.foreignKey().name() ) );
+					key.setForeignKeyName( nullIfEmpty( jpaSecondaryTable.foreignKey().name() ) );
-					( (SimpleValue) join.getKey() ).setForeignKeyDefinition( StringHelper.nullIfEmpty( jpaSecondaryTable.foreignKey().foreignKeyDefinition() ) );
+					key.setForeignKeyDefinition( nullIfEmpty( jpaSecondaryTable.foreignKey().foreignKeyDefinition() ) );
 				}
 			}
 		}
@@ -951,12 +905,10 @@ public class EntityBinder {
 
 	private SecondaryTable findMatchingSecondaryTable(Join join) {
 		final String nameToMatch = join.getTable().getQuotedName();
-
 		SecondaryTable secondaryTable = annotatedClass.getAnnotation( SecondaryTable.class );
 		if ( secondaryTable != null && nameToMatch.equals( secondaryTable.name() ) ) {
 			return secondaryTable;
 		}
-
 		SecondaryTables secondaryTables = annotatedClass.getAnnotation( SecondaryTables.class );
 		if ( secondaryTables != null ) {
 			for ( SecondaryTable secondaryTablesEntry : secondaryTables.value() ) {
@@ -965,11 +917,10 @@ public class EntityBinder {
 				}
 			}
 		}
-
 		return null;
 	}
 
-	private org.hibernate.annotations.Table findMatchingComplimentTableAnnotation(Join join) {
+	private org.hibernate.annotations.Table findMatchingComplementaryTableAnnotation(Join join) {
 		String tableName = join.getTable().getQuotedName();
 		org.hibernate.annotations.Table table = annotatedClass.getAnnotation( org.hibernate.annotations.Table.class );
 		org.hibernate.annotations.Table matchingTable = null;
@@ -990,31 +941,21 @@ public class EntityBinder {
 		return matchingTable;
 	}
 
-	public void firstLevelSecondaryTablesBinding(
-			SecondaryTable secTable, SecondaryTables secTables
-	) {
-		if ( secTables != null ) {
-			//loop through it
-			for (SecondaryTable tab : secTables.value()) {
-				addJoin( tab, null, null, false );
-			}
-		}
-		else {
-			if ( secTable != null ) addJoin( secTable, null, null, false );
-		}
-	}
-
 	//Used for @*ToMany @JoinTable
 	public Join addJoin(JoinTable joinTable, PropertyHolder holder, boolean noDelayInPkColumnCreation) {
 		return addJoin( null, joinTable, holder, noDelayInPkColumnCreation );
 	}
 
+	public Join addJoin(SecondaryTable secondaryTable, PropertyHolder holder, boolean noDelayInPkColumnCreation) {
+		return addJoin( secondaryTable, null, holder, noDelayInPkColumnCreation );
+	}
+
 	private Join addJoin(
 			SecondaryTable secondaryTable,
 			JoinTable joinTable,
 			PropertyHolder propertyHolder,
 			boolean noDelayInPkColumnCreation) {
-		// A non null propertyHolder means than we process the Pk creation without delay
+		// A non-null propertyHolder means than we process the Pk creation without delay
 		Join join = new Join();
 		join.setPersistentClass( persistentClass );
 
@@ -1071,7 +1012,8 @@ public class EntityBinder {
 				null
 		);
 
-		final InFlightMetadataCollector.EntityTableXref tableXref = context.getMetadataCollector().getEntityTableXref( persistentClass.getEntityName() );
+		final InFlightMetadataCollector.EntityTableXref tableXref
+				= context.getMetadataCollector().getEntityTableXref( persistentClass.getEntityName() );
 		assert tableXref != null : "Could not locate EntityTableXref for entity [" + persistentClass.getEntityName() + "]";
 		tableXref.addSecondaryTable( logicalName, join );
 
@@ -1084,34 +1026,35 @@ public class EntityBinder {
 
 		//somehow keep joins() for later.
 		//Has to do the work later because it needs persistentClass id!
-		LOG.debugf( "Adding secondary table to entity %s -> %s", persistentClass.getEntityName(), join.getTable().getName() );
+		LOG.debugf( "Adding secondary table to entity %s -> %s",
-		org.hibernate.annotations.Table matchingTable = findMatchingComplimentTableAnnotation( join );
+				persistentClass.getEntityName(), join.getTable().getName() );
+		org.hibernate.annotations.Table matchingTable = findMatchingComplementaryTableAnnotation( join );
 		if ( matchingTable != null ) {
 			join.setSequentialSelect( FetchMode.JOIN != matchingTable.fetch() );
 			join.setInverse( matchingTable.inverse() );
 			join.setOptional( matchingTable.optional() );
-			if ( !BinderHelper.isEmptyAnnotationValue( matchingTable.sqlInsert().sql() ) ) {
+			String insertSql = matchingTable.sqlInsert().sql();
-				join.setCustomSQLInsert( matchingTable.sqlInsert().sql().trim(),
+			if ( !isEmptyAnnotationValue(insertSql) ) {
+				join.setCustomSQLInsert(
+						insertSql.trim(),
 						matchingTable.sqlInsert().callable(),
-						ExecuteUpdateResultCheckStyle.fromExternalName(
+						fromExternalName( matchingTable.sqlInsert().check().toString().toLowerCase(Locale.ROOT) )
-								matchingTable.sqlInsert().check().toString().toLowerCase(Locale.ROOT)
-						)
 				);
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( matchingTable.sqlUpdate().sql() ) ) {
+			String updateSql = matchingTable.sqlUpdate().sql();
-				join.setCustomSQLUpdate( matchingTable.sqlUpdate().sql().trim(),
+			if ( !isEmptyAnnotationValue(updateSql) ) {
+				join.setCustomSQLUpdate(
+						updateSql.trim(),
 						matchingTable.sqlUpdate().callable(),
-						ExecuteUpdateResultCheckStyle.fromExternalName(
+						fromExternalName( matchingTable.sqlUpdate().check().toString().toLowerCase(Locale.ROOT) )
-								matchingTable.sqlUpdate().check().toString().toLowerCase(Locale.ROOT)
-						)
 				);
 			}
-			if ( !BinderHelper.isEmptyAnnotationValue( matchingTable.sqlDelete().sql() ) ) {
+			String deleteSql = matchingTable.sqlDelete().sql();
-				join.setCustomSQLDelete( matchingTable.sqlDelete().sql().trim(),
+			if ( !isEmptyAnnotationValue(deleteSql) ) {
+				join.setCustomSQLDelete(
+						deleteSql.trim(),
 						matchingTable.sqlDelete().callable(),
-						ExecuteUpdateResultCheckStyle.fromExternalName(
+						fromExternalName( matchingTable.sqlDelete().check().toString().toLowerCase(Locale.ROOT) )
-								matchingTable.sqlDelete().check().toString().toLowerCase(Locale.ROOT)
-						)
 				);
 			}
 		}
@@ -1160,10 +1103,13 @@ public class EntityBinder {
 	public void setIgnoreIdAnnotations(boolean ignoreIdAnnotations) {
 		this.ignoreIdAnnotations = ignoreIdAnnotations;
 	}
+
 	public void processComplementaryTableDefinitions(jakarta.persistence.Table table) {
-		if ( table == null ) return;
+		if ( table != null ) {
-		TableBinder.addIndexes( persistentClass.getTable(), table.indexes(), context );
+			TableBinder.addIndexes( persistentClass.getTable(), table.indexes(), context );
+		}
 	}
+
 	public void processComplementaryTableDefinitions(org.hibernate.annotations.Table table) {
 		//comment and index are processed here
 		if ( table == null ) return;
@@ -1190,10 +1136,10 @@ public class EntityBinder {
 					"@org.hibernate.annotations.Table references an unknown table: " + appliedTable
 			);
 		}
-		if ( !BinderHelper.isEmptyAnnotationValue( table.comment() ) ) {
+		if ( !isEmptyAnnotationValue( table.comment() ) ) {
 			hibTable.setComment( table.comment() );
 		}
-		if ( !BinderHelper.isEmptyAnnotationValue( table.checkConstraint() ) ) {
+		if ( !isEmptyAnnotationValue( table.checkConstraint() ) ) {
 			hibTable.addCheckConstraint( table.checkConstraint() );
 		}
 		TableBinder.addIndexes( hibTable, table.indexes(), context );

hibernate-core/src/main/java/org/hibernate/cfg/annotations/IdBagBinder.java

@@ -35,6 +35,8 @@ import org.hibernate.usertype.UserCollectionType;
 
 import jakarta.persistence.Column;
 
+import static org.hibernate.cfg.BinderHelper.makeIdGenerator;
+
 /**
  * @author Emmanuel Bernard
  */
@@ -170,7 +172,7 @@ public class IdBagBinder extends BagBinder {
 			buildingContext.getMetadataCollector().addSecondPass( secondPass );
 		}
 		else {
-			BinderHelper.makeIdGenerator(
+			makeIdGenerator(
 					id,
 					property,
 					generatorType,

hibernate-core/src/main/java/org/hibernate/cfg/annotations/ListBinder.java

@@ -34,6 +34,8 @@ import org.hibernate.usertype.UserCollectionType;
 
 import org.jboss.logging.Logger;
 
+import static org.hibernate.internal.util.StringHelper.qualify;
+
 /**
  * Bind a list to the underlying Hibernate configuration
  *
@@ -102,7 +104,7 @@ public class ListBinder extends CollectionBinder {
 	private void bindIndex(XProperty property, XClass collType, final MetadataBuildingContext buildingContext) {
 		final PropertyHolder valueHolder = PropertyHolderBuilder.buildPropertyHolder(
 				collection,
-				StringHelper.qualify( collection.getRole(), "key" ),
+				qualify( collection.getRole(), "key" ),
 				null,
 				null,
 				propertyHolder,

hibernate-core/src/main/java/org/hibernate/cfg/annotations/MapBinder.java

@@ -62,7 +62,11 @@ import jakarta.persistence.MapKeyColumn;
 import jakarta.persistence.MapKeyJoinColumn;
 import jakarta.persistence.MapKeyJoinColumns;
 
+import static org.hibernate.cfg.BinderHelper.PRIMITIVE_NAMES;
+import static org.hibernate.cfg.BinderHelper.findPropertyByName;
 import static org.hibernate.cfg.PropertyHolderBuilder.buildPropertyHolder;
+import static org.hibernate.internal.util.StringHelper.nullIfEmpty;
+import static org.hibernate.internal.util.StringHelper.qualify;
 
 /**
  * Implementation to bind a Map
@@ -174,7 +178,7 @@ public class MapBinder extends CollectionBinder {
 			//this is an EJB3 @MapKey
 			PersistentClass associatedClass = persistentClasses.get( collType.getName() );
 			if ( associatedClass == null ) throw new AnnotationException( "Associated class not found: " + collType );
-			Property mapProperty = BinderHelper.findPropertyByName( associatedClass, mapKeyPropertyName );
+			Property mapProperty = findPropertyByName( associatedClass, mapKeyPropertyName );
 			if ( mapProperty == null ) {
 				throw new AnnotationException(
 						"Map key property not found: " + collType + "." + mapKeyPropertyName
@@ -228,7 +232,7 @@ public class MapBinder extends CollectionBinder {
 			else {
 				final XClass keyXClass;
 				AnnotatedClassType classType;
-				if ( BinderHelper.PRIMITIVE_NAMES.contains( mapKeyType ) ) {
+				if ( PRIMITIVE_NAMES.contains( mapKeyType ) ) {
 					classType = AnnotatedClassType.NONE;
 					keyXClass = null;
 				}
@@ -246,7 +250,7 @@ public class MapBinder extends CollectionBinder {
 
 				CollectionPropertyHolder holder = buildPropertyHolder(
 						mapValue,
-						StringHelper.qualify( mapValue.getRole(), "mapkey" ),
+						qualify( mapValue.getRole(), "mapkey" ),
 						keyXClass,
 						property,
 						propertyHolder,
@@ -361,8 +365,8 @@ public class MapBinder extends CollectionBinder {
 						element.disableForeignKey();
 					}
 					else {
-						element.setForeignKeyName( StringHelper.nullIfEmpty( foreignKey.name() ) );
+						element.setForeignKeyName( nullIfEmpty( foreignKey.name() ) );
-						element.setForeignKeyDefinition( StringHelper.nullIfEmpty( foreignKey.foreignKeyDefinition() ) );
+						element.setForeignKeyDefinition( nullIfEmpty( foreignKey.foreignKeyDefinition() ) );
 					}
 				}
 			}

hibernate-core/src/main/java/org/hibernate/cfg/annotations/NamedEntityGraphDefinition.java

@@ -10,6 +10,8 @@ import jakarta.persistence.NamedEntityGraph;
 
 import org.hibernate.internal.util.StringHelper;
 
+import static org.hibernate.internal.util.StringHelper.isNotEmpty;
+
 /**
  * Models the definition of a {@link NamedEntityGraph} annotation
  *
@@ -25,9 +27,7 @@ public class NamedEntityGraphDefinition {
 		this.annotation = annotation;
 		this.jpaEntityName = jpaEntityName;
 		this.entityName = entityName;
-		this.name = StringHelper.isNotEmpty( annotation.name() )
+		this.name = isNotEmpty( annotation.name() ) ? annotation.name() : jpaEntityName;
-				? annotation.name()
-				: jpaEntityName;
 		if ( name == null ) {
 			throw new IllegalArgumentException( "Named entity graph name cannot be null" );
 		}

hibernate-core/src/main/java/org/hibernate/cfg/annotations/PropertyBinder.java

@@ -51,7 +51,9 @@ import org.jboss.logging.Logger;
 import java.lang.annotation.Annotation;
 import java.util.Map;
 
+import static org.hibernate.cfg.BinderHelper.getMappedSuperclassOrNull;
 import static org.hibernate.cfg.annotations.HCANNHelper.findContainingAnnotation;
+import static org.hibernate.internal.util.StringHelper.qualify;
 
 /**
  * @author Emmanuel Bernard
@@ -265,7 +267,7 @@ public class PropertyBinder {
 				}
 				else {
 					rootClass.setIdentifierProperty( prop );
-					final org.hibernate.mapping.MappedSuperclass superclass = BinderHelper.getMappedSuperclassOrNull(
+					final org.hibernate.mapping.MappedSuperclass superclass = getMappedSuperclassOrNull(
 							declaringClass,
 							inheritanceStatePerClass,
 							buildingContext
@@ -368,19 +370,19 @@ public class PropertyBinder {
 			if ( property.isAnnotationPresent(Version.class) ) {
 				throw new AnnotationException(
 						"@OptimisticLock(excluded=true) incompatible with @Version: "
-								+ StringHelper.qualify(holder.getPath(), name)
+								+ qualify( holder.getPath(), name )
 				);
 			}
 			if ( property.isAnnotationPresent(Id.class) ) {
 				throw new AnnotationException(
 						"@OptimisticLock(excluded=true) incompatible with @Id: "
-								+ StringHelper.qualify(holder.getPath(), name)
+								+ qualify( holder.getPath(), name )
 				);
 			}
 			if ( property.isAnnotationPresent(EmbeddedId.class) ) {
 				throw new AnnotationException(
 						"@OptimisticLock(excluded=true) incompatible with @EmbeddedId: "
-								+ StringHelper.qualify(holder.getPath(), name)
+								+ qualify( holder.getPath(), name )
 				);
 			}
 		}
@@ -414,7 +416,7 @@ public class PropertyBinder {
 			if ( candidate != null ) {
 				if ( valueGeneration != null ) {
 					throw new AnnotationException(
-							"Only one generator annotation is allowed: " + StringHelper.qualify(
+							"Only one generator annotation is allowed: " + qualify(
 									holder.getPath(),
 									name
 							)
@@ -451,7 +453,7 @@ public class PropertyBinder {
 
 			throw new AnnotationException(
 					"@Generated(INSERT) on a @Version property not allowed, use ALWAYS (or NEVER): "
-							+ StringHelper.qualify( holder.getPath(), name )
+							+ qualify( holder.getPath(), name )
 			);
 		}
 
@@ -487,7 +489,7 @@ public class PropertyBinder {
 		}
 		catch (Exception e) {
 			throw new AnnotationException(
-					"Exception occurred during processing of generator annotation: " + StringHelper.qualify(
+					"Exception occurred during processing of generator annotation: " + qualify(
 							holder.getPath(),
 							name
 					), e

hibernate-core/src/main/java/org/hibernate/cfg/annotations/QueryBinder.java

@@ -47,6 +47,10 @@ import jakarta.persistence.SqlResultSetMapping;
 import jakarta.persistence.SqlResultSetMappings;
 import jakarta.persistence.StoredProcedureParameter;
 
+import static org.hibernate.cfg.BinderHelper.getAnnotationValueStringOrNull;
+import static org.hibernate.cfg.BinderHelper.isEmptyAnnotationValue;
+import static org.hibernate.internal.util.collections.CollectionHelper.setOf;
+
 /**
  * Query binder
  *
@@ -63,7 +67,7 @@ public abstract class QueryBinder {
 			return;
 		}
 
-		if ( BinderHelper.isEmptyAnnotationValue( queryAnn.name() ) ) {
+		if ( isEmptyAnnotationValue( queryAnn.name() ) ) {
 			throw new AnnotationException( "A named query must have a name when used in class or package level" );
 		}
 
@@ -106,7 +110,7 @@ public abstract class QueryBinder {
 			return;
 		}
 
-		if ( BinderHelper.isEmptyAnnotationValue( queryAnn.name() ) ) {
+		if ( isEmptyAnnotationValue( queryAnn.name() ) ) {
 			throw new AnnotationException( "A named query must have a name when used in class or package level" );
 		}
 
@@ -160,7 +164,7 @@ public abstract class QueryBinder {
 		final String registrationName = queryAnn.name();
 
 		//ResultSetMappingDefinition mappingDefinition = mappings.getJdbcValuesMappingProducer( queryAnn.resultSetMapping() );
-		if ( BinderHelper.isEmptyAnnotationValue( registrationName ) ) {
+		if ( isEmptyAnnotationValue( registrationName ) ) {
 			throw new AnnotationException( "A named query must have a name when used in class or package level" );
 		}
 
@@ -175,14 +179,14 @@ public abstract class QueryBinder {
 				.setResultSetMappingClassName( resultSetMappingClassName )
 				.setQuerySpaces( null )
 				.setCacheable( queryAnn.cacheable() )
-				.setCacheRegion( BinderHelper.getAnnotationValueStringOrNull( queryAnn.cacheRegion() ) )
+				.setCacheRegion( getAnnotationValueStringOrNull( queryAnn.cacheRegion() ) )
 				.setCacheMode( getCacheMode( queryAnn.cacheMode() ) )
 				.setTimeout( queryAnn.timeout() < 0 ? null : queryAnn.timeout() )
 				.setFetchSize( queryAnn.fetchSize() < 0 ? null : queryAnn.fetchSize() )
 				.setFlushMode( getFlushMode( queryAnn.flushMode() ) )
 				.setReadOnly( queryAnn.readOnly() )
-				.setQuerySpaces( CollectionHelper.setOf( queryAnn.querySpaces() ) )
+				.setQuerySpaces( setOf( queryAnn.querySpaces() ) )
-				.setComment( BinderHelper.getAnnotationValueStringOrNull( queryAnn.comment() ) );
+				.setComment( getAnnotationValueStringOrNull( queryAnn.comment() ) );
 
 		if ( queryAnn.callable() ) {
 			final NamedProcedureCallDefinition definition = createStoredProcedure(
@@ -338,7 +342,7 @@ public abstract class QueryBinder {
 		final String registrationName = queryAnn.name();
 
 		//ResultSetMappingDefinition mappingDefinition = mappings.getJdbcValuesMappingProducer( queryAnn.resultSetMapping() );
-		if ( BinderHelper.isEmptyAnnotationValue( registrationName ) ) {
+		if ( isEmptyAnnotationValue( registrationName ) ) {
 			throw new AnnotationException( "A named query must have a name when used in class or package level" );
 		}
 
@@ -346,13 +350,13 @@ public abstract class QueryBinder {
 		final NamedHqlQueryDefinition.Builder builder = new NamedHqlQueryDefinition.Builder( registrationName )
 				.setHqlString( queryAnn.query() )
 				.setCacheable( queryAnn.cacheable() )
-				.setCacheRegion( BinderHelper.getAnnotationValueStringOrNull( queryAnn.cacheRegion() ) )
+				.setCacheRegion( getAnnotationValueStringOrNull( queryAnn.cacheRegion() ) )
 				.setCacheMode( getCacheMode( queryAnn.cacheMode() ) )
 				.setTimeout( queryAnn.timeout() < 0 ? null : queryAnn.timeout() )
 				.setFetchSize( queryAnn.fetchSize() < 0 ? null : queryAnn.fetchSize() )
 				.setFlushMode( getFlushMode( queryAnn.flushMode() ) )
 				.setReadOnly( queryAnn.readOnly() )
-				.setComment( BinderHelper.isEmptyAnnotationValue( queryAnn.comment() ) ? null : queryAnn.comment() );
+				.setComment( isEmptyAnnotationValue( queryAnn.comment() ) ? null : queryAnn.comment() );
 
 		final NamedHqlQueryDefinitionImpl hqlQueryDefinition = builder.build();
 
@@ -427,7 +431,7 @@ public abstract class QueryBinder {
 
 		final String registrationName = annotation.name();
 
-		if ( BinderHelper.isEmptyAnnotationValue( registrationName ) ) {
+		if ( isEmptyAnnotationValue( registrationName ) ) {
 			throw new AnnotationException( "A named query must have a name when used in class or package level" );
 		}
 

hibernate-core/src/main/java/org/hibernate/cfg/annotations/TableBinder.java

@@ -48,6 +48,12 @@ import org.hibernate.mapping.Value;
 
 import org.jboss.logging.Logger;
 
+import static org.hibernate.cfg.BinderHelper.isEmptyOrNullAnnotationValue;
+import static org.hibernate.internal.util.StringHelper.isNotEmpty;
+import static org.hibernate.internal.util.StringHelper.isQuoted;
+import static org.hibernate.internal.util.StringHelper.unquote;
+import static org.hibernate.internal.util.collections.CollectionHelper.arrayList;
+
 /**
  * Table related operations
  *
@@ -141,13 +147,13 @@ public class TableBinder {
 		final Identifier ownerEntityTableNameIdentifier = toIdentifier( ownerEntityTable );
 
 		//logicalName only accurate for assoc table...
-		final String unquotedOwnerTable = StringHelper.unquote( ownerEntityTable );
+		final String unquotedOwnerTable = unquote( ownerEntityTable );
-		final String unquotedAssocTable = StringHelper.unquote( associatedEntityTable );
+		final String unquotedAssocTable = unquote( associatedEntityTable );
 
 		final ObjectNameSource nameSource = buildNameContext();
 
-		final boolean ownerEntityTableQuoted = StringHelper.isQuoted( ownerEntityTable );
+		final boolean ownerEntityTableQuoted = isQuoted( ownerEntityTable );
-		final boolean associatedEntityTableQuoted = StringHelper.isQuoted( associatedEntityTable );
+		final boolean associatedEntityTableQuoted = isQuoted( associatedEntityTable );
 		final NamingStrategyHelper namingStrategyHelper = new NamingStrategyHelper() {
 			@Override
 			public Identifier determineImplicitName(final MetadataBuildingContext buildingContext) {
@@ -446,7 +452,7 @@ public class TableBinder {
 			String subselect,
 			InFlightMetadataCollector.EntityTableXref denormalizedSuperTableXref) {
 		final Identifier logicalName;
-		if ( StringHelper.isNotEmpty( nameSource.getExplicitName() ) ) {
+		if ( isNotEmpty( nameSource.getExplicitName() ) ) {
 			logicalName = namingStrategyHelper.handleExplicitName( nameSource.getExplicitName(), buildingContext );
 		}
 		else {
@@ -478,12 +484,8 @@ public class TableBinder {
 			MetadataBuildingContext buildingContext,
 			String subselect,
 			InFlightMetadataCollector.EntityTableXref denormalizedSuperTableXref) {
-		schema = BinderHelper.isEmptyOrNullAnnotationValue( schema )
+		schema = isEmptyOrNullAnnotationValue( schema )  ? null : schema;
-				? null
+		catalog = isEmptyOrNullAnnotationValue( catalog ) ? null : catalog;
-				: schema;
-		catalog = BinderHelper.isEmptyOrNullAnnotationValue( catalog )
-				? null
-				: catalog;
 
 		final Table table;
 		if ( denormalizedSuperTableXref != null ) {
@@ -543,7 +545,7 @@ public class TableBinder {
 					: columns[0].getPropertyHolder().getPersistentClass();
 		}
 		final String mappedByProperty = columns[0].getMappedBy();
-		if ( StringHelper.isNotEmpty( mappedByProperty ) ) {
+		if ( isNotEmpty( mappedByProperty ) ) {
 			/*
 			 * Get the columns of the mapped-by property
 			 * copy them and link the copy to the actual value
@@ -762,7 +764,7 @@ public class TableBinder {
 			result = java.util.Collections.emptyList();
 		}
 		else {
-			result = CollectionHelper.arrayList( annotations.length );
+			result = arrayList( annotations.length );
 			for ( UniqueConstraint uc : annotations ) {
 				result.add(
 						new UniqueConstraintHolder()

hibernate-core/src/test/java/org/hibernate/orm/test/jpa/cacheable/annotation/ConfigurationTest.java

@@ -15,7 +15,6 @@ import jakarta.persistence.SharedCacheMode;
 import org.hibernate.boot.spi.MetadataImplementor;
 import org.hibernate.cache.spi.access.AccessType;
 import org.hibernate.cfg.AvailableSettings;
-import org.hibernate.cfg.Environment;
 import org.hibernate.jpa.boot.internal.EntityManagerFactoryBuilderImpl;
 import org.hibernate.jpa.boot.spi.Bootstrap;
 import org.hibernate.testing.orm.jpa.PersistenceUnitInfoAdapter;

hibernate-core/src/test/java/org/hibernate/orm/test/persister/entity/CustomSqlSchemaResolvingIdentityTest.java

@@ -61,7 +61,7 @@ public class CustomSqlSchemaResolvingIdentityTest {
 				"DELETE FROM FOO WHERE id = ?", deleteQuery );
 		
 		assertEquals( "Incorrect custom SQL for update in  Entity: " + className,
-				"UPDATE FOO SET name = ? WHERE id = ? ", updateQuery );
+				"UPDATE FOO SET name = ? WHERE id = ?", updateQuery );
 
 		CustomEntity _entitty = scope.fromTransaction( session -> {
 			CustomEntity entity = new CustomEntity();

hibernate-core/src/test/java/org/hibernate/orm/test/persister/entity/CustomSqlSchemaResolvingTest.java

@@ -55,7 +55,7 @@ public class CustomSqlSchemaResolvingTest {
 				"DELETE FROM FOO WHERE id = ?", deleteQuery );
 		
 		assertEquals( "Incorrect custom SQL for update in  Entity: " + className,
-				"UPDATE FOO SET name = ? WHERE id = ? ", updateQuery );
+				"UPDATE FOO SET name = ? WHERE id = ?", updateQuery );
 
 		scope.inTransaction( session -> {
 			CustomEntity entity = new CustomEntity();