NH-3480 - Code reorganization and cleanup

Author: JKeyser

src/NHibernate/Loader/Collection/OneToManyJoinWalker.cs

@@ -58,7 +58,7 @@ private void InitStatementString(IOuterJoinLoadable elementPersister, string ali
 			int collectionJoins = CountCollectionPersisters(associations) + 1;
 			CollectionSuffixes = BasicLoader.GenerateSuffixes(joins + 1, collectionJoins);
 
-			SqlStringBuilder whereString = WhereString(oneToManyPersister.CollectionType.UseLHSPrimaryKey ? alias : alias + "owner_", oneToManyPersister.KeyColumnNames, subquery, batchSize);
+			SqlStringBuilder whereString = WhereString(oneToManyPersister.GenerateTableAliasForKeyColumns(alias), oneToManyPersister.KeyColumnNames, subquery, batchSize);
 			string filter = oneToManyPersister.FilterFragment(alias, EnabledFilters);
 			whereString.Insert(0, StringHelper.MoveAndToBeginning(filter));
 

src/NHibernate/Persister/Collection/AbstractCollectionPersister.cs

@@ -254,7 +254,7 @@ public AbstractCollectionPersister(Mapping.Collection collection, ICacheConcurre
 				foreach (Column col in collection.Owner.Key.ColumnIterator)
 				{
 					keyColumnNames[k] = col.GetQuotedName(dialect);
-					keyColumnAliases[k] = col.GetAlias(dialect) + "_owner_";
+					keyColumnAliases[k] = col.GetAlias(dialect) + "_owner_"; // Force the alias to be unique in case it conflicts with an alias in the entity
 					k++;
 				}
 				joinColumnNames = new string[collection.Key.ColumnSpan];
@@ -1987,6 +1987,11 @@ protected Dialect.Dialect Dialect
 		public abstract bool CascadeDeleteEnabled { get; }
 		public abstract bool IsOneToMany { get; }
 
+		public virtual string GenerateTableAliasForKeyColumns(string alias)
+		{
+			return alias;
+		}
+
 		protected object PerformInsert(object ownerId, IPersistentCollection collection, IExpectation expectation,
 									   object entry, int index, bool useBatch, bool callable, ISessionImplementor session)
 		{

src/NHibernate/Persister/Collection/IQueryableCollection.cs

@@ -80,5 +80,12 @@ public interface IQueryableCollection : IPropertyMapping, IJoinable, ICollection
 		/// <param name="alias">The alias for the many-to-many target table</param>
 		/// <returns>Appropriate order-by fragment or empty string.</returns>
 		string GetManyToManyOrderByString(string alias);
+
+		/// <summary>
+		/// Generate the table alias to use for the collection's key columns
+		/// </summary>
+		/// <param name="alias">The alias for thetarget table</param>
+		/// <returns>Appropriate table alias.</returns>
+		string GenerateTableAliasForKeyColumns(string alias);
 	}
 }

src/NHibernate/Persister/Collection/OneToManyPersister.cs

@@ -64,69 +64,31 @@ public override bool IsManyToMany
 		/// <returns></returns>
 		protected override SqlCommandInfo GenerateDeleteString()
 		{
+			var update = new SqlUpdateBuilder(Factory.Dialect, Factory)
+				.SetTableName(qualifiedTableName)
+				.AddColumns(JoinColumnNames, "null");
+
 			if (CollectionType.UseLHSPrimaryKey)
 			{
-				var update = new SqlUpdateBuilder(Factory.Dialect, Factory)
-					.SetTableName(qualifiedTableName)
-					.AddColumns(KeyColumnNames, "null")
-					.SetIdentityColumn(KeyColumnNames, KeyType);
-
-				if (HasIndex)
-					update.AddColumns(IndexColumnNames, "null");
-
-				if (HasWhere)
-					update.AddWhereFragment(sqlWhereString);
-
-				if (Factory.Settings.IsCommentsEnabled)
-					update.SetComment("delete one-to-many " + Role);
-
-				return update.ToSqlCommandInfo();
+				update.SetIdentityColumn(KeyColumnNames, KeyType);
 			}
 			else
 			{
-				var deleteSql = new SqlStringBuilder();
+				var ownerPersister = (IOuterJoinLoadable)OwnerEntityPersister;
+				update.SetJoin(ownerPersister.TableName, JoinColumnNames, ownerPersister.GetPropertyColumnNames(CollectionType.LHSPropertyName));
+				update.SetIdentityColumn(ownerPersister.TableName, KeyColumnNames, KeyType);
+			}
 
-				deleteSql.Add("update " + qualifiedTableName + " set ");
-				bool andNeeded = false;
-				foreach (string nextColumn in JoinColumnNames)
-				{
-					if (andNeeded)
-					{
-						deleteSql.Add(" and ");
-					}
-					deleteSql.Add(nextColumn + " = null");
-					andNeeded = true;
-				}
+			if (HasIndex)
+				update.AddColumns(IndexColumnNames, "null");
 
-				var ownerPersister = (IOuterJoinLoadable)OwnerEntityPersister;
-				deleteSql.Add(" from " + qualifiedTableName + " inner join " + ownerPersister.TableName + " on ");
-				var ownerJoinColumns = ownerPersister.GetPropertyColumnNames(CollectionType.LHSPropertyName);
-				andNeeded = false;
-				for (int columnIndex = 0; columnIndex < JoinColumnNames.Length; columnIndex++)
-				{
-					if (andNeeded)
-					{
-						deleteSql.Add(" and ");
-					}
-					deleteSql.Add(qualifiedTableName + StringHelper.Dot + JoinColumnNames[columnIndex] + " = " + ownerPersister.TableName + StringHelper.Dot + ownerJoinColumns[columnIndex]);
-					andNeeded = true;
-				}
+			if (HasWhere)
+				update.AddWhereFragment(sqlWhereString);
 
-				deleteSql.Add(" where ");
-				andNeeded = false;
-				foreach (string nextColumn in KeyColumnNames)
-				{
-					if (andNeeded)
-					{
-						deleteSql.Add(" and ");
-					}
-					deleteSql.Add(ownerPersister.TableName + StringHelper.Dot + nextColumn + " = ");
-					deleteSql.AddParameter();
-					andNeeded = true;
-				}
+			if (Factory.Settings.IsCommentsEnabled)
+				update.SetComment("delete one-to-many " + Role);
 
-				return new SqlCommandInfo(deleteSql.ToSqlString(), KeyType.SqlTypes(Factory));
-			}
+			return update.ToSqlCommandInfo();
 		}
 
 		/// <summary>
@@ -198,6 +160,11 @@ public override bool ConsumesCollectionAlias()
 			return true;
 		}
 
+		public override string GenerateTableAliasForKeyColumns(string alias)
+		{
+			return CollectionType.UseLHSPrimaryKey ? alias : alias + "owner_";
+		}
+
 		protected override int DoUpdateRows(object id, IPersistentCollection collection, ISessionImplementor session)
 		{
 			// we finish all the "removes" first to take care of possible unique 
@@ -368,7 +335,7 @@ protected override SelectFragment GenerateSelectFragment(string alias, string co
 			for (int i = 0; i < columnNames.Length; i++)
 			{
 				var column = columnNames[i];
-				var tableAlias = ojl.GenerateTableAliasForColumn(alias + (CollectionType.UseLHSPrimaryKey ? String.Empty : "owner_"), column);
+				var tableAlias = CollectionType.UseLHSPrimaryKey ? ojl.GenerateTableAliasForColumn(alias, column) : GenerateTableAliasForKeyColumns(alias);
 				selectFragment.AddColumn(tableAlias, column, columnAliases[i]);
 			}
 			return selectFragment;
@@ -402,7 +369,7 @@ public override SqlString FromJoinFragment(string alias, bool innerJoin, bool in
 			}
 
 			var ownerPersister = (IOuterJoinLoadable)OwnerEntityPersister;
-			join.AddJoin(ownerPersister.TableName, alias + "owner_", lhsKeyColumnNames, ownerPersister.GetPropertyColumnNames(CollectionType.LHSPropertyName), JoinType.LeftOuterJoin);
+			join.AddJoin(ownerPersister.TableName, GenerateTableAliasForKeyColumns(alias), lhsKeyColumnNames, ownerPersister.GetPropertyColumnNames(CollectionType.LHSPropertyName), innerJoin ? JoinType.InnerJoin : JoinType.LeftOuterJoin);
 			return join.ToFromFragmentString + elementJoinFragment;
 		}
 

src/NHibernate/SqlCommand/SqlUpdateBuilder.cs

@@ -20,6 +20,8 @@ public class SqlUpdateBuilder : SqlBaseBuilder, ISqlStringBuilder
 		// columns-> (ColumnName, Value) or (ColumnName, SqlType) for parametrized column
 		private readonly LinkedHashMap<string, object> columns = new LinkedHashMap<string, object>();
 
+		private SqlString join = null;
+
 		private List<SqlString> whereStrings = new List<SqlString>();
 		private readonly List<SqlType> whereParameterTypes = new List<SqlType>();
 		private SqlString assignments;
@@ -128,6 +130,28 @@ public SqlUpdateBuilder AppendAssignmentFragment(SqlString fragment)
 			return this;
 		}
 
+		public SqlUpdateBuilder SetJoin(string joinTableName, string[] lhsColumnNames, string[] rhsColumnNames)
+		{
+			SqlStringBuilder joinStringBuilder = new SqlStringBuilder();
+
+			joinStringBuilder.Add(" INNER JOIN ");
+			joinStringBuilder.Add(joinTableName);
+			joinStringBuilder.Add(" ON ");
+			bool andNeeded = false;
+			for (int columnIndex = 0; columnIndex < lhsColumnNames.Length; columnIndex++)
+			{
+				if (andNeeded)
+				{
+					joinStringBuilder.Add(" AND ");
+				}
+				joinStringBuilder.Add(tableName + StringHelper.Dot + lhsColumnNames[columnIndex] + " = " + joinTableName + StringHelper.Dot + rhsColumnNames[columnIndex]);
+				andNeeded = true;
+			}
+
+			join = joinStringBuilder.ToSqlString();
+			return this;
+		}
+
 		public SqlUpdateBuilder SetWhere(string whereSql)
 		{
 			if (StringHelper.IsNotEmpty(whereSql))
@@ -150,6 +174,20 @@ public SqlUpdateBuilder SetIdentityColumn(string[] columnNames, IType identityTy
 			return this;
 		}
 
+ 		/// <summary>
+		/// Sets the IdentityColumn for the <c>UPDATE</c> sql to use.
+		/// </summary>
+		/// <param name="tableName">Table name to prepend to the columns.</param>
+		/// <param name="columnNames">An array of the column names for the Property</param>
+		/// <param name="identityType">The IType of the Identity Property.</param>
+		/// <returns>The SqlUpdateBuilder.</returns>
+		public SqlUpdateBuilder SetIdentityColumn(string tableName, string[] columnNames, IType identityType)
+		{
+			whereStrings.Add(ToWhereString(tableName, columnNames));
+			whereParameterTypes.AddRange(identityType.SqlTypes(Mapping));
+			return this;
+		}
+
 		/// <summary>
 		/// Sets the VersionColumn for the <c>UPDATE</c> sql to use.
 		/// </summary>
@@ -232,6 +270,11 @@ public SqlString ToSqlString()
 			{
 				initialCapacity += (columns.Count - 1) + (columns.Count * 3);
 			}
+
+			// 3 - JOIN
+			if (join != null)
+				initialCapacity += 3;
+
 			// 1 = "WHERE" 
 			initialCapacity++;
 
@@ -282,6 +325,13 @@ public SqlString ToSqlString()
 				sqlBuilder.Add(assignments);
 			}
 
+			if (join != null)
+			{
+				sqlBuilder.Add(" FROM ");
+				sqlBuilder.Add(tableName);
+				sqlBuilder.Add(join);
+			}
+
 			sqlBuilder.Add(" WHERE ");
 			bool andNeeded = false;
 			foreach (SqlString whereString in whereStrings)