Skip to content

Commit

Permalink
HHH-7547 include table number and column position on column alias
Browse files Browse the repository at this point in the history
  • Loading branch information
@brmeyer
brmeyer committed Aug 23, 2013
1 parent 7901118 commit 715a1e1
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 113 deletions.
39 changes: 10 additions & 29 deletions hibernate-core/src/main/java/org/hibernate/metamodel/spi/relational/Column.java
View file
Expand Up @@ -23,7 +23,6 @@
*/
package org.hibernate.metamodel.spi.relational;

import org.hibernate.MappingException;
import org.hibernate.dialect.Dialect;
import org.hibernate.dialect.function.SQLFunctionRegistry;
import org.hibernate.internal.util.StringHelper;
Expand Down Expand Up @@ -170,21 +169,19 @@ public String toLoggableString() {
return getColumnName().getText();
}

// TODO: this is fairly complicated logic. It would be more straightforward
// to always include the column position and a table number in the unique
// suffix. That might cause unreadable aliases to be generated more often though...
@Override
public String getAlias(Dialect dialect, TableSpecification tableSpecification) {
if ( tableSpecification == null ) {
// see HHH-7547 -- protect against ambiguity
throw new IllegalArgumentException("To ensure uniqueness, tableSpecification must not be null");
}

final int lastLetter = StringHelper.lastIndexOfLetter( columnName.getText() );
final String colPositionSuffix = String.valueOf( getPosition() ) + '_';
final String tableNumberSuffix =
tableSpecification == null ?
"" :
String.valueOf( tableSpecification.getTableNumber() ) + "_";
final String tableNumberSuffix = String.valueOf( tableSpecification.getTableNumber() ) + "_";
final String suffix = colPositionSuffix + tableNumberSuffix;

String alias;
String suffix = colPositionSuffix + tableNumberSuffix;
boolean useRawName = false;
if ( lastLetter == -1 ) {
alias = "column" ;
}
Expand All @@ -193,28 +190,12 @@ else if ( columnName.getText().length() > lastLetter + 1 ) {
}
else {
alias = columnName.getText();
if (columnName.getText().length() + tableNumberSuffix.length() <= dialect.getMaxAliasLength() &&
! columnName.isQuoted() &&
! columnName.getText().toLowerCase().equals( "rowid" ) ) {
useRawName = true;
suffix = tableNumberSuffix;
}
}

if ( ! useRawName ) {
if ( suffix.length() >= dialect.getMaxAliasLength() ) {
throw new MappingException(
String.format( "Unique suffix [%s%s] length must be less than maximum [%d]",
colPositionSuffix,
tableNumberSuffix,
dialect.getMaxAliasLength()
)
);
}
if ( alias.length() + suffix.length() > dialect.getMaxAliasLength() ) {
alias = alias.substring( 0, dialect.getMaxAliasLength() - suffix.length() );
}
if ( alias.length() + suffix.length() > dialect.getMaxAliasLength() ) {
alias = alias.substring( 0, dialect.getMaxAliasLength() - suffix.length() );
}

return alias + suffix;
}

Expand Down
3 changes: 2 additions & 1 deletion ...te-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java
View file
Expand Up @@ -857,7 +857,8 @@ public AbstractCollectionPersister(
for ( int i = 0 ; i < indexSpan ; i++ ) {
final RelationalValueBinding rb = indexRelationalValueBindings.get( i );
final Value value = rb.getValue();
indexColumnAliases[ i ] = value.getAlias( dialect, null );
indexColumnAliases[ i ] = value.getAlias( dialect,
collection.getContainer().seekEntityBinding().getPrimaryTable() );
if ( !rb.isDerived() ) {
indexColumnIsSettable[ i ] = true;
Column column = ( Column ) value;
Expand Down
101 changes: 18 additions & 83 deletions hibernate-core/src/test/java/org/hibernate/metamodel/spi/relational/ColumnAliasTest.java
View file
Expand Up @@ -23,18 +23,15 @@
*/
package org.hibernate.metamodel.spi.relational;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

import org.hibernate.MappingException;
import org.hibernate.dialect.Dialect;
import org.hibernate.testing.FailureExpectedWithNewMetamodel;
import org.hibernate.testing.junit4.BaseUnitTestCase;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

/**
*
Expand Down Expand Up @@ -84,11 +81,9 @@ public void testNoCharactersInNameNoTruncation() {
final Dialect dialect = createDialect( 25 );

Column column = table0.createColumn( "1" );
assertEquals( "column" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "column" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );

column = table0.createColumn( "`1`" );
assertEquals( "column" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "column" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );
}

Expand All @@ -97,19 +92,15 @@ public void testNameStartsWithNonCharacterNoTruncation() {
final Dialect dialect = createDialect( 25 );

Column column = table0.createColumn( "1abc" );
assertEquals( "column" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "column" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );

column = table0.createColumn( "1abc`" );
assertEquals( "column" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "column" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );

column = table0.createColumn( "_abc" );
assertEquals( "column" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "column" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );

column = table0.createColumn( "`_abc`" );
assertEquals( "column" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "column" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );
}

Expand Down Expand Up @@ -144,42 +135,32 @@ public void testNameIncludingNonCharacter() {
final Dialect dialect = createDialect( 10 );

Column column = table0.createColumn( "a1" );
assertEquals( "a" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "a" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );
column = table0.createColumn( "`a1`" );
assertEquals( "a" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "a" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );

column = table0.createColumn( "a1b" );
assertEquals( "a" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "a" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );

column = table0.createColumn( "`a1b`" );
assertEquals( "a" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "a" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );

column = table0.createColumn( "a_b" );
assertEquals( "a" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "a" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );

column = table0.createColumn( "`a_b`" );
assertEquals( "a" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "a" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );

column = table0.createColumn( "ab1" );
assertEquals( "ab" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "ab" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );

column = table0.createColumn( "`ab1`" );
assertEquals( "ab" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "ab" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );

column = table0.createColumn( "a1b2" );
assertEquals( "a" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "a" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );

column = table0.createColumn( "`a1b2`" );
assertEquals( "a" + getExpectedSuffix( column, null ) , column.getAlias( dialect, null ) );
assertEquals( "a" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );
}

Expand All @@ -189,33 +170,16 @@ public void testUseNameAsIs() {
final Dialect dialect = createDialect( 25 );

Column column = table0.createColumn( "abc" );
assertEquals( "abc" + getExpectedSuffix( null, null ) , column.getAlias( dialect, null ) );

column = table0.createColumn( "abc" );
assertEquals( "abc" + getExpectedSuffix( null, table1 ) , column.getAlias( dialect, table1 ) );
assertEquals( "abc" + getExpectedSuffix( column, table1 ) , column.getAlias( dialect, table1 ) );
}

@Test
public void testUseNameAsIsWithMaxLengthNoTableSuffix() {
public void testUseNameAsIsWithMaxLength() {
// create dialect with a large enough max alias length so there is no trucation.
final Dialect dialect = createDialect( 10 );
String name = "abcdefghij";
String name = "abcdef";
Column column = table0.createColumn( name );
assertEquals( dialect.getMaxAliasLength(), column.getColumnName().getText().length() );
assertEquals( name + getExpectedSuffix( null, null ) , column.getAlias( dialect, null ) );
}

@Test
public void testUseNameAsIsWithMaxLengthWithTableSuffix() {
// create dialect with a large enough max alias length so there is no trucation.
final Dialect dialect = createDialect( 10 );
String name = "abcdefghij";
assertEquals( dialect.getMaxAliasLength(), name.length() );
name = name.substring( 0, name.length() - getExpectedTableSuffix( table1 ).length() );
Column column = table0.createColumn( name );
String expectedAlias = name + getExpectedTableSuffix( table1 );
assertEquals( dialect.getMaxAliasLength(), expectedAlias.length() );
assertEquals( expectedAlias , column.getAlias( dialect, table1 ) );
assertEquals( name + getExpectedSuffix( column, table0 ) , column.getAlias( dialect, table0 ) );
}

@Test
Expand All @@ -225,9 +189,9 @@ public void testQuotedNameAllCharactersNoTrucation() {

String name = "`abc`";
Column column = table0.createColumn( name );
assertEquals( column.getColumnName().getText() + getExpectedSuffix( column, null ), column.getAlias(
assertEquals( column.getColumnName().getText() + getExpectedSuffix( column, table0 ), column.getAlias(
dialect,
null
table0
) );
assertEquals( column.getColumnName().getText() + getExpectedSuffix( column, table1 ), column.getAlias( dialect, table1 ) );
}
Expand All @@ -238,22 +202,20 @@ public void testRowIdNameNoTruncation() {
final Dialect dialect = createDialect( 25 );

Column column = table0.createColumn( "RowId" );
assertEquals( "RowId" + getExpectedSuffix( column, null ), column.getAlias( dialect, null ) );
assertEquals( "RowId" + getExpectedSuffix( column, table1 ), column.getAlias( dialect, table1 ) );

column = table0.createColumn( "`rowid`" );
assertEquals( "rowid" + getExpectedSuffix( column, null ), column.getAlias( dialect, null ) );
assertEquals( "rowid" + getExpectedSuffix( column, table1 ), column.getAlias( dialect, table1 ) );
}

@Test
public void testRowIdNameTruncation() {
Column column = table0.createColumn( "RowId" );
String expectedSuffix = getExpectedSuffix( column, null );
String expectedSuffix = getExpectedSuffix( column, table0 );
Dialect dialect = createDialect( column.getColumnName().getText().length() + expectedSuffix.length() - 1 );
String nameTruncated = "RowId".substring( 0, dialect.getMaxAliasLength() - expectedSuffix.length() );
assertTrue( nameTruncated.length() < "RowId".length() );
String alias = column.getAlias( dialect, null );
String alias = column.getAlias( dialect, table0 );
assertEquals( dialect.getMaxAliasLength(), alias.length() );
assertEquals( nameTruncated + expectedSuffix, alias );

Expand All @@ -269,7 +231,7 @@ public void testRowIdNameTruncation() {
@Test
public void testTruncatedName() {
Column column = table0.createColumn( "abcdefghijk" );
String expectedSuffix = getExpectedSuffix( column, null );
String expectedSuffix = getExpectedSuffix( column, table0 );
// Force max alias length to be less than the column name to that
// the name is not used as is (and the expected suffix will be used).
Dialect dialect = createDialect( column.getColumnName().getText().length() - 1 );
Expand All @@ -279,7 +241,7 @@ public void testTruncatedName() {
0,
dialect.getMaxAliasLength() - expectedSuffix.length()
);
String alias = column.getAlias( dialect, null );
String alias = column.getAlias( dialect, table0 );
assertEquals( dialect.getMaxAliasLength(), alias.length() );
assertEquals( nameTruncated + expectedSuffix, alias );

Expand All @@ -298,14 +260,14 @@ public void testTruncatedName() {
@Test
public void testTruncatedQuotedName() {
Column column = table0.createColumn( "`abcdefghijk`" );
String expectedSuffix = getExpectedSuffix( column, null );
String expectedSuffix = getExpectedSuffix( column, table0 );
Dialect dialect = createDialect( column.getColumnName().getText().length() + expectedSuffix.length() - 1 );
String nameTruncated =
column.getColumnName().getText().substring(
0,
dialect.getMaxAliasLength() - expectedSuffix.length()
);
String alias = column.getAlias( dialect, null );
String alias = column.getAlias( dialect, table0 );
assertEquals( dialect.getMaxAliasLength(), alias.length() );
assertEquals( nameTruncated + expectedSuffix, alias );

Expand All @@ -321,25 +283,6 @@ public void testTruncatedQuotedName() {
assertEquals( nameTruncated + expectedSuffix, alias );
}

@Test
public void testMaxAliasLengthTooSmall() {
Column column = table0.createColumn( "a" );
try {
column.getAlias(
new Dialect() {
public int getMaxAliasLength() {
return 2;
}
},
table1
);
fail( "should have failed because max alias length cannot accommodate more than just the unique suffix");
}
catch (MappingException exception) {
// expected
}
}

private Dialect createDialect(final int maxAliasLength) {
return new Dialect() {
public int getMaxAliasLength() {
Expand All @@ -349,14 +292,6 @@ public int getMaxAliasLength() {
}

private String getExpectedSuffix(Column column, TableSpecification table) {
return getExpectedColumnSuffix( column ) + getExpectedTableSuffix( table );
}

private String getExpectedColumnSuffix(Column column) {
return column == null ? "" : String.valueOf( column.getPosition() ) + "_";
}

private String getExpectedTableSuffix(TableSpecification table) {
return table == null ? "" : String.valueOf( table.getTableNumber() ) + "_";
return String.valueOf( column.getPosition() ) + "_" + String.valueOf( table.getTableNumber() ) + "_" ;
}
}

0 comments on commit 715a1e1

Please sign in to comment.