From f675b67c27e5c3025c80ec37baddc22e6ba6ab43 Mon Sep 17 00:00:00 2001 From: Brett Meyer Date: Thu, 10 Oct 2013 11:27:28 -0400 Subject: [PATCH] HHH-8228 cleanup, formatting, check for empty constraint alter table statements before executing --- .../dialect/AbstractHANADialect.java | 137 +++++++++--------- .../dialect/HANAColumnStoreDialect.java | 8 +- .../dialect/HANARowStoreDialect.java | 7 +- .../internal/StandardDialectResolver.java | 1 + .../org/hibernate/mapping/Constraint.java | 18 ++- .../test/hql/ASTParserLoadingTest.java | 11 +- 6 files changed, 91 insertions(+), 91 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/AbstractHANADialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/AbstractHANADialect.java index 314559506f..41cf9f183f 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/AbstractHANADialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/AbstractHANADialect.java @@ -63,18 +63,19 @@ import org.hibernate.type.descriptor.sql.SqlTypeDescriptor; /** * An abstract base class for HANA dialects.
* SAP HANA Reference
- * This dialect is currently configured to not create foreign keys by - * returning the empty string from {@link #getAddForeignKeyConstraintString} - * since they currently have caveats compared to other databases. This does not + * + * NOTE: This dialect is currently configured to not create foreign keys by + * returning the empty string from {@link #getAddForeignKeyConstraintString} since they currently have caveats compared + * to other databases. This does not * affect using this dialect with your own DDL scripts which use foreign keys. - * + * * @author Andrew Clemons */ public abstract class AbstractHANADialect extends Dialect { private static class CloseSuppressingReader extends FilterReader { - protected CloseSuppressingReader( final Reader in ) { - super(in); + protected CloseSuppressingReader(final Reader in) { + super( in ); } @Override @@ -91,56 +92,58 @@ public abstract class AbstractHANADialect extends Dialect { // using non-contexual lob creation and HANA then closes our StringReader. // see test case LobLocatorTest - private static final ClobTypeDescriptor HANA_CLOB_STREAM_BINDING = - new ClobTypeDescriptor() { - /** serial version uid. */ - private static final long serialVersionUID = -379042275442752102L; - - @Override - public BasicBinder getClobBinder( final JavaTypeDescriptor javaTypeDescriptor ) { - return new BasicBinder( javaTypeDescriptor, this ) { - @Override - protected void doBind( final PreparedStatement st, final X value, final int index, final WrapperOptions options ) - throws SQLException { - final CharacterStream characterStream = javaTypeDescriptor.unwrap( value, CharacterStream.class, options ); - - if (value instanceof ClobImplementer) { - st.setCharacterStream( index, new CloseSuppressingReader(characterStream.asReader()), characterStream.getLength() ); - } else { - st.setCharacterStream( index, characterStream.asReader(), characterStream.getLength() ); - } - - } - }; - } - }; - - private static final NClobTypeDescriptor HANA_NCLOB_STREAM_BINDING = - new NClobTypeDescriptor() { - /** serial version uid. */ - private static final long serialVersionUID = 5651116091681647859L; + private static final ClobTypeDescriptor HANA_CLOB_STREAM_BINDING = new ClobTypeDescriptor() { + /** serial version uid. */ + private static final long serialVersionUID = -379042275442752102L; + @Override + public BasicBinder getClobBinder(final JavaTypeDescriptor javaTypeDescriptor) { + return new BasicBinder( javaTypeDescriptor, this ) { @Override - public BasicBinder getNClobBinder( final JavaTypeDescriptor javaTypeDescriptor ) { - return new BasicBinder( javaTypeDescriptor, this ) { - @Override - protected void doBind( final PreparedStatement st, final X value, final int index, final WrapperOptions options ) - throws SQLException { - final CharacterStream characterStream = javaTypeDescriptor.unwrap( value, CharacterStream.class, options ); + protected void doBind(final PreparedStatement st, final X value, final int index, + final WrapperOptions options) throws SQLException { + final CharacterStream characterStream = javaTypeDescriptor.unwrap( value, CharacterStream.class, + options ); - if (value instanceof NClobImplementer) { - st.setCharacterStream( index, new CloseSuppressingReader(characterStream.asReader()), characterStream.getLength() ); - } else { - st.setCharacterStream( index, characterStream.asReader(), characterStream.getLength() ); - } + if ( value instanceof ClobImplementer ) { + st.setCharacterStream( index, new CloseSuppressingReader( characterStream.asReader() ), + characterStream.getLength() ); + } + else { + st.setCharacterStream( index, characterStream.asReader(), characterStream.getLength() ); + } - } - }; } }; + } + }; + + private static final NClobTypeDescriptor HANA_NCLOB_STREAM_BINDING = new NClobTypeDescriptor() { + /** serial version uid. */ + private static final long serialVersionUID = 5651116091681647859L; + + @Override + public BasicBinder getNClobBinder(final JavaTypeDescriptor javaTypeDescriptor) { + return new BasicBinder( javaTypeDescriptor, this ) { + @Override + protected void doBind(final PreparedStatement st, final X value, final int index, + final WrapperOptions options) throws SQLException { + final CharacterStream characterStream = javaTypeDescriptor.unwrap( value, CharacterStream.class, + options ); + + if ( value instanceof NClobImplementer ) { + st.setCharacterStream( index, new CloseSuppressingReader( characterStream.asReader() ), + characterStream.getLength() ); + } + else { + st.setCharacterStream( index, characterStream.asReader(), characterStream.getLength() ); + } + + } + }; + } + }; - /** - */ public AbstractHANADialect() { super(); @@ -359,7 +362,6 @@ public abstract class AbstractHANADialect extends Dialect { return new ConstraintViolationException( message, sqlException, sql, constraintName ); } - return null; } }; @@ -372,7 +374,7 @@ public abstract class AbstractHANADialect extends Dialect { @Override public String getAddColumnString() { - return "add ("; + return "add ("; } @Override @@ -386,7 +388,7 @@ public abstract class AbstractHANADialect extends Dialect { } @Override - public String getCreateSequenceString( final String sequenceName ) { + public String getCreateSequenceString(final String sequenceName) { return "create sequence " + sequenceName; } @@ -401,17 +403,17 @@ public abstract class AbstractHANADialect extends Dialect { } @Override - public String getDropSequenceString( final String sequenceName ) { + public String getDropSequenceString(final String sequenceName) { return "drop sequence " + sequenceName; } @Override - public String getForUpdateString( final String aliases ) { + public String getForUpdateString(final String aliases) { return getForUpdateString() + " of " + aliases; } @Override - public String getForUpdateString( final String aliases, final LockOptions lockOptions ) { + public String getForUpdateString(final String aliases, final LockOptions lockOptions) { LockMode lockMode = lockOptions.getLockMode(); final Iterator> itr = lockOptions.getAliasLockIterator(); while ( itr.hasNext() ) { @@ -433,13 +435,13 @@ public abstract class AbstractHANADialect extends Dialect { @Deprecated @Override - public String getLimitString( final String sql, final boolean hasOffset ) { + public String getLimitString(final String sql, final boolean hasOffset) { return new StringBuilder( sql.length() + 20 ).append( sql ) .append( hasOffset ? " limit ? offset ?" : " limit ?" ).toString(); } @Override - public String getNotExpression( final String expression ) { + public String getNotExpression(final String expression) { return "not (" + expression + ")"; } @@ -449,17 +451,17 @@ public abstract class AbstractHANADialect extends Dialect { } @Override - public String getSelectSequenceNextValString( final String sequenceName ) { + public String getSelectSequenceNextValString(final String sequenceName) { return sequenceName + ".nextval"; } @Override - public String getSequenceNextValString( final String sequenceName ) { + public String getSequenceNextValString(final String sequenceName) { return "select " + getSelectSequenceNextValString( sequenceName ) + " from dummy"; } @Override - protected SqlTypeDescriptor getSqlTypeDescriptorOverride( final int sqlCode ) { + protected SqlTypeDescriptor getSqlTypeDescriptorOverride(final int sqlCode) { switch ( sqlCode ) { case Types.BOOLEAN: return BitTypeDescriptor.INSTANCE; @@ -572,12 +574,11 @@ public abstract class AbstractHANADialect extends Dialect { return false; } - /** - * HANA does support cascade deletes, but since we have overridden the - * foreign key support, this should also be false. - */ @Override public boolean supportsCascadeDelete() { + // HANA does support cascade deletes, but since we have (temporarily) overridden the foreign key support, + // this should also be false. + // TODO: Enable once FK support is solidified and getAddForeignKeyConstraintString is corrected. return false; } @@ -673,13 +674,13 @@ public abstract class AbstractHANADialect extends Dialect { /** * Currently disabling foreign key creation when using Hibernate's auto-ddl - * feature. HANA does allow creating foreign keys, but they do not always + * feature. HANA does allow creating foreign keys, but currently they do not always * behave as expected. */ @Override - public String getAddForeignKeyConstraintString( final String constraintName, - final String[] foreignKey, final String referencedTable, - final String[] primaryKey, final boolean referencesPrimaryKey ) { + public String getAddForeignKeyConstraintString(final String constraintName, final String[] foreignKey, + final String referencedTable, final String[] primaryKey, final boolean referencesPrimaryKey) { + // TODO: Re-evaluate in a later HANA release where, hopefully, this has been solidified. return ""; } } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/HANAColumnStoreDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/HANAColumnStoreDialect.java index ad62a219f2..e00c7d89af 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/HANAColumnStoreDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/HANAColumnStoreDialect.java @@ -27,14 +27,11 @@ package org.hibernate.dialect; * An SQL dialect for HANA.
* SAP HANA Reference
* Column tables are created by this dialect when using the auto-ddl feature. - * This dialect was tested with HANA Rev 67 and HDB JDBC 1.00.67.383230. - * + * * @author Andrew Clemons */ public class HANAColumnStoreDialect extends AbstractHANADialect { - /** - */ public HANAColumnStoreDialect() { super(); } @@ -43,7 +40,4 @@ public class HANAColumnStoreDialect extends AbstractHANADialect { public String getCreateTableString() { return "create column table"; } - - - } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/HANARowStoreDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/HANARowStoreDialect.java index 5af28ab210..c17b6eb564 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/HANARowStoreDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/HANARowStoreDialect.java @@ -27,16 +27,15 @@ package org.hibernate.dialect; * An SQL dialect for HANA.
* SAP HANA Reference
* Row tables are created by this dialect when using the auto-ddl feature. - * This dialect was tested with HANA Rev 67 and HDB JDBC 1.00.67.383230. * * @author Andrew Clemons */ public class HANARowStoreDialect extends AbstractHANADialect { + + // Even though it's currently pointless, provide this structure in case HANA row store merits additional + // differences in the future. - /** - */ public HANARowStoreDialect() { super(); } - } diff --git a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/dialect/internal/StandardDialectResolver.java b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/dialect/internal/StandardDialectResolver.java index b99ae51f1b..50b8a88000 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/jdbc/dialect/internal/StandardDialectResolver.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/jdbc/dialect/internal/StandardDialectResolver.java @@ -208,6 +208,7 @@ public class StandardDialectResolver implements DialectResolver { } if ( "HDB".equals( databaseName ) ) { + // SAP recommends defaulting to column store. return new HANAColumnStoreDialect(); } diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Constraint.java b/hibernate-core/src/main/java/org/hibernate/mapping/Constraint.java index e11fbdd9dd..cfe1869e60 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/Constraint.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/Constraint.java @@ -33,6 +33,7 @@ import java.util.Iterator; import java.util.List; import org.hibernate.HibernateException; +import org.hibernate.annotations.common.util.StringHelper; import org.hibernate.dialect.Dialect; import org.hibernate.engine.spi.Mapping; @@ -198,15 +199,18 @@ public abstract class Constraint implements RelationalModel, Serializable { public String sqlCreateString(Dialect dialect, Mapping p, String defaultCatalog, String defaultSchema) { if ( isGenerated( dialect ) ) { + // Certain dialects (ex: HANA) don't support FKs as expected, but other constraints can still be created. + // If that's the case, hasAlterTable() will be true, but getAddForeignKeyConstraintString will return + // empty string. Prevent blank "alter table" statements. String constraintString = sqlConstraintString( dialect, getName(), defaultCatalog, defaultSchema ); - StringBuilder buf = new StringBuilder( "alter table " ) - .append( getTable().getQualifiedName( dialect, defaultCatalog, defaultSchema ) ) - .append( constraintString ); - return buf.toString(); - } - else { - return null; + if ( !StringHelper.isEmpty( constraintString ) ) { + StringBuilder buf = new StringBuilder( "alter table " ) + .append( getTable().getQualifiedName( dialect, defaultCatalog, defaultSchema ) ) + .append( constraintString ); + return buf.toString(); + } } + return null; } public List getColumns() { diff --git a/hibernate-core/src/test/java/org/hibernate/test/hql/ASTParserLoadingTest.java b/hibernate-core/src/test/java/org/hibernate/test/hql/ASTParserLoadingTest.java index dcb4214371..26cb5bf005 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/hql/ASTParserLoadingTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/hql/ASTParserLoadingTest.java @@ -756,13 +756,14 @@ public class ASTParserLoadingTest extends BaseCoreFunctionalTestCase { else { s.createQuery( "from Animal where lower(upper('foo') || upper(:bar)) like 'f%'" ).setString( "bar", "xyz" ).list(); } - if ( !( getDialect() instanceof PostgreSQLDialect || getDialect() instanceof PostgreSQL81Dialect - || getDialect() instanceof MySQLDialect || getDialect() instanceof AbstractHANADialect ) ) { - s.createQuery( "from Animal where abs(cast(1 as float) - cast(:param as float)) = 1.0" ) + + if ( getDialect() instanceof AbstractHANADialect ) { + s.createQuery( "from Animal where abs(cast(1 as double) - cast(:param as double)) = 1.0" ) .setLong( "param", 1 ).list(); } - else if ( getDialect() instanceof AbstractHANADialect ) { - s.createQuery( "from Animal where abs(cast(1 as double) - cast(:param as double)) = 1.0" ) + else if ( !( getDialect() instanceof PostgreSQLDialect || getDialect() instanceof PostgreSQL81Dialect + || getDialect() instanceof MySQLDialect ) ) { + s.createQuery( "from Animal where abs(cast(1 as float) - cast(:param as float)) = 1.0" ) .setLong( "param", 1 ).list(); }