HHH-7612 - Fix and test

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

@@ -29,6 +29,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 
@@ -457,7 +458,7 @@ public class Table implements RelationalModel, Serializable {
 				.append( ' ' )
 				.append( name )
 				.append( " (" );
-		Iterator itr = getColumnIterator();
+		Iterator itr = getSortedColumnIterator();
 		while ( itr.hasNext() ) {
 			final Column column = (Column) itr.next();
 			buffer.append( column.getQuotedName( dialect ) ).append( ' ' );
@@ -491,7 +492,7 @@ public class Table implements RelationalModel, Serializable {
 			pkname = ( (Column) getPrimaryKey().getColumnIterator().next() ).getQuotedName( dialect );
 		}
 
-		Iterator iter = getColumnIterator();
+		Iterator iter = getSortedColumnIterator();
 		while ( iter.hasNext() ) {
 			Column col = (Column) iter.next();
 
@@ -591,6 +592,33 @@ public class Table implements RelationalModel, Serializable {
 		return buf.append( dialect.getTableTypeString() ).toString();
 	}
 
+	/**
+	 * @return Sorted column list so that primary key appears first, followed by foreign keys and other properties.
+	 * Within each group columns are not sorted in any way.
+	 */
+	private Iterator<Column> getSortedColumnIterator() {
+		final LinkedHashSet<Column> sortedColumns = new LinkedHashSet<Column>();
+		// Adding primary key columns.
+		if ( hasPrimaryKey() ) {
+			sortedColumns.addAll( getPrimaryKey().getColumns() );
+		}
+		// Adding foreign key columns.
+		Iterator iter = getForeignKeyIterator();
+		while ( iter.hasNext() ) {
+			ForeignKey fk = (ForeignKey) iter.next();
+			sortedColumns.addAll( fk.getColumns() );
+		}
+		// Adding other columns.
+		iter = getColumnIterator();
+		while ( iter.hasNext() ) {
+			final Column column = (Column) iter.next();
+			if ( ! sortedColumns.contains( column ) ) {
+				sortedColumns.add( column );
+			}
+		}
+		return sortedColumns.iterator();
+	}
+
 	public String sqlDropString(Dialect dialect, String defaultCatalog, String defaultSchema) {
 		StringBuilder buf = new StringBuilder( "drop table " );
 		if ( dialect.supportsIfExistsBeforeTableName() ) {

hibernate-core/src/main/java/org/hibernate/metamodel/spi/relational/Column.java

@@ -27,6 +27,7 @@ import org.hibernate.MappingException;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.dialect.function.SQLFunctionRegistry;
 import org.hibernate.internal.util.StringHelper;
+import org.hibernate.internal.util.compare.EqualsHelper;
 import org.hibernate.sql.Template;
 
 /**
@@ -225,4 +226,26 @@ public class Column extends AbstractValue {
 		}
 		return alias + suffix;
 	}
+
+	@Override
+	public boolean equals(Object o) {
+		if ( this == o ) {
+			return true;
+		}
+		if ( ! ( o instanceof Column ) ) {
+			return false;
+		}
+
+		final Column that = (Column) o;
+
+		return EqualsHelper.equals( this.columnName, that.columnName )
+				&& EqualsHelper.equals( this.getTable(), that.getTable() );
+	}
+
+	@Override
+	public int hashCode() {
+		int result = columnName != null ? columnName.hashCode() : 0;
+		result = 31 * result + ( getTable() != null ? getTable().hashCode() : 0 );
+		return result;
+	}
 }

hibernate-core/src/main/java/org/hibernate/metamodel/spi/relational/Table.java

@@ -26,11 +26,13 @@ package org.hibernate.metamodel.spi.relational;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.LinkedHashSet;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
 
 import org.hibernate.dialect.Dialect;
 import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment;
+import org.hibernate.internal.util.compare.EqualsHelper;
 import org.hibernate.tool.schema.extract.spi.ColumnInformation;
 import org.hibernate.tool.schema.extract.spi.TableInformation;
 
@@ -185,6 +187,30 @@ public class Table extends AbstractTableSpecification implements Exportable {
 		return "Table{name=" + exportIdentifier + '}';
 	}
 
+	@Override
+	public boolean equals(Object o) {
+		if ( this == o ) {
+			return true;
+		}
+		if ( ! ( o instanceof Table ) ) {
+			return false;
+		}
+
+		final Table that = (Table) o;
+
+		return EqualsHelper.equals( this.database, that.database )
+				&& EqualsHelper.equals( this.logicalName, that.logicalName )
+				&& EqualsHelper.equals( this.physicalName, that.physicalName );
+	}
+
+	@Override
+	public int hashCode() {
+		int result = database != null ? database.hashCode() : 0;
+		result = 31 * result + ( physicalName != null ? physicalName.hashCode() : 0 );
+		result = 31 * result + ( logicalName != null ? logicalName.hashCode() : 0 );
+		return result;
+	}
+
 	public String[] sqlAlterStrings(TableInformation tableInformation, JdbcEnvironment jdbcEnvironment) {
 		final Dialect dialect = jdbcEnvironment.getDialect();
 		final String baseAlterCommand = new StringBuilder( "alter table " )
@@ -251,4 +277,28 @@ public class Table extends AbstractTableSpecification implements Exportable {
 
 		return commands.toArray( new String[ commands.size() ] );
 	}
+
+	/**
+	 * @return Sorted column list so that primary key appears first, followed by foreign keys and other properties.
+	 * Within each group columns are not sorted in any way.
+	 */
+	public Iterable<Column> sortedColumns() {
+		final Set<Column> sortedColumns = new LinkedHashSet<Column>();
+		// Adding primary key columns.
+		sortedColumns.addAll( getPrimaryKey().getColumns() );
+		// Adding foreign key columns.
+		for ( ForeignKey fk : getForeignKeys() ) {
+			sortedColumns.addAll( fk.getColumns() );
+		}
+		// Adding other columns.
+		for ( Value value : values() ) {
+			if ( value instanceof Column ) {
+				final Column column = (Column) value;
+				if ( ! sortedColumns.contains( column ) ) {
+					sortedColumns.add( column );
+				}
+			}
+		}
+		return sortedColumns;
+	}
 }

hibernate-core/src/main/java/org/hibernate/tool/schema/internal/StandardTableExporter.java

@@ -70,17 +70,13 @@ public class StandardTableExporter implements Exporter<Table> {
 		}
 
 		boolean isFirst = true;
-		for ( Value simpleValue : table.values() ) {
+		for ( Column col : table.sortedColumns() ) {
-			if ( ! Column.class.isInstance( simpleValue ) ) {
-				continue;
-			}
 			if ( isFirst ) {
 				isFirst = false;
 			}
 			else {
 				buf.append( ", " );
 			}
-			Column col = ( Column ) simpleValue;
 			String colName = col.getColumnName().getText( dialect );
 
 			buf.append( colName ).append( ' ' );

hibernate-core/src/test/java/org/hibernate/test/annotations/onetoone/primarykey/NullablePrimaryKeyTest.java

@@ -34,8 +34,8 @@ public class NullablePrimaryKeyTest {
 			for (String s : schema) {
                 log.debug(s);
 			}
-			String expectedMappingTableSql = "create table personAddress (address_id numeric(19,0), " +
+			String expectedMappingTableSql = "create table personAddress (person_id numeric(19,0) not null, " +
-					"person_id numeric(19,0) not null, primary key (person_id))";
+					"address_id numeric(19,0), primary key (person_id))";
             Assert.assertEquals( "Wrong SQL", expectedMappingTableSql, schema[2] );
 		} catch (Exception e) {
 			Assert.fail(e.getMessage());

hibernate-core/src/test/java/org/hibernate/test/propertyref/import.sql

@@ -1,3 +1,3 @@
 INSERT INTO `vgras007_v031` VALUES ('ZZZ','00',1);
 
-INSERT INTO `vgras029_v031` VALUES (1,'Foo Foo Foo',1), (1,'Bar Bar Bar',2);
+INSERT INTO `vgras029_v031` VALUES (1,1,'Foo Foo Foo'), (1,2,'Bar Bar Bar');

hibernate-core/src/test/java/org/hibernate/test/schemaupdate/SchemaGenerationTest.java

@@ -0,0 +1,92 @@
+package org.hibernate.test.schemaupdate;
+
+import java.sql.Types;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.hibernate.cfg.Environment;
+import org.hibernate.dialect.Dialect;
+import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment;
+import org.hibernate.metamodel.spi.relational.Column;
+import org.hibernate.metamodel.spi.relational.ForeignKey;
+import org.hibernate.metamodel.spi.relational.Identifier;
+import org.hibernate.metamodel.spi.relational.Schema;
+import org.hibernate.metamodel.spi.relational.Table;
+import org.hibernate.service.ServiceRegistry;
+import org.hibernate.testing.ServiceRegistryBuilder;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseUnitTestCase;
+
+/**
+ * @author Lukasz Antoniak (lukasz dot antoniak at gmail dot com)
+ */
+public class SchemaGenerationTest extends BaseUnitTestCase {
+	private ServiceRegistry serviceRegistry;
+
+	@Before
+	public void setUp() {
+		serviceRegistry = ServiceRegistryBuilder.buildServiceRegistry( Environment.getProperties() );
+	}
+
+	@After
+	public void tearDown() {
+		if ( serviceRegistry != null ) {
+			ServiceRegistryBuilder.destroy( serviceRegistry );
+		}
+	}
+
+	@Test
+	@TestForIssue(jiraKey = "HHH-7612")
+	public void testSqlCreatePrimaryAndForeignKeyOrder() {
+		final Dialect dialect = createDialect( 32 );
+		final Table table = new Table(
+				new Schema( null, null ),
+				Identifier.toIdentifier( "col_order_table" ),
+				Identifier.toIdentifier( "col_order_table" )
+		);
+		addForeignKeyColumn( table, "fk1", "cot_fk1", dialect.getTypeName( Types.INTEGER ) );
+		addForeignKeyColumn( table, "fk1", "cot_fk2", dialect.getTypeName( Types.VARCHAR, 32, 0, 0 ) );
+		table.createColumn( "desc1" ).setSqlType( dialect.getTypeName( Types.VARCHAR, 100, 0, 0 ) );
+		addPrimaryKeyColumn( table, "pk1", "id1", dialect.getTypeName( Types.INTEGER ) );
+		addPrimaryKeyColumn( table, "pk1", "id2", dialect.getTypeName( Types.VARCHAR, 32, 0, 0 ) );
+		table.createColumn( "desc2" ).setSqlType( dialect.getTypeName( Types.BLOB )  );
+
+		final String[] sqlCreate = dialect.getTableExporter().getSqlCreateStrings( table, serviceRegistry.getService( JdbcEnvironment.class ) );
+
+		Assert.assertEquals(
+				"PK and FK columns should appear first in CREATE TABLE statement.",
+				"create table col_order_table (id1 integer, id2 varchar(32), cot_fk1 integer, cot_fk2 varchar(32), desc1 varchar(100), desc2 blob, primary key (id1, id2))",
+				sqlCreate[0]
+		);
+	}
+
+	private Dialect createDialect(final int maxAliasLength) {
+		return new Dialect() {
+			public int getMaxAliasLength() {
+				return maxAliasLength;
+			}
+		};
+	}
+
+	private void addPrimaryKeyColumn(Table table, String name, String colName, String sqlType) {
+		final Column column = table.createColumn( colName );
+		column.setSqlType( sqlType );
+		if ( table.getPrimaryKey().getName() == null ) {
+			table.getPrimaryKey().setName( name );
+		}
+		table.getPrimaryKey().addColumn( column );
+	}
+
+	private void addForeignKeyColumn(Table table, String name, String colName, String sqlType) {
+		final Column column = table.createColumn( colName );
+		column.setSqlType( sqlType );
+		ForeignKey foreignKey = table.locateForeignKey( name );
+		if ( foreignKey == null ) {
+			foreignKey = table.createForeignKey( table, name );
+		}
+		foreignKey.addColumn( column );
+	}
+}