Author: stliu Date: 2010-07-26 10:05:03 -0400 (Mon, 26 Jul 2010) New Revision: 20066 Added: core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/idgen/enhanced/table/Person.hbm.xml core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/idgen/enhanced/table/Person.java Modified: core/branches/Branch_3_2_4_SP1_CP/src/org/hibernate/id/enhanced/TableGenerator.java core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/idgen/enhanced/table/BasicTableTest.java Log: JBPAPP-4599 HHH-3231 org.hibernate.id.enhanced.TableGenerator throws IllegalArgumentException: alias not found: tbl under Oracle Modified: core/branches/Branch_3_2_4_SP1_CP/src/org/hibernate/id/enhanced/TableGenerator.java =================================================================== --- core/branches/Branch_3_2_4_SP1_CP/src/org/hibernate/id/enhanced/TableGenerator.java 2010-07-26 11:23:01 UTC (rev 20065) +++ core/branches/Branch_3_2_4_SP1_CP/src/org/hibernate/id/enhanced/TableGenerator.java 2010-07-26 14:05:03 UTC (rev 20066) @@ -5,6 +5,8 @@ import java.sql.SQLException; import java.sql.PreparedStatement; import java.sql.ResultSet; +import java.util.Collections; +import java.util.Map; import java.util.Properties; import java.util.HashMap; import java.io.Serializable; @@ -27,10 +29,27 @@ import org.hibernate.util.CollectionHelper; /** - * A "segmented" version of the enhanced table generator. The term "segmented" - * refers to the fact that this table can hold multiple value generators, - * segmented by a key. + * An enhanced version of table-based id generation. * <p/> + * Unlike the simplistic legacy one (which, btw, was only ever intended for subclassing + * support) we "segment" the table into multiple values. Thus a single table can + * actually serve as the persistent storage for multiple independent generators. One + * approach would be to segment the values by the name of the entity for which we are + * performing generation, which would mean that we would have a row in the generator + * table for each entity name. Or any configuration really; the setup is very flexible. + * <p/> + * In this respect it is very simliar to the legacy + * {@link org.hibernate.id.MultipleHiLoPerTableGenerator} in terms of the + * underlying storage structure (namely a single table capable of holding + * multiple generator values). The differentiator is, as with + * {@link SequenceStyleGenerator} as well, the externalized notion + * of an optimizer. + * <p/> + * <b>NOTE</b> that by default we use a single row for all genertators (based + * on {@link #DEF_SEGMENT_VALUE}). The configuration parameter + * {@link #CONFIG_PREFER_SEGMENT_PER_ENTITY} can be used to change that to + * instead default to using a row for each entity name. + * <p/> * Configuration parameters: * <table> * <tr> @@ -84,7 +103,7 @@ */ public class TableGenerator extends TransactionHelper implements PersistentIdentifierGenerator, Configurable { private static final Log log = LogFactory.getLog( TableGenerator.class ); - + public static final String CONFIG_PREFER_SEGMENT_PER_ENTITY = "hibernate.id.enhanced.table.prefer_segment_per_entity"; public static final String TABLE_PARAM = "table_name"; public static final String DEF_TABLE = "hibernate_sequences"; @@ -119,91 +138,260 @@ private Type identifierType; - private String query; - private String insert; - private String update; + private String selectQuery; + private String insertQuery; + private String updateQuery; private Optimizer optimizer; private long accessCount = 0; - public String getTableName() { + /** + * {@inheritDoc} + */ + public Object generatorKey() { return tableName; } - public String getSegmentColumnName() { + /** + * Type mapping for the identifier. + * + * @return The identifier type mapping. + */ + public final Type getIdentifierType() { + return identifierType; + } + + /** + * The name of the table in which we store this generator's persistent state. + * + * @return The table name. + */ + public final String getTableName() { + return tableName; + } + + /** + * The name of the column in which we store the segment to which each row + * belongs. The value here acts as PK. + * + * @return The segment column name + */ + public final String getSegmentColumnName() { return segmentColumnName; } - public String getSegmentValue() { + /** + * The value in {@link #getSegmentColumnName segment column} which + * corresponding to this generator instance. In other words this value + * indicates the row in which this generator instance will store values. + * + * @return The segment value for this generator instance. + */ + public final String getSegmentValue() { return segmentValue; } - - public int getSegmentValueLength() { + /** + * The size of the {@link #getSegmentColumnName segment column} in the + * underlying table. + * <p/> + * <b>NOTE</b> : should really have been called 'segmentColumnLength' or + * even better 'segmentColumnSize' + * + * @return the column size. + */ + public final int getSegmentValueLength() { return segmentValueLength; } - public String getValueColumnName() { + /** + * The name of the column in which we store our persistent generator value. + * + * @return The name of the value column. + */ + public final String getValueColumnName() { return valueColumnName; } - public Type getIdentifierType() { - return identifierType; - } - - public int getInitialValue() { + /** + * The initial value to use when we find no previous state in the + * generator table corresponding to our sequence. + * + * @return The initial value to use. + */ + public final int getInitialValue() { return initialValue; } - public int getIncrementSize() { + /** + * The amount of increment to use. The exact implications of this + * depends on the {@link #getOptimizer() optimizer} being used. + * + * @return The increment amount. + */ + public final int getIncrementSize() { return incrementSize; } - public Optimizer getOptimizer() { + /** + * The optimizer being used by this generator. + * + * @return Out optimizer. + */ + public final Optimizer getOptimizer() { return optimizer; } - public long getTableAccessCount() { + /** + * Getter for property 'tableAccessCount'. Only really useful for unit test + * assertions. + * + * @return Value for property 'tableAccessCount'. + */ + public final long getTableAccessCount() { return accessCount; } + /** + * {@inheritDoc} + */ + public void configure(Type type, Properties params, Dialect dialect) throws MappingException { + identifierType = type; - public void configure(Type type, Properties params, Dialect dialect) throws MappingException { - tableName = PropertiesHelper.getString( TABLE_PARAM, params, DEF_TABLE ); - if ( tableName.indexOf( '.' ) < 0 ) { + tableName = determneGeneratorTableName( params ); + segmentColumnName = determineSegmentColumnName( params ); + valueColumnName = determineValueColumnName( params ); + + segmentValue = determineSegmentValue( params ); + + segmentValueLength = determineSegmentColumnSize( params ); + initialValue = determineInitialValue( params ); + incrementSize = determineIncrementSize( params ); + + this.selectQuery = buildSelectQuery( dialect ); + this.updateQuery = buildUpdateQuery(); + this.insertQuery = buildInsertQuery(); + + String defOptStrategy = incrementSize <= 1 ? OptimizerFactory.NONE : OptimizerFactory.POOL; + String optimizationStrategy = PropertiesHelper.getString( OPT_PARAM, params, defOptStrategy ); + optimizer = OptimizerFactory.buildOptimizer( optimizationStrategy, identifierType.getReturnedClass(), incrementSize ); + } + + /** + * Determine the table name to use for the generator values. + * <p/> + * Called during {@link #configure configuration}. + * + * @see #getTableName() + * @param params The params supplied in the generator config (plus some standard useful extras). + * @return The table name to use. + */ + protected String determneGeneratorTableName(Properties params) { + String name = PropertiesHelper.getString( TABLE_PARAM, params, DEF_TABLE ); + boolean isGivenNameUnqualified = name.indexOf( '.' ) < 0; + if ( isGivenNameUnqualified ) { + // if the given name is un-qualified we may neen to qualify it String schemaName = params.getProperty( SCHEMA ); String catalogName = params.getProperty( CATALOG ); - tableName = Table.qualify( catalogName, schemaName, tableName ); + name = Table.qualify( catalogName, schemaName, name ); } + return name; + } + /** + * Determine the name of the column used to indicate the segment for each + * row. This column acts as the primary key. + * <p/> + * Called during {@link #configure configuration}. + * + * @see #getSegmentColumnName() + * @param params The params supplied in the generator config (plus some standard useful extras). + * @return The name of the segment column + */ + protected String determineSegmentColumnName(Properties params) { + return PropertiesHelper.getString( SEGMENT_COLUMN_PARAM, params, DEF_SEGMENT_COLUMN ); + } - segmentColumnName = PropertiesHelper.getString( SEGMENT_COLUMN_PARAM, params, DEF_SEGMENT_COLUMN ); - segmentValue = params.getProperty( SEGMENT_VALUE_PARAM ); + /** + * Determine the name of the column in which we will store the generator persistent value. + * <p/> + * Called during {@link #configure configuration}. + * + * @see #getValueColumnName() + * @param params The params supplied in the generator config (plus some standard useful extras). + * @return The name of the value column + */ + protected String determineValueColumnName(Properties params) { + return PropertiesHelper.getString( VALUE_COLUMN_PARAM, params, DEF_VALUE_COLUMN ); + } + + /** + * Determine the segment value corresponding to this generator instance. + * <p/> + * Called during {@link #configure configuration}. + * + * @see #getSegmentValue() + * @param params The params supplied in the generator config (plus some standard useful extras). + * @return The name of the value column + */ + protected String determineSegmentValue(Properties params) { + String segmentValue = params.getProperty( SEGMENT_VALUE_PARAM ); if ( StringHelper.isEmpty( segmentValue ) ) { - log.debug( "explicit segment value for id generator [" + tableName + '.' + segmentColumnName + "] suggested; using default [" + DEF_SEGMENT_VALUE + "]" ); - segmentValue = DEF_SEGMENT_VALUE; + segmentValue = determineDefaultSegmentValue( params ); } - segmentValueLength = PropertiesHelper.getInt( SEGMENT_LENGTH_PARAM, params, DEF_SEGMENT_LENGTH ); - valueColumnName = PropertiesHelper.getString( VALUE_COLUMN_PARAM, params, DEF_VALUE_COLUMN ); - initialValue = PropertiesHelper.getInt( INITIAL_PARAM, params, DEFAULT_INITIAL_VALUE ); - incrementSize = PropertiesHelper.getInt( INCREMENT_PARAM, params, DEFAULT_INCREMENT_SIZE ); - identifierType = type; + return segmentValue; + } - String query = "select " + valueColumnName + - " from " + tableName + " tbl" + - " where tbl." + segmentColumnName + "=?"; + /** + * Used in the cases where {@link #determineSegmentValue} is unable to + * determine the value to use. + * + * @param params The params supplied in the generator config (plus some standard useful extras). + * @return The default segment value to use. + */ + protected String determineDefaultSegmentValue(Properties params) { + boolean preferSegmentPerEntity = PropertiesHelper.getBoolean( CONFIG_PREFER_SEGMENT_PER_ENTITY, params, false ); + String defaultToUse = preferSegmentPerEntity ? params.getProperty( TABLE ) : DEF_SEGMENT_VALUE; + log.info( "explicit segment value for id generator [" + tableName + '.' + segmentColumnName + "] suggested; using default [" + defaultToUse + "]" ); + return defaultToUse; + } + + /** + * Determine the size of the {@link #getSegmentColumnName segment column} + * <p/> + * Called during {@link #configure configuration}. + * + * @see #getSegmentValueLength() + * @param params The params supplied in the generator config (plus some standard useful extras). + * @return The size of the segment column + */ + protected int determineSegmentColumnSize(Properties params) { + return PropertiesHelper.getInt( SEGMENT_LENGTH_PARAM, params, DEF_SEGMENT_LENGTH ); + } + + protected int determineInitialValue(Properties params) { + return PropertiesHelper.getInt( INITIAL_PARAM, params, DEFAULT_INITIAL_VALUE ); + } + + protected int determineIncrementSize(Properties params) { + return PropertiesHelper.getInt( INCREMENT_PARAM, params, DEFAULT_INCREMENT_SIZE ); + } + protected String buildSelectQuery(Dialect dialect) { + final String alias = "tbl"; + String query = "select " + StringHelper.qualify( alias, valueColumnName ) + + " from " + tableName + ' ' + alias + + " where " + StringHelper.qualify( alias, segmentColumnName ) + "=?"; HashMap lockMap = new HashMap(); - lockMap.put( "tbl", LockMode.UPGRADE ); - this.query = dialect.applyLocksToSql( query, lockMap, CollectionHelper.EMPTY_MAP ); + lockMap.put( alias, LockMode.UPGRADE ); + Map updateTargetColumnsMap = Collections.singletonMap( alias, new String[] { valueColumnName } ); + return dialect.applyLocksToSql( query, lockMap, updateTargetColumnsMap ); + } - update = "update " + tableName + + protected String buildUpdateQuery() { + return "update " + tableName + " set " + valueColumnName + "=? " + " where " + valueColumnName + "=? and " + segmentColumnName + "=?"; - - insert = "insert into " + tableName + " (" + segmentColumnName + ", " + valueColumnName + ") " + " values (?,?)"; - - String defOptStrategy = incrementSize <= 1 ? OptimizerFactory.NONE : OptimizerFactory.POOL; - String optimizationStrategy = PropertiesHelper.getString( OPT_PARAM, params, defOptStrategy ); - optimizer = OptimizerFactory.buildOptimizer( optimizationStrategy, identifierType.getReturnedClass(), incrementSize ); } - + protected String buildInsertQuery() { + return "insert into " + tableName + " (" + segmentColumnName + ", " + valueColumnName + ") " + " values (?,?)"; + } public synchronized Serializable generate(final SessionImplementor session, Object obj) { return optimizer.generate( new AccessCallback() { @@ -214,23 +402,24 @@ ); } + /** + * {@inheritDoc} + */ public Serializable doWorkInCurrentTransaction(Connection conn, String sql) throws SQLException { int result; int rows; do { - sql = query; - SQL.debug( sql ); - PreparedStatement queryPS = conn.prepareStatement( query ); + SQL.debug( selectQuery ); + PreparedStatement selectPS = conn.prepareStatement( selectQuery ); try { - queryPS.setString( 1, segmentValue ); - ResultSet queryRS = queryPS.executeQuery(); - if ( !queryRS.next() ) { + selectPS.setString( 1, segmentValue ); + ResultSet selectRS = selectPS.executeQuery(); + if ( !selectRS.next() ) { PreparedStatement insertPS = null; try { result = initialValue; - sql = insert; - SQL.debug( sql ); - insertPS = conn.prepareStatement( insert ); + SQL.debug( insertQuery ); + insertPS = conn.prepareStatement( insertQuery ); insertPS.setString( 1, segmentValue ); insertPS.setLong( 2, result ); insertPS.execute(); @@ -242,21 +431,20 @@ } } else { - result = queryRS.getInt( 1 ); + result = selectRS.getInt( 1 ); } - queryRS.close(); + selectRS.close(); } catch ( SQLException sqle ) { log.error( "could not read or init a hi value", sqle ); throw sqle; } finally { - queryPS.close(); + selectPS.close(); } - sql = update; - SQL.debug( sql ); - PreparedStatement updatePS = conn.prepareStatement( update ); + SQL.debug( updateQuery ); + PreparedStatement updatePS = conn.prepareStatement( updateQuery ); try { long newValue = optimizer.applyIncrementSizeToSourceValues() ? result + incrementSize : result + 1; @@ -266,7 +454,7 @@ rows = updatePS.executeUpdate(); } catch ( SQLException sqle ) { - log.error( "could not update hi value in: " + tableName, sqle ); + log.error( "could not updateQuery hi value in: " + tableName, sqle ); throw sqle; } finally { @@ -294,7 +482,9 @@ .append( valueColumnName ) .append( ' ' ) .append( dialect.getTypeName( Types.BIGINT ) ) - .append( " ) " ) + .append( ", primary key ( " ) + .append( segmentColumnName ) + .append( " ) ) " ) .toString() }; } @@ -311,7 +501,4 @@ return new String[] { sqlDropString.toString() }; } - public Object generatorKey() { - return tableName; - } } Modified: core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/idgen/enhanced/table/BasicTableTest.java =================================================================== --- core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/idgen/enhanced/table/BasicTableTest.java 2010-07-26 11:23:01 UTC (rev 20065) +++ core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/idgen/enhanced/table/BasicTableTest.java 2010-07-26 14:05:03 UTC (rev 20066) @@ -19,13 +19,22 @@ } public String[] getMappings() { - return new String[] { "idgen/enhanced/table/Basic.hbm.xml" }; + return new String[] { "idgen/enhanced/table/Basic.hbm.xml","idgen/enhanced/table/Person.hbm.xml" }; } public static Test suite() { return new FunctionalTestClassTestSuite( BasicTableTest.class ); } - + public void testHHH3231(){ + Session s = openSession(); + s.beginTransaction(); + Person p = new Person(); + p.setName( "stliu" ); + s.save( p ); + s.getTransaction().commit(); + s.close(); + + } public void testNormalBoundary() { EntityPersister persister = sfi().getEntityPersister( Entity.class.getName() ); assertClassAssignability( TableGenerator.class, persister.getIdentifierGenerator().getClass() ); Added: core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/idgen/enhanced/table/Person.hbm.xml =================================================================== --- core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/idgen/enhanced/table/Person.hbm.xml (rev 0) +++ core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/idgen/enhanced/table/Person.hbm.xml 2010-07-26 14:05:03 UTC (rev 20066) @@ -0,0 +1,19 @@ +<?xml version="1.0"?> +<!DOCTYPE hibernate-mapping PUBLIC "-//Hibernate/Hibernate Mapping DTD 3.0//EN" + "http://hibernate.sourceforge.net/hibernate-mapping-3.0.dtd"> +<hibernate-mapping package="org.hibernate.test.idgen.enhanced.table"> + <class name="Person" table="PERSON"> + <id name="id" type="long" column="PID"> + + <generator + class="org.hibernate.id.enhanced.TableGenerator" > +<!-- + <generator + class="org.hibernate.id.enhanced.TableGeneratorModified" > +--> + <param name="segment_value">sq_person_pid</param> + </generator> + </id> + <property name="name" type="string" length="50" column="NAME" /> + </class> +</hibernate-mapping> Added: core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/idgen/enhanced/table/Person.java =================================================================== --- core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/idgen/enhanced/table/Person.java (rev 0) +++ core/branches/Branch_3_2_4_SP1_CP/test/org/hibernate/test/idgen/enhanced/table/Person.java 2010-07-26 14:05:03 UTC (rev 20066) @@ -0,0 +1,51 @@ +package org.hibernate.test.idgen.enhanced.table; + +import org.hibernate.Session; +import org.hibernate.SessionFactory; +import org.hibernate.cfg.Configuration; + +public class Person { + + private Long id; + private String name; + + public Person() { + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public static void main(String[] args) { + + SessionFactory sessionFactory = new Configuration().configure() + .buildSessionFactory(); + + Session session = sessionFactory.getCurrentSession(); + session.beginTransaction(); + + Person thePerson = new Person(); + thePerson.setName("Foo Bar"); + + session.save(thePerson); + + session.getTransaction().commit(); + + Long personId = thePerson.getId(); + System.out.println("Added person " + personId); + + sessionFactory.close(); + } +} \ No newline at end of file