Skip to content

Commit

Permalink
HHH-16531 ColumnDefinitions should respect @column(columnDefinition)
Browse files Browse the repository at this point in the history
  • Loading branch information
quaff committed Apr 15, 2024
1 parent adec141 commit 0bb331d
Show file tree
Hide file tree
Showing 11 changed files with 141 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ public class AnnotatedColumn {
private boolean updatable = true;
private String explicitTableName; // the JPA @Column annotation lets you specify a table name
private boolean isImplicit;
public String sqlType;
private String columnDefinition;
protected String sqlType;
private Long length;
private Integer precision;
private Integer scale;
Expand Down Expand Up @@ -109,6 +110,10 @@ public String getLogicalColumnName() {
return logicalColumnName;
}

public String getColumnDefinition() {
return columnDefinition;
}

public String getSqlType() {
return sqlType;
}
Expand Down Expand Up @@ -169,6 +174,10 @@ public void setImplicit(boolean implicit) {
isImplicit = implicit;
}

public void setColumnDefinition(String columnDefinition) {
this.columnDefinition = nullIfEmpty(columnDefinition);
}

public void setSqlType(String sqlType) {
this.sqlType = sqlType;
}
Expand Down Expand Up @@ -290,6 +299,7 @@ protected void initMappingColumn(
mappingColumn = new Column();
mappingColumn.setExplicit( !isImplicit );
redefineColumnName( columnName, propertyName, applyNamingStrategy );
mappingColumn.setColumnDefinition( columnDefinition );
mappingColumn.setLength( length );
if ( precision != null && precision > 0 ) { //relevant precision
mappingColumn.setPrecision( precision );
Expand Down Expand Up @@ -769,6 +779,7 @@ private static AnnotatedColumn buildColumn(
final AnnotatedColumn annotatedColumn = new AnnotatedColumn();
annotatedColumn.setLogicalColumnName( columnName );
annotatedColumn.setImplicit( false );
annotatedColumn.setColumnDefinition( column.columnDefinition() );
annotatedColumn.setSqlType( sqlType );
annotatedColumn.setLength( (long) column.length() );
if ( fractionalSeconds != null ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ else if ( discriminatorColumn != null ) {
discriminatorType = discriminatorColumn.discriminatorType();
column.setImplicit( false );
if ( !discriminatorColumn.columnDefinition().isEmpty() ) {
column.setColumnDefinition( discriminatorColumn.columnDefinition() );
column.setSqlType( discriminatorColumn.columnDefinition() );
}
if ( !discriminatorColumn.name().isEmpty() ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ public void applyJoinAnnotation(JoinColumn joinColumn, String defaultName) {
else {
setImplicit( false );
if ( !joinColumn.columnDefinition().isEmpty() ) {
setColumnDefinition( joinColumn.columnDefinition() );
setSqlType( getBuildingContext().getObjectNameNormalizer()
.applyGlobalQuoting( joinColumn.columnDefinition() ) );
}
Expand Down Expand Up @@ -426,6 +427,7 @@ public void overrideFromReferencedColumnIfNecessary(Column column) {
}

// these properties can only be applied on the referenced column - we can just take them over
mappingColumn.setColumnDefinition( column.getColumnDefinition() );
mappingColumn.setLength( column.getLength() );
mappingColumn.setPrecision( column.getPrecision() );
mappingColumn.setScale( column.getScale() );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ private Property createSimpleProperty(

private void applyComponentColumnSizeValueToJoinColumn(Column column, AnnotatedJoinColumn joinColumn) {
final Column mappingColumn = joinColumn.getMappingColumn();
mappingColumn.setColumnDefinition( column.getColumnDefinition() );
mappingColumn.setLength( column.getLength() );
mappingColumn.setPrecision( column.getPrecision() );
mappingColumn.setScale( column.getScale() );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ public static IndexColumn buildColumnFromAnnotation(
: orderColumn.name();
final IndexColumn column = new IndexColumn();
column.setLogicalColumnName( name );
column.setColumnDefinition( orderColumn.columnDefinition() );
column.setSqlType( sqlType );
column.setNullable( orderColumn.nullable() );
// column.setJoins( secondaryTables );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class AggregateColumn extends Column {
private final Component component;

public AggregateColumn(Column column, Component component) {
setColumnDefinition( column.getColumnDefinition() );
setLength( column.getLength() );
setPrecision( column.getPrecision() );
setScale( column.getScale() );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
*/
public class Column implements Selectable, Serializable, Cloneable, ColumnTypeInformation {

private String columnDefinition; // column definition specified by user
private Long length;
private Integer precision;
private Integer scale;
Expand Down Expand Up @@ -87,6 +88,14 @@ public Column(String columnName) {
setName( columnName );
}

public String getColumnDefinition() {
return columnDefinition;
}

public void setColumnDefinition(String columnDefinition) {
this.columnDefinition = columnDefinition;
}

public Long getLength() {
return length;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ public void alignColumns() {
for ( int i = 0; i<columnSpan; i++ ) {
Column referencedColumn = primaryKey.getColumn(i);
Column referencingColumn = getColumn(i);
referencingColumn.setColumnDefinition( referencingColumn.getColumnDefinition() );
referencingColumn.setLength( referencedColumn.getLength() );
referencingColumn.setScale( referencedColumn.getScale() );
referencingColumn.setPrecision( referencedColumn.getPrecision() );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ protected void validateColumnType(
Metadata metadata,
ExecutionOptions options,
Dialect dialect) {
if ( !ColumnDefinitions.hasMatchingType( column, columnInformation, metadata, dialect ) ) {
if ( !ColumnDefinitions.hasMatchingType( column, columnInformation, metadata, dialect, true ) ) {
throw new SchemaManagementException(
String.format(
"Schema-validation: wrong column type encountered in column [%s] in " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,19 @@
class ColumnDefinitions {

static boolean hasMatchingType(Column column, ColumnInformation columnInformation, Metadata metadata, Dialect dialect) {
return hasMatchingType( column, columnInformation, metadata, dialect, false );
}

static boolean hasMatchingType(Column column, ColumnInformation columnInformation, Metadata metadata, Dialect dialect, boolean forValidation) {
if ( !forValidation && column.getColumnDefinition() != null ) {
// validation shouldn't use columnDefinition
boolean match = extractTypeName( column.getColumnDefinition() ).equals( normalize( columnInformation.getTypeName() ) );
if ( !match ) {
return false;
}
}
boolean typesMatch = dialect.equivalentTypes( column.getSqlTypeCode(metadata), columnInformation.getTypeCode() )
|| normalize( stripArgs( column.getSqlType( metadata ) ) ).equals( normalize( columnInformation.getTypeName() ) );
|| extractTypeName( column.getSqlType( metadata ) ).equals( normalize( columnInformation.getTypeName() ) );
if ( typesMatch ) {
return true;
}
Expand Down Expand Up @@ -268,13 +279,23 @@ private static String normalize(String typeName) {
}
}


private static String extractTypeName(String typeExpression) {
String typeName = typeExpression.toLowerCase(Locale.ROOT);
typeName = keepBefore( typeName, " default " ); // Strip default value
typeName = keepBefore( typeName, " not null" );
typeName = stripArgs( typeName );
typeName = normalize( typeName );
return typeName;
}

private static String keepBefore(String input, String token) {
int i = input.indexOf( token );
return i > 0 ? input.substring( 0, i ).trim() : input;
}

private static String stripArgs(String typeExpression) {
if ( typeExpression == null ) {
return null;
}
else {
int i = typeExpression.indexOf('(');
return i>0 ? typeExpression.substring(0,i).trim() : typeExpression;
}
// 'timestamp(6) with time zone' should be 'timestamp with time zone'
return typeExpression.replaceAll("\\(.*\\)", "").trim();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Hibernate, Relational Persistence for Idiomatic Java
*
* License: GNU Lesser General Public License (LGPL), version 2.1 or later.
* See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
*/
package org.hibernate.tool.schema.internal;

import org.hibernate.boot.Metadata;
import org.hibernate.boot.MetadataSources;
import org.hibernate.boot.model.TruthValue;
import org.hibernate.mapping.Column;
import org.hibernate.mapping.Value;
import org.hibernate.testing.util.ServiceRegistryUtil;
import org.hibernate.tool.schema.extract.internal.ColumnInformationImpl;
import org.hibernate.tool.schema.extract.spi.ColumnInformation;
import org.hibernate.type.JavaObjectType;
import org.hibernate.type.SqlTypes;
import org.junit.jupiter.api.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;

/**
* @author Yanming Zhou
*/
public class ColumnDefinitionsTest {

@Test
public void matchIntegerType() {
assertHasMatchingType("integer", SqlTypes.INTEGER, "integer", SqlTypes.INTEGER, true);
assertHasMatchingType("integer not null", SqlTypes.INTEGER, "integer", SqlTypes.INTEGER, true);
assertHasMatchingType("integer default 0", SqlTypes.INTEGER, "integer", SqlTypes.INTEGER, true);
assertHasMatchingType("integer not null default 0", SqlTypes.INTEGER, "integer", SqlTypes.INTEGER, true);
assertHasMatchingType("integer", SqlTypes.INTEGER, "int", SqlTypes.INTEGER, true);
assertHasMatchingType("int", SqlTypes.INTEGER, "integer", SqlTypes.INTEGER, true);
assertHasMatchingType("integer", SqlTypes.INTEGER, "bigint", SqlTypes.BIGINT, false);
assertHasMatchingType("bigint", SqlTypes.BIGINT, "integer", SqlTypes.INTEGER, false);
}

@Test
public void matchDecimalType() {
assertHasMatchingType("decimal(10,2)", SqlTypes.DECIMAL, "decimal", SqlTypes.DECIMAL, true);
assertHasMatchingType("decimal( 10 , 2 )", SqlTypes.DECIMAL, "decimal", SqlTypes.DECIMAL, true);
}

@Test
public void matchComplexCharacterVaryingType() {
assertHasMatchingType("character varying(255) not null default '-'", SqlTypes.VARCHAR, "varchar", SqlTypes.VARCHAR, true);
}

@Test
public void shouldNotMatchIfColumnDefinitionNotEqualsToActualTypeNameButInferredTypeCodeEqualsToActualTypeCode() {
assertHasMatchingType("integer not null default 0", SqlTypes.BIGINT, "bigint", SqlTypes.BIGINT, false);
}

private void assertHasMatchingType(String columnDefinition, int inferredTypeCode, String actualTypeName, int actualTypeCode, boolean matching) {
Column column = new Column();
Value value = mock();
given(value.getType()).willReturn(JavaObjectType.INSTANCE);
column.setValue(value);
column.setColumnDefinition(columnDefinition);
column.setSqlType(columnDefinition);
column.setSqlTypeCode(inferredTypeCode);

ColumnInformation columnInformation = new ColumnInformationImpl(
null,
null,
actualTypeCode,
actualTypeName,
255,
0,
TruthValue.TRUE
);

Metadata metadata = new MetadataSources( ServiceRegistryUtil.serviceRegistry() ).buildMetadata();
assertThat(ColumnDefinitions.hasMatchingType(column, columnInformation, metadata, metadata.getDatabase().getDialect()),
is(matching));
}

}

0 comments on commit 0bb331d

Please sign in to comment.