From 174d78838eeeb604fac7e31eaff1eedc9b789d16 Mon Sep 17 00:00:00 2001 From: Malishev Georgy Date: Tue, 5 Aug 2014 21:03:31 +0400 Subject: [PATCH 1/8] Implementation (fix) NH-3427 Joining to association twice using criteria. --- .../NHSpecificTest/NH3472/Domain.cs | 27 ++++ .../NHSpecificTest/NH3472/Fixture.cs | 58 +++++++ .../NHSpecificTest/NH3472/Mappings.hbm.xml | 23 +++ .../Loader/Criteria/CriteriaJoinWalker.cs | 31 ++-- .../Criteria/CriteriaQueryTranslator.cs | 151 ++++++++++++++---- src/NHibernate/Loader/JoinWalker.cs | 100 ++++++++---- 6 files changed, 309 insertions(+), 81 deletions(-) create mode 100644 src/NHibernate.Test/NHSpecificTest/NH3472/Domain.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/NH3472/Fixture.cs create mode 100644 src/NHibernate.Test/NHSpecificTest/NH3472/Mappings.hbm.xml diff --git a/src/NHibernate.Test/NHSpecificTest/NH3472/Domain.cs b/src/NHibernate.Test/NHSpecificTest/NH3472/Domain.cs new file mode 100644 index 00000000000..6eb5fa7c35e --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH3472/Domain.cs @@ -0,0 +1,27 @@ +using System.Collections; +using System.Collections.Generic; + +namespace NHibernate.Test.NHSpecificTest.NH3472 +{ + + public class Cat + { + private int id; + private IList children = new List(); + + public virtual string Color { get; set; } + public int Age { get; set; } + + public int Id + { + get { return id; } + set { id = value; } + } + + public IList Children + { + get { return children; } + set { children = value; } + } + } +} \ No newline at end of file diff --git a/src/NHibernate.Test/NHSpecificTest/NH3472/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/NH3472/Fixture.cs new file mode 100644 index 00000000000..f872078f51b --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH3472/Fixture.cs @@ -0,0 +1,58 @@ +using System.Collections.Generic; +using NHibernate.Criterion; +using NHibernate.SqlCommand; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH3472 +{ + [TestFixture] + public class Fixture : BugTestCase + { + [Test] + public void CriteriaQueryWithMultipleJoinsToSameAssociation() + { + using (ISession sess = OpenSession()) + { + Cat c = new Cat(); + c.Age = 0; + c.Children = new List + { + new Cat {Color = "Ginger", Age = 1}, + new Cat {Color = "Black", Age = 3} + }; + sess.Save(c); + sess.Flush(); + } + + using (ISession sess = OpenSession()) + { + try + { + var list = sess.CreateCriteria("cat") + .CreateAlias("cat.Children", "gingerCat", JoinType.LeftOuterJoin, Restrictions.Eq("Color", "Ginger")) + + .CreateAlias("cat.Children", "blackCat", JoinType.LeftOuterJoin, + Restrictions.Eq("Color", "Black")) + .SetProjection( + Projections.Alias(Projections.Property("gingerCat.Age"), "gingerCatAge"), + Projections.Alias(Projections.Property("blackCat.Age"), "blackCatAge") + ).AddOrder(new Order(Projections.Property("Age"), true)).List(); + Assert.AreEqual(list.Count, 3); + Assert.AreEqual(list[0][0], 1); + Assert.AreEqual(list[0][1], 3); + + Assert.IsNull(list[1][0]); + Assert.IsNull(list[1][1]); + + Assert.IsNull(list[2][0]); + Assert.IsNull(list[2][1]); + } + finally + { + sess.Delete("from Cat"); + sess.Flush(); + } + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/NH3472/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/NH3472/Mappings.hbm.xml new file mode 100644 index 00000000000..a2564201cbc --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/NH3472/Mappings.hbm.xml @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs b/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs index 75ba986cc20..73bdc86df50 100644 --- a/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs +++ b/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs @@ -80,7 +80,7 @@ protected override void AddAssociations() foreach (var entityJoinInfo in translator.GetEntityJoins().Values) { var tableAlias = translator.GetSQLAlias(entityJoinInfo.Criteria); - var criteriaPath = entityJoinInfo.Criteria.Alias; //path for entity join is equal to alias + var criteriaPath = entityJoinInfo.Criteria.Alias; var persister = entityJoinInfo.Persister as IOuterJoinLoadable; AddExplicitEntityJoinAssociation(persister, tableAlias, translator.GetJoinType(criteriaPath), criteriaPath); IncludeInResultIfNeeded(persister, entityJoinInfo.Criteria, tableAlias, criteriaPath); @@ -166,13 +166,20 @@ public override string Comment get { return "criteria query"; } } - protected override JoinType GetJoinType(IAssociationType type, FetchMode config, string path, string lhsTable, - string[] lhsColumns, bool nullable, int currentDepth, - CascadeStyle cascadeStyle) + protected override IList AliasByPath(string path, string sqlAlias) { - if (translator.IsJoin(path)) + var alias = translator.AliasByPath(path, sqlAlias); + if (alias.Count == 0) + return base.AliasByPath(path, sqlAlias); + return alias; + } + + protected override JoinType GetJoinType(IAssociationType type, FetchMode config, string path, string pathAlias, + string lhsTable, string[] lhsColumns, bool nullable, int currentDepth, CascadeStyle cascadeStyle) + { + if (translator.IsJoin(path, pathAlias)) { - return translator.GetJoinType(path); + return translator.GetJoinType(path, pathAlias); } if (translator.HasProjection) @@ -184,7 +191,7 @@ protected override JoinType GetJoinType(IAssociationType type, FetchMode config, switch (selectMode) { case SelectMode.Undefined: - return base.GetJoinType(type, config, path, lhsTable, lhsColumns, nullable, currentDepth, cascadeStyle); + return base.GetJoinType(type, config, path, pathAlias, lhsTable, lhsColumns, nullable, currentDepth, cascadeStyle); case SelectMode.Fetch: case SelectMode.FetchLazyProperties: @@ -200,7 +207,7 @@ protected override JoinType GetJoinType(IAssociationType type, FetchMode config, } } - protected override string GenerateTableAlias(int n, string path, IJoinable joinable) + protected override string GenerateTableAlias(int n, string path, string pathAlias, IJoinable joinable) { // TODO: deal with side-effects (changes to includeInSelectList, userAliasList, resultTypeList)!!! @@ -225,14 +232,14 @@ protected override string GenerateTableAlias(int n, string path, IJoinable joina if (shouldCreateUserAlias) { - ICriteria subcriteria = translator.GetCriteria(path); + ICriteria subcriteria = translator.GetCriteria(path, pathAlias); sqlAlias = subcriteria == null ? null : translator.GetSQLAlias(subcriteria); IncludeInResultIfNeeded(joinable, subcriteria, sqlAlias, path); } if (sqlAlias == null) - sqlAlias = base.GenerateTableAlias(n + translator.SQLAliasCount, path, joinable); + sqlAlias = base.GenerateTableAlias(n + translator.SQLAliasCount, path, pathAlias, joinable); return sqlAlias; } @@ -261,9 +268,9 @@ protected override string GenerateRootAlias(string tableName) // NH: really not used (we are using a different ctor to support SubQueryCriteria) } - protected override SqlString GetWithClause(string path) + protected override SqlString GetWithClause(string path, string pathAlias) { - return translator.GetWithClause(path); + return translator.GetWithClause(path, pathAlias); } } } diff --git a/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs b/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs index 9a9acd430fd..100013fc397 100644 --- a/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs +++ b/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs @@ -44,10 +44,12 @@ public class EntityJoinInfo private readonly HashSet uncacheableCollectionPersisters = new HashSet(); private readonly ISet criteriaCollectionPersisters = new HashSet(); private readonly IDictionary criteriaSQLAliasMap = new Dictionary(); + private readonly IDictionary criteriaSQLAliasToAliasMap = new Dictionary(); + private readonly IDictionary> associationAliasToChild = new Dictionary>(); private readonly IDictionary aliasCriteriaMap = new Dictionary(); - private readonly Dictionary associationPathCriteriaMap = new Dictionary(); - private readonly IDictionary associationPathJoinTypesMap = new LinkedHashMap(); - private readonly IDictionary withClauseMap = new Dictionary(); + private readonly Dictionary associationPathCriteriaMap = new Dictionary(); + private readonly IDictionary associationPathJoinTypesMap = new LinkedHashMap(); + private readonly IDictionary withClauseMap = new Dictionary(); private readonly ISessionFactoryImplementor sessionFactory; private SessionFactoryHelper helper; @@ -58,6 +60,47 @@ public class EntityJoinInfo private Dictionary entityJoins = new Dictionary(); private readonly IQueryable rootPersister; + //Key for the dictionary sub-criteria + protected class AliasKey + { + + public AliasKey(string alias, string path) + { + Alias = alias; + Path = path; + } + + protected bool Equals(AliasKey other) + { + return string.Equals(Alias, other.Alias) && string.Equals(Path, other.Path); + } + + public override int GetHashCode() + { + unchecked + { + return ((Alias != null ? Alias.GetHashCode() : 0) * 397) ^ (Path != null ? Path.GetHashCode() : 0); + } + } + + public string Alias { get; set; } + + public string Path { get; set; } + + public override bool Equals(object obj) + { + if (ReferenceEquals(null, obj)) return false; + if (ReferenceEquals(this, obj)) return true; + if (obj.GetType() != this.GetType()) return false; + return Equals((AliasKey)obj); + } + + public override string ToString() + { + return string.Format("path: {0} key: {1}", Path, Alias); + } + } + public CriteriaQueryTranslator(ISessionFactoryImplementor factory, CriteriaImpl criteria, string rootEntityName, string rootSQLAlias, ICriteriaQuery outerQuery) : this(factory, criteria, rootEntityName, rootSQLAlias) @@ -280,23 +323,49 @@ private ICriteria GetAliasedCriteria(string alias) return result; } - public bool IsJoin(string path) + public bool IsJoin(string path, string critAlias) + { + AliasKey key = new AliasKey(critAlias, path); + return associationPathCriteriaMap.ContainsKey(key); + } + + /// + /// Returns a list of aliases criteria along the path and sql alias + /// + public IList AliasByPath(string path, string sqlAlias) { - return associationPathCriteriaMap.ContainsKey(path); + List alias = new List(); + + string critAlias; + if (!criteriaSQLAliasToAliasMap.TryGetValue(sqlAlias, out critAlias)) + critAlias = rootCriteria.Alias; + + ISet childs; + if (!associationAliasToChild.TryGetValue(critAlias, out childs)) + return alias; + + foreach (var child in childs) + { + AliasKey key = new AliasKey(child, path); + if (associationPathJoinTypesMap.ContainsKey(key)) alias.Add(child); + } + + return alias; } - public JoinType GetJoinType(string path) + public JoinType GetJoinType(string path, string critAlias) { JoinType result; - if (associationPathJoinTypesMap.TryGetValue(path, out result)) + if (associationPathJoinTypesMap.TryGetValue(new AliasKey(critAlias, path), out result)) return result; return JoinType.InnerJoin; } - public ICriteria GetCriteria(string path) + public ICriteria GetCriteria(string path, string critAlias) { - associationPathCriteriaMap.TryGetValue(path, out var result); - logger.Debug("getCriteria for path={0} crit={1}", path, result); + AliasKey key = new AliasKey(critAlias, path); + associationPathCriteriaMap.TryGetValue(key, out var result); + logger.Debug("getCriteria for key={0} crit={1}", key, result); return result; } @@ -324,40 +393,49 @@ private void CreateAssociationPathCriteriaMap() { foreach (CriteriaImpl.Subcriteria crit in rootCriteria.IterateSubcriteria()) { - string wholeAssociationPath = GetWholeAssociationPath(crit); + string parentAlias; + string wholeAssociationPath = GetWholeAssociationPath(crit, out parentAlias); + if (parentAlias != null) + { + ISet childAliases; + if (!associationAliasToChild.TryGetValue(parentAlias, out childAliases)) + associationAliasToChild.Add(parentAlias, new HashSet()); + associationAliasToChild[parentAlias].Add(crit.Alias); + } + AliasKey key = new AliasKey(crit.Alias, wholeAssociationPath); try { - associationPathCriteriaMap.Add(wholeAssociationPath, crit); + associationPathCriteriaMap.Add(key, crit); } catch (ArgumentException ae) { - throw new QueryException("duplicate association path: " + wholeAssociationPath, ae); + throw new QueryException("duplicate association path: " + key, ae); } try { - associationPathJoinTypesMap.Add(wholeAssociationPath, crit.JoinType); + + associationPathJoinTypesMap.Add(key, crit.JoinType); } catch (ArgumentException ae) { - throw new QueryException("duplicate association path: " + wholeAssociationPath, ae); + throw new QueryException("duplicate association path: " + key, ae); } try { - if (crit.WithClause != null) - { - withClauseMap.Add(wholeAssociationPath, crit.WithClause); - } + + withClauseMap.Add(key, crit.WithClause); + } catch (ArgumentException ae) { - throw new QueryException("duplicate association path: " + wholeAssociationPath, ae); + throw new QueryException("duplicate association path: " + key, ae); } } } - private string GetWholeAssociationPath(CriteriaImpl.Subcriteria subcriteria) + private string GetWholeAssociationPath(CriteriaImpl.Subcriteria subcriteria, out string parentAlias) { string path = subcriteria.Path; @@ -379,10 +457,12 @@ private string GetWholeAssociationPath(CriteriaImpl.Subcriteria subcriteria) { // otherwise assume the parent is the the criteria that created us parent = subcriteria.Parent; + parentAlias = parent.Alias; } else { path = StringHelper.Unroot(path); + parentAlias = parent.Alias; } if (parent.Equals(rootCriteria)) @@ -393,7 +473,7 @@ private string GetWholeAssociationPath(CriteriaImpl.Subcriteria subcriteria) else { // otherwise, recurse - return GetWholeAssociationPath((CriteriaImpl.Subcriteria)parent) + '.' + path; + return GetWholeAssociationPath((CriteriaImpl.Subcriteria) parent, out _) + '.' + path; } } @@ -405,11 +485,11 @@ private void CreateCriteriaEntityNameMap() nameCriteriaInfoMap.Add(rootProvider.Name, rootProvider); - foreach (KeyValuePair me in associationPathCriteriaMap) + foreach (KeyValuePair me in associationPathCriteriaMap) { - ICriteriaInfoProvider info = GetPathInfo(me.Key, rootProvider); + ICriteriaInfoProvider info = GetPathInfo(me.Key.Path, rootProvider); criteriaInfoMap.Add(me.Value, info); - nameCriteriaInfoMap[info.Name] = info; + nameCriteriaInfoMap[info.Name] = info; } } @@ -432,9 +512,9 @@ private void CreateEntityJoinMap() private void CreateCriteriaCollectionPersisters() { - foreach (KeyValuePair me in associationPathCriteriaMap) + foreach (KeyValuePair me in associationPathCriteriaMap) { - if (GetPathJoinable(me.Key) is ICollectionPersister collectionPersister) + if (GetPathJoinable(me.Key.Path) is ICollectionPersister collectionPersister) { criteriaCollectionPersisters.Add(collectionPersister); @@ -570,10 +650,12 @@ private void CreateCriteriaSQLAliasMap() alias = me.Value.Name; // the entity name } criteriaSQLAliasMap[crit] = StringHelper.GenerateAlias(alias, i++); - logger.Debug("put criteria={0} alias={1}", - crit, criteriaSQLAliasMap[crit]); + if (!string.IsNullOrEmpty(crit.Alias)) + criteriaSQLAliasToAliasMap[criteriaSQLAliasMap[crit]] = alias; + logger.Debug("put criteria={0} alias={1}", crit, criteriaSQLAliasMap[crit]); } criteriaSQLAliasMap[rootCriteria] = rootSQLAlias; + criteriaSQLAliasToAliasMap[criteriaSQLAliasMap[rootCriteria]] = rootCriteria.Alias; } public bool HasProjection @@ -768,14 +850,13 @@ public string GetPropertyName(string propertyName) return propertyName; } - public SqlString GetWithClause(string path) + public SqlString GetWithClause(string path, string pathAlias) { ICriterion crit; - if (withClauseMap.TryGetValue(path, out crit)) - { - return crit == null ? null : crit.ToSqlString(GetCriteria(path), this); - } - return null; + AliasKey key = new AliasKey(pathAlias, path); + withClauseMap.TryGetValue(key, out crit); + + return crit == null ? null : crit.ToSqlString(GetCriteria(path, key.Alias), this); } #region NH specific diff --git a/src/NHibernate/Loader/JoinWalker.cs b/src/NHibernate/Loader/JoinWalker.cs index fe3ea666811..55eb547c815 100644 --- a/src/NHibernate/Loader/JoinWalker.cs +++ b/src/NHibernate/Loader/JoinWalker.cs @@ -134,15 +134,15 @@ protected JoinWalker(ISessionFactoryImplementor factory, IDictionary private void AddAssociationToJoinTreeIfNecessary(IAssociationType type, string[] aliasedLhsColumns, - string alias, string path, int currentDepth, JoinType joinType) + string alias, string path, string subPathAlias, int currentDepth, JoinType joinType) { if (joinType >= JoinType.InnerJoin) { - AddAssociationToJoinTree(type, aliasedLhsColumns, alias, path, currentDepth, joinType); + AddAssociationToJoinTree(type, aliasedLhsColumns, alias, path, subPathAlias, currentDepth, joinType); } } - protected virtual SqlString GetWithClause(string path) + protected virtual SqlString GetWithClause(string path, string sqlAlias) { return SqlString.Empty; } @@ -152,14 +152,14 @@ protected virtual SqlString GetWithClause(string path) /// of associations to be fetched by outerjoin /// private void AddAssociationToJoinTree(IAssociationType type, string[] aliasedLhsColumns, string alias, - string path, int currentDepth, JoinType joinType) + string path, string subPathAlias, int currentDepth, JoinType joinType) { IJoinable joinable = type.GetAssociatedJoinable(Factory); - string subalias = GenerateTableAlias(associations.Count + 1, path, joinable); + string subalias = GenerateTableAlias(associations.Count + 1, path, subPathAlias, joinable); OuterJoinableAssociation assoc = - new OuterJoinableAssociation(type, alias, aliasedLhsColumns, subalias, joinType, GetWithClause(path), Factory, enabledFilters, GetSelectMode(path)); + new OuterJoinableAssociation(type, alias, aliasedLhsColumns, subalias, joinType, GetWithClause(path, subPathAlias), Factory, enabledFilters, GetSelectMode(path)); assoc.ValidateJoin(path); AddAssociation(subalias, assoc); @@ -175,7 +175,7 @@ private void AddAssociationToJoinTree(IAssociationType type, string[] aliasedLhs { IQueryableCollection qc = joinable as IQueryableCollection; if (qc != null) - WalkCollectionTree(qc, subalias, path, nextDepth); + WalkCollectionTree(qc, subalias, path, subPathAlias, nextDepth); } } @@ -257,18 +257,18 @@ protected void WalkEntityTree(IOuterJoinLoadable persister, string alias) /// protected void WalkCollectionTree(IQueryableCollection persister, string alias) { - WalkCollectionTree(persister, alias, string.Empty, 0); + WalkCollectionTree(persister, alias, string.Empty,string.Empty, 0); //TODO: when this is the entry point, we should use an INNER_JOIN for fetching the many-to-many elements! } /// /// For a collection role, return a list of associations to be fetched by outerjoin /// - private void WalkCollectionTree(IQueryableCollection persister, string alias, string path, int currentDepth) + private void WalkCollectionTree(IQueryableCollection persister, string alias, string path, string subPathAlias, int currentDepth) { if (persister.IsOneToMany) { - WalkEntityTree((IOuterJoinLoadable)persister.ElementPersister, alias, path, currentDepth); + WalkEntityTree((IOuterJoinLoadable)persister.ElementPersister, alias, path, currentDepth); } else { @@ -278,7 +278,7 @@ private void WalkCollectionTree(IQueryableCollection persister, string alias, st // a many-to-many // decrement currentDepth here to allow join across the association table // without exceeding MAX_FETCH_DEPTH (i.e. the "currentDepth - 1" bit) - IAssociationType associationType = (IAssociationType)type; + IAssociationType associationType = (IAssociationType) type; string[] aliasedLhsColumns = persister.GetElementColumnNames(alias); string[] lhsColumns = persister.ElementColumnNames; @@ -288,15 +288,18 @@ private void WalkCollectionTree(IQueryableCollection persister, string alias, st bool useInnerJoin = currentDepth == 0; JoinType joinType = - GetJoinType(associationType, persister.FetchMode, path, persister.TableName, lhsColumns, !useInnerJoin, - currentDepth - 1, null); + GetJoinType(associationType, persister.FetchMode, path, subPathAlias, persister.TableName, lhsColumns, + !useInnerJoin, + currentDepth - 1, null); - AddAssociationToJoinTreeIfNecessary(associationType, aliasedLhsColumns, alias, path, currentDepth - 1, joinType); + AddAssociationToJoinTreeIfNecessary(associationType, aliasedLhsColumns, alias, path, subPathAlias, currentDepth - 1, + joinType); } else if (type.IsComponentType) { - WalkCompositeElementTree((IAbstractComponentType)type, persister.ElementColumnNames, persister, alias, path, - currentDepth); + WalkCompositeElementTree((IAbstractComponentType) type, persister.ElementColumnNames, persister, alias, path, + subPathAlias, + currentDepth); } } } @@ -331,17 +334,26 @@ private void WalkEntityAssociationTree(IAssociationType associationType, IOuterJ string subpath = SubPath(path, persister.GetSubclassPropertyName(propertyNumber)); - JoinType joinType = GetJoinType(associationType, persister.GetFetchMode(propertyNumber), subpath, lhsTable, - lhsColumns, nullable, currentDepth, persister.GetCascadeStyle(propertyNumber)); + //Obtain related aliases to the current path + var subPathAliases = AliasByPath(subpath,alias); + + foreach (var subPathAliase in subPathAliases) + { + JoinType joinType = GetJoinType(associationType, persister.GetFetchMode(propertyNumber), subpath, subPathAliase, + lhsTable, + lhsColumns, nullable, currentDepth, persister.GetCascadeStyle(propertyNumber)); + + AddAssociationToJoinTreeIfNecessary(associationType, aliasedLhsColumns, alias, subpath, subPathAliase, currentDepth, + joinType); + } - AddAssociationToJoinTreeIfNecessary(associationType, aliasedLhsColumns, alias, subpath, currentDepth, joinType); } /// /// For an entity class, add to a list of associations to be fetched /// by outerjoin /// - protected virtual void WalkEntityTree(IOuterJoinLoadable persister, string alias, string path, int currentDepth) + protected virtual void WalkEntityTree(IOuterJoinLoadable persister, string alias, string path, int currentDepth) { int n = persister.CountSubclassProperties(); for (int i = 0; i < n; i++) @@ -381,18 +393,27 @@ protected void WalkComponentTree(IAbstractComponentType componentType, int begin string subpath = SubPath(path, propertyNames[i]); bool[] propertyNullability = componentType.PropertyNullability; + + //Obtain related aliases to the current path + var subPathAliases = AliasByPath(subpath,alias); + foreach (var subPathAliase in subPathAliases) + { + JoinType joinType = GetJoinType(associationType, componentType.GetFetchMode(i), subpath, subPathAliase, lhsTable, + lhsColumns, + propertyNullability == null || propertyNullability[i], currentDepth, + componentType.GetCascadeStyle(i)); + + AddAssociationToJoinTreeIfNecessary(associationType, aliasedLhsColumns, alias, subpath, subPathAliase, + currentDepth, + joinType); + } - JoinType joinType = GetJoinType(associationType, componentType.GetFetchMode(i), subpath, lhsTable, lhsColumns, - propertyNullability == null || propertyNullability[i], currentDepth, - componentType.GetCascadeStyle(i)); - - AddAssociationToJoinTreeIfNecessary(associationType, aliasedLhsColumns, alias, subpath, currentDepth, joinType); } else if (types[i].IsComponentType) { string subpath = SubPath(path, propertyNames[i]); - WalkComponentTree((IAbstractComponentType) types[i], begin, alias, subpath, currentDepth, associationTypeSQLInfo); + WalkComponentTree((IAbstractComponentType) types[i], begin, alias, subpath, currentDepth, associationTypeSQLInfo); } begin += types[i].GetColumnSpan(Factory); } @@ -402,7 +423,7 @@ protected void WalkComponentTree(IAbstractComponentType componentType, int begin /// For a composite element, add to a list of associations to be fetched by outerjoin /// private void WalkCompositeElementTree(IAbstractComponentType compositeType, string[] cols, - IQueryableCollection persister, string alias, string path, int currentDepth) + IQueryableCollection persister, string alias, string path, string subPathAlias, int currentDepth) { IType[] types = compositeType.Subtypes; string[] propertyNames = compositeType.PropertyNames; @@ -423,15 +444,17 @@ private void WalkCompositeElementTree(IAbstractComponentType compositeType, stri bool[] propertyNullability = compositeType.PropertyNullability; JoinType joinType = - GetJoinType(associationType, compositeType.GetFetchMode(i), subpath, persister.TableName, lhsColumns, - propertyNullability == null || propertyNullability[i], currentDepth, compositeType.GetCascadeStyle(i)); + GetJoinType(associationType, compositeType.GetFetchMode(i), subpath, alias, persister.TableName, lhsColumns, + propertyNullability == null || propertyNullability[i], currentDepth, compositeType.GetCascadeStyle(i)); - AddAssociationToJoinTreeIfNecessary(associationType, aliasedLhsColumns, alias, subpath, currentDepth, joinType); + AddAssociationToJoinTreeIfNecessary(associationType, aliasedLhsColumns, alias, subpath, subPathAlias, currentDepth, + joinType); } else if (types[i].IsComponentType) { string subpath = SubPath(path, propertyNames[i]); - WalkCompositeElementTree((IAbstractComponentType)types[i], lhsColumns, persister, alias, subpath, currentDepth); + WalkCompositeElementTree((IAbstractComponentType) types[i], lhsColumns, persister, alias, subpath, subPathAlias, + currentDepth); } begin += length; } @@ -452,8 +475,8 @@ protected static string SubPath(string path, string property) /// association should not be joined. Override on /// subclasses. /// - protected virtual JoinType GetJoinType(IAssociationType type, FetchMode config, string path, string lhsTable, - string[] lhsColumns, bool nullable, int currentDepth, CascadeStyle cascadeStyle) + protected virtual JoinType GetJoinType(IAssociationType type, FetchMode config, string path, string pathAlias, + string lhsTable, string[] lhsColumns, bool nullable, int currentDepth, CascadeStyle cascadeStyle) { if (!IsJoinedFetchEnabled(type, config, cascadeStyle)) return JoinType.None; @@ -468,6 +491,15 @@ protected virtual JoinType GetJoinType(IAssociationType type, FetchMode config, return GetJoinType(nullable, currentDepth); } + /// + /// Returns the "work" alias on the SQL alias and the path to the query field (entity) + /// + protected virtual IList AliasByPath(string path, string sqlAlias) + { + //Returns an empty string to start the default behavior + return new List() {string.Empty}; + } + /// /// Use an inner join if it is a non-null association and this /// is the "first" join in a series @@ -535,7 +567,7 @@ protected virtual bool IsJoinedFetchEnabled(IAssociationType type, FetchMode con return type.IsEntityType && IsJoinedFetchEnabledInMapping(config, type); } - protected virtual string GenerateTableAlias(int n, string path, IJoinable joinable) + protected virtual string GenerateTableAlias(int n, string path, string pathAlias, IJoinable joinable) { return StringHelper.GenerateAlias(joinable.Name, n); } From b6ede57a9b7c8519ba3a622209c6f8130a63f6dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Tue, 20 Nov 2018 16:14:55 +0100 Subject: [PATCH 2/8] Fix compilation after rebase --- .../Loader/Criteria/CriteriaJoinWalker.cs | 10 ++++++++-- .../Loader/Criteria/CriteriaQueryTranslator.cs | 14 +++++++++++--- src/NHibernate/Loader/JoinWalker.cs | 7 ++++--- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs b/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs index 73bdc86df50..c68fc0cb7e8 100644 --- a/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs +++ b/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs @@ -80,9 +80,15 @@ protected override void AddAssociations() foreach (var entityJoinInfo in translator.GetEntityJoins().Values) { var tableAlias = translator.GetSQLAlias(entityJoinInfo.Criteria); - var criteriaPath = entityJoinInfo.Criteria.Alias; + var criteriaPath = entityJoinInfo.Criteria.Path; + var criteriaAlias = entityJoinInfo.Criteria.Alias; var persister = entityJoinInfo.Persister as IOuterJoinLoadable; - AddExplicitEntityJoinAssociation(persister, tableAlias, translator.GetJoinType(criteriaPath), criteriaPath); + AddExplicitEntityJoinAssociation( + persister, + tableAlias, + translator.GetJoinType(criteriaPath, criteriaAlias), + criteriaPath, + criteriaAlias); IncludeInResultIfNeeded(persister, entityJoinInfo.Criteria, tableAlias, criteriaPath); //collect mapped associations for entity join WalkEntityTree(persister, tableAlias, criteriaPath, 1); diff --git a/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs b/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs index 100013fc397..671c36daa0d 100644 --- a/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs +++ b/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs @@ -18,12 +18,20 @@ namespace NHibernate.Loader.Criteria { public class CriteriaQueryTranslator : ICriteriaQuery, ISupportEntityProjectionCriteriaQuery { + // Since v5.2 + [Obsolete("This class has no usage and will be removed")] public class EntityJoinInfo { public ICriteria Criteria; public IQueryable Persister; } + internal class EntityJoinInformation + { + internal CriteriaImpl.Subcriteria Criteria; + internal IQueryable Persister; + } + public static readonly string RootSqlAlias = CriteriaSpecification.RootAlias + '_'; private static readonly INHibernateLogger logger = NHibernateLogger.For(typeof(CriteriaQueryTranslator)); @@ -57,7 +65,7 @@ public class EntityJoinInfo private readonly ICollection namedParameters; private readonly ISet subQuerySpaces = new HashSet(); - private Dictionary entityJoins = new Dictionary(); + private Dictionary entityJoins = new Dictionary(); private readonly IQueryable rootPersister; //Key for the dictionary sub-criteria @@ -169,7 +177,7 @@ public CriteriaImpl RootCriteria ICriteria ISupportEntityProjectionCriteriaQuery.RootCriteria => rootCriteria; - internal IReadOnlyDictionary GetEntityJoins() + internal IReadOnlyDictionary GetEntityJoins() { return entityJoins; } @@ -501,7 +509,7 @@ private void CreateEntityJoinMap() if (criteria.IsEntityJoin) { var entityJoinPersister = GetQueryablePersister(criteria.JoinEntityName); - entityJoins[criteria.Alias] = new EntityJoinInfo + entityJoins[criteria.Alias] = new EntityJoinInformation { Persister = entityJoinPersister, Criteria = criteria, diff --git a/src/NHibernate/Loader/JoinWalker.cs b/src/NHibernate/Loader/JoinWalker.cs index 55eb547c815..c5130dd9ade 100644 --- a/src/NHibernate/Loader/JoinWalker.cs +++ b/src/NHibernate/Loader/JoinWalker.cs @@ -142,7 +142,7 @@ private void AddAssociationToJoinTreeIfNecessary(IAssociationType type, string[] } } - protected virtual SqlString GetWithClause(string path, string sqlAlias) + protected virtual SqlString GetWithClause(string path, string pathAlias) { return SqlString.Empty; } @@ -308,7 +308,8 @@ internal void AddExplicitEntityJoinAssociation( IOuterJoinLoadable persister, string tableAlias, JoinType joinType, - string path) + string path, + string alias) { OuterJoinableAssociation assoc = new OuterJoinableAssociation( @@ -317,7 +318,7 @@ internal void AddExplicitEntityJoinAssociation( Array.Empty(), tableAlias, joinType, - GetWithClause(path), + GetWithClause(path, alias), Factory, enabledFilters, GetSelectMode(path)); From 675c70fd118c5ce9394e558e209b9cbfd94aa5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Wed, 21 Nov 2018 18:13:52 +0100 Subject: [PATCH 3/8] Clean the PR --- .../NHSpecificTest/NH3472/Domain.cs | 21 +-- .../NHSpecificTest/NH3472/Fixture.cs | 73 +++++---- .../NHSpecificTest/NH3472/Mappings.hbm.xml | 25 ++- .../Loader/Criteria/CriteriaJoinWalker.cs | 9 +- .../Criteria/CriteriaQueryTranslator.cs | 119 +++++++------- src/NHibernate/Loader/JoinWalker.cs | 149 +++++++++++++----- 6 files changed, 226 insertions(+), 170 deletions(-) diff --git a/src/NHibernate.Test/NHSpecificTest/NH3472/Domain.cs b/src/NHibernate.Test/NHSpecificTest/NH3472/Domain.cs index 6eb5fa7c35e..4e03e1bd45b 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH3472/Domain.cs +++ b/src/NHibernate.Test/NHSpecificTest/NH3472/Domain.cs @@ -1,27 +1,14 @@ -using System.Collections; using System.Collections.Generic; namespace NHibernate.Test.NHSpecificTest.NH3472 { - public class Cat { - private int id; - private IList children = new List(); + public virtual int Id { get; set; } public virtual string Color { get; set; } - public int Age { get; set; } - - public int Id - { - get { return id; } - set { id = value; } - } + public virtual int Age { get; set; } - public IList Children - { - get { return children; } - set { children = value; } - } + public virtual IList Children { get; set; } = new List(); } -} \ No newline at end of file +} diff --git a/src/NHibernate.Test/NHSpecificTest/NH3472/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/NH3472/Fixture.cs index f872078f51b..be5da48050c 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH3472/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/NH3472/Fixture.cs @@ -8,50 +8,61 @@ namespace NHibernate.Test.NHSpecificTest.NH3472 [TestFixture] public class Fixture : BugTestCase { - [Test] - public void CriteriaQueryWithMultipleJoinsToSameAssociation() + protected override void OnSetUp() { - using (ISession sess = OpenSession()) + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) { - Cat c = new Cat(); - c.Age = 0; - c.Children = new List + var c = new Cat { - new Cat {Color = "Ginger", Age = 1}, - new Cat {Color = "Black", Age = 3} + Age = 4, + Children = new List + { + new Cat { Color = "Ginger", Age = 1 }, + new Cat { Color = "Black", Age = 3 } + } }; - sess.Save(c); - sess.Flush(); + s.Save(c); + t.Commit(); } + } - using (ISession sess = OpenSession()) + protected override void OnTearDown() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) { - try - { - var list = sess.CreateCriteria("cat") - .CreateAlias("cat.Children", "gingerCat", JoinType.LeftOuterJoin, Restrictions.Eq("Color", "Ginger")) + s.Delete("from Cat"); + t.Commit(); + } + } - .CreateAlias("cat.Children", "blackCat", JoinType.LeftOuterJoin, + [Test] + public void CriteriaQueryWithMultipleJoinsToSameAssociation() + { + using (var s = OpenSession()) + { + var list = + s + .CreateCriteria("cat") + .CreateAlias( + "cat.Children", + "gingerCat", + JoinType.LeftOuterJoin, + Restrictions.Eq("Color", "Ginger")) + .CreateAlias( + "cat.Children", + "blackCat", + JoinType.LeftOuterJoin, Restrictions.Eq("Color", "Black")) .SetProjection( Projections.Alias(Projections.Property("gingerCat.Age"), "gingerCatAge"), Projections.Alias(Projections.Property("blackCat.Age"), "blackCatAge") ).AddOrder(new Order(Projections.Property("Age"), true)).List(); - Assert.AreEqual(list.Count, 3); - Assert.AreEqual(list[0][0], 1); - Assert.AreEqual(list[0][1], 3); - - Assert.IsNull(list[1][0]); - Assert.IsNull(list[1][1]); - - Assert.IsNull(list[2][0]); - Assert.IsNull(list[2][1]); - } - finally - { - sess.Delete("from Cat"); - sess.Flush(); - } + Assert.That(list, Has.Count.EqualTo(3)); + Assert.That(list[0], Is.EqualTo(new object[] { null, null })); + Assert.That(list[1], Is.EqualTo(new object[] { null, null })); + Assert.That(list[2], Is.EqualTo(new object[] { 1, 3 })); } } } diff --git a/src/NHibernate.Test/NHSpecificTest/NH3472/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/NH3472/Mappings.hbm.xml index a2564201cbc..bce993f76b0 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH3472/Mappings.hbm.xml +++ b/src/NHibernate.Test/NHSpecificTest/NH3472/Mappings.hbm.xml @@ -1,23 +1,18 @@ - + + assembly="NHibernate.Test" + namespace="NHibernate.Test.NHSpecificTest.NH3472"> - - - + - - + + - - - + + + - - - \ No newline at end of file + diff --git a/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs b/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs index c68fc0cb7e8..b9c9a805d41 100644 --- a/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs +++ b/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs @@ -172,11 +172,12 @@ public override string Comment get { return "criteria query"; } } - protected override IList AliasByPath(string path, string sqlAlias) + /// + protected override IReadOnlyCollection GetChildAliases(string parentSqlAlias, string childPath) { - var alias = translator.AliasByPath(path, sqlAlias); + var alias = translator.GetChildAliases(parentSqlAlias, childPath); if (alias.Count == 0) - return base.AliasByPath(path, sqlAlias); + return base.GetChildAliases(parentSqlAlias, childPath); return alias; } @@ -238,7 +239,7 @@ protected override string GenerateTableAlias(int n, string path, string pathAlia if (shouldCreateUserAlias) { - ICriteria subcriteria = translator.GetCriteria(path, pathAlias); + var subcriteria = translator.GetCriteria(path, pathAlias); sqlAlias = subcriteria == null ? null : translator.GetSQLAlias(subcriteria); IncludeInResultIfNeeded(joinable, subcriteria, sqlAlias, path); diff --git a/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs b/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs index 671c36daa0d..5df1d77a221 100644 --- a/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs +++ b/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs @@ -52,12 +52,12 @@ internal class EntityJoinInformation private readonly HashSet uncacheableCollectionPersisters = new HashSet(); private readonly ISet criteriaCollectionPersisters = new HashSet(); private readonly IDictionary criteriaSQLAliasMap = new Dictionary(); - private readonly IDictionary criteriaSQLAliasToAliasMap = new Dictionary(); - private readonly IDictionary> associationAliasToChild = new Dictionary>(); + private readonly Dictionary sqlAliasToCriteriaAliasMap = new Dictionary(); + private readonly Dictionary> associationAliasToChildrenAliasesMap = new Dictionary>(); private readonly IDictionary aliasCriteriaMap = new Dictionary(); private readonly Dictionary associationPathCriteriaMap = new Dictionary(); - private readonly IDictionary associationPathJoinTypesMap = new LinkedHashMap(); - private readonly IDictionary withClauseMap = new Dictionary(); + private readonly Dictionary associationPathJoinTypesMap = new Dictionary(); + private readonly Dictionary withClauseMap = new Dictionary(); private readonly ISessionFactoryImplementor sessionFactory; private SessionFactoryHelper helper; @@ -69,18 +69,20 @@ internal class EntityJoinInformation private readonly IQueryable rootPersister; //Key for the dictionary sub-criteria - protected class AliasKey + private class AliasKey : IEquatable { - public AliasKey(string alias, string path) { Alias = alias; Path = path; } - protected bool Equals(AliasKey other) + public string Alias { get; } + public string Path { get; } + + public bool Equals(AliasKey other) { - return string.Equals(Alias, other.Alias) && string.Equals(Path, other.Path); + return other != null && string.Equals(Alias, other.Alias) && string.Equals(Path, other.Path); } public override int GetHashCode() @@ -91,21 +93,16 @@ public override int GetHashCode() } } - public string Alias { get; set; } - - public string Path { get; set; } - public override bool Equals(object obj) { - if (ReferenceEquals(null, obj)) return false; - if (ReferenceEquals(this, obj)) return true; - if (obj.GetType() != this.GetType()) return false; - return Equals((AliasKey)obj); + if (ReferenceEquals(this, obj)) + return true; + return Equals(obj as AliasKey); } public override string ToString() { - return string.Format("path: {0} key: {1}", Path, Alias); + return $"path: {Path}; alias: {Alias}"; } } @@ -333,29 +330,26 @@ private ICriteria GetAliasedCriteria(string alias) public bool IsJoin(string path, string critAlias) { - AliasKey key = new AliasKey(critAlias, path); - return associationPathCriteriaMap.ContainsKey(key); + return associationPathCriteriaMap.ContainsKey(new AliasKey(critAlias, path)); } /// - /// Returns a list of aliases criteria along the path and sql alias - /// - public IList AliasByPath(string path, string sqlAlias) + /// Returns the child criteria aliases for a parent SQL alias and a child path. + /// + public IReadOnlyCollection GetChildAliases(string parentSqlAlias, string childPath) { - List alias = new List(); + var alias = new List(); - string critAlias; - if (!criteriaSQLAliasToAliasMap.TryGetValue(sqlAlias, out critAlias)) - critAlias = rootCriteria.Alias; + if (!sqlAliasToCriteriaAliasMap.TryGetValue(parentSqlAlias, out var parentAlias)) + parentAlias = rootCriteria.Alias; - ISet childs; - if (!associationAliasToChild.TryGetValue(critAlias, out childs)) + if (!associationAliasToChildrenAliasesMap.TryGetValue(parentAlias, out var children)) return alias; - foreach (var child in childs) + foreach (var child in children) { - AliasKey key = new AliasKey(child, path); - if (associationPathJoinTypesMap.ContainsKey(key)) alias.Add(child); + if (associationPathJoinTypesMap.ContainsKey(new AliasKey(child, childPath))) + alias.Add(child); } return alias; @@ -363,17 +357,16 @@ public IList AliasByPath(string path, string sqlAlias) public JoinType GetJoinType(string path, string critAlias) { - JoinType result; - if (associationPathJoinTypesMap.TryGetValue(new AliasKey(critAlias, path), out result)) + if (associationPathJoinTypesMap.TryGetValue(new AliasKey(critAlias, path), out var result)) return result; return JoinType.InnerJoin; } public ICriteria GetCriteria(string path, string critAlias) { - AliasKey key = new AliasKey(critAlias, path); + var key = new AliasKey(critAlias, path); associationPathCriteriaMap.TryGetValue(key, out var result); - logger.Debug("getCriteria for key={0} crit={1}", key, result); + logger.Debug("getCriteria for {0}: crit={1}", key, result); return result; } @@ -399,30 +392,30 @@ private void CreateAliasCriteriaMap() private void CreateAssociationPathCriteriaMap() { - foreach (CriteriaImpl.Subcriteria crit in rootCriteria.IterateSubcriteria()) + foreach (var crit in rootCriteria.IterateSubcriteria()) { - string parentAlias; - string wholeAssociationPath = GetWholeAssociationPath(crit, out parentAlias); - if (parentAlias != null) - { - ISet childAliases; - if (!associationAliasToChild.TryGetValue(parentAlias, out childAliases)) - associationAliasToChild.Add(parentAlias, new HashSet()); - associationAliasToChild[parentAlias].Add(crit.Alias); - } - AliasKey key = new AliasKey(crit.Alias, wholeAssociationPath); + var wholeAssociationPath = GetWholeAssociationPath(crit, out var parentAlias); + if (parentAlias != null) + { + if (!associationAliasToChildrenAliasesMap.TryGetValue(parentAlias, out var children)) + { + children = new HashSet(); + associationAliasToChildrenAliasesMap.Add(parentAlias, children); + } + children.Add(crit.Alias); + } + var key = new AliasKey(crit.Alias, wholeAssociationPath); try { - associationPathCriteriaMap.Add(key, crit); + associationPathCriteriaMap.Add(key, crit); } catch (ArgumentException ae) { - throw new QueryException("duplicate association path: " + key, ae); + throw new QueryException("duplicate association path: " + key, ae); } try { - associationPathJoinTypesMap.Add(key, crit.JoinType); } catch (ArgumentException ae) @@ -432,9 +425,7 @@ private void CreateAssociationPathCriteriaMap() try { - withClauseMap.Add(key, crit.WithClause); - } catch (ArgumentException ae) { @@ -465,14 +456,14 @@ private string GetWholeAssociationPath(CriteriaImpl.Subcriteria subcriteria, out { // otherwise assume the parent is the the criteria that created us parent = subcriteria.Parent; - parentAlias = parent.Alias; } else { path = StringHelper.Unroot(path); - parentAlias = parent.Alias; } + parentAlias = parent.Alias; + if (parent.Equals(rootCriteria)) { // if its the root criteria, we are done @@ -493,9 +484,9 @@ private void CreateCriteriaEntityNameMap() nameCriteriaInfoMap.Add(rootProvider.Name, rootProvider); - foreach (KeyValuePair me in associationPathCriteriaMap) + foreach (var me in associationPathCriteriaMap) { - ICriteriaInfoProvider info = GetPathInfo(me.Key.Path, rootProvider); + var info = GetPathInfo(me.Key.Path, rootProvider); criteriaInfoMap.Add(me.Value, info); nameCriteriaInfoMap[info.Name] = info; } @@ -520,7 +511,7 @@ private void CreateEntityJoinMap() private void CreateCriteriaCollectionPersisters() { - foreach (KeyValuePair me in associationPathCriteriaMap) + foreach (var me in associationPathCriteriaMap) { if (GetPathJoinable(me.Key.Path) is ICollectionPersister collectionPersister) { @@ -657,13 +648,14 @@ private void CreateCriteriaSQLAliasMap() { alias = me.Value.Name; // the entity name } - criteriaSQLAliasMap[crit] = StringHelper.GenerateAlias(alias, i++); - if (!string.IsNullOrEmpty(crit.Alias)) - criteriaSQLAliasToAliasMap[criteriaSQLAliasMap[crit]] = alias; + var sqlAlias = StringHelper.GenerateAlias(alias, i++); + criteriaSQLAliasMap[crit] = sqlAlias; logger.Debug("put criteria={0} alias={1}", crit, criteriaSQLAliasMap[crit]); + if (!string.IsNullOrEmpty(crit.Alias)) + sqlAliasToCriteriaAliasMap[sqlAlias] = alias; } criteriaSQLAliasMap[rootCriteria] = rootSQLAlias; - criteriaSQLAliasToAliasMap[criteriaSQLAliasMap[rootCriteria]] = rootCriteria.Alias; + sqlAliasToCriteriaAliasMap[rootSQLAlias] = rootCriteria.Alias; } public bool HasProjection @@ -860,11 +852,10 @@ public string GetPropertyName(string propertyName) public SqlString GetWithClause(string path, string pathAlias) { - ICriterion crit; - AliasKey key = new AliasKey(pathAlias, path); - withClauseMap.TryGetValue(key, out crit); + var key = new AliasKey(pathAlias, path); + withClauseMap.TryGetValue(key, out var crit); - return crit == null ? null : crit.ToSqlString(GetCriteria(path, key.Alias), this); + return crit?.ToSqlString(GetCriteria(path, key.Alias), this); } #region NH specific diff --git a/src/NHibernate/Loader/JoinWalker.cs b/src/NHibernate/Loader/JoinWalker.cs index c5130dd9ade..f5b2237dafe 100644 --- a/src/NHibernate/Loader/JoinWalker.cs +++ b/src/NHibernate/Loader/JoinWalker.cs @@ -158,8 +158,17 @@ private void AddAssociationToJoinTree(IAssociationType type, string[] aliasedLhs string subalias = GenerateTableAlias(associations.Count + 1, path, subPathAlias, joinable); - OuterJoinableAssociation assoc = - new OuterJoinableAssociation(type, alias, aliasedLhsColumns, subalias, joinType, GetWithClause(path, subPathAlias), Factory, enabledFilters, GetSelectMode(path)); + var assoc = + new OuterJoinableAssociation( + type, + alias, + aliasedLhsColumns, + subalias, + joinType, + GetWithClause(path, subPathAlias), + Factory, + enabledFilters, + GetSelectMode(path)); assoc.ValidateJoin(path); AddAssociation(subalias, assoc); @@ -257,7 +266,7 @@ protected void WalkEntityTree(IOuterJoinLoadable persister, string alias) /// protected void WalkCollectionTree(IQueryableCollection persister, string alias) { - WalkCollectionTree(persister, alias, string.Empty,string.Empty, 0); + WalkCollectionTree(persister, alias, string.Empty, string.Empty, 0); //TODO: when this is the entry point, we should use an INNER_JOIN for fetching the many-to-many elements! } @@ -268,7 +277,7 @@ private void WalkCollectionTree(IQueryableCollection persister, string alias, st { if (persister.IsOneToMany) { - WalkEntityTree((IOuterJoinLoadable)persister.ElementPersister, alias, path, currentDepth); + WalkEntityTree((IOuterJoinLoadable)persister.ElementPersister, alias, path, currentDepth); } else { @@ -287,17 +296,35 @@ private void WalkCollectionTree(IQueryableCollection persister, string alias, st // an inner join... bool useInnerJoin = currentDepth == 0; - JoinType joinType = - GetJoinType(associationType, persister.FetchMode, path, subPathAlias, persister.TableName, lhsColumns, + var joinType = + GetJoinType( + associationType, + persister.FetchMode, + path, + subPathAlias, + persister.TableName, + lhsColumns, !useInnerJoin, - currentDepth - 1, null); - - AddAssociationToJoinTreeIfNecessary(associationType, aliasedLhsColumns, alias, path, subPathAlias, currentDepth - 1, + currentDepth - 1, + null); + + AddAssociationToJoinTreeIfNecessary( + associationType, + aliasedLhsColumns, + alias, + path, + subPathAlias, + currentDepth - 1, joinType); } else if (type.IsComponentType) { - WalkCompositeElementTree((IAbstractComponentType) type, persister.ElementColumnNames, persister, alias, path, + WalkCompositeElementTree( + (IAbstractComponentType) type, + persister.ElementColumnNames, + persister, + alias, + path, subPathAlias, currentDepth); } @@ -335,26 +362,37 @@ private void WalkEntityAssociationTree(IAssociationType associationType, IOuterJ string subpath = SubPath(path, persister.GetSubclassPropertyName(propertyNumber)); - //Obtain related aliases to the current path - var subPathAliases = AliasByPath(subpath,alias); - - foreach (var subPathAliase in subPathAliases) + // Obtain children aliases for the current path and alias + var subPathAliases = GetChildAliases(alias, subpath); + foreach (var subPathAlias in subPathAliases) { - JoinType joinType = GetJoinType(associationType, persister.GetFetchMode(propertyNumber), subpath, subPathAliase, + var joinType = GetJoinType( + associationType, + persister.GetFetchMode(propertyNumber), + subpath, + subPathAlias, lhsTable, - lhsColumns, nullable, currentDepth, persister.GetCascadeStyle(propertyNumber)); - - AddAssociationToJoinTreeIfNecessary(associationType, aliasedLhsColumns, alias, subpath, subPathAliase, currentDepth, + lhsColumns, + nullable, + currentDepth, + persister.GetCascadeStyle(propertyNumber)); + + AddAssociationToJoinTreeIfNecessary( + associationType, + aliasedLhsColumns, + alias, + subpath, + subPathAlias, + currentDepth, joinType); } - } /// /// For an entity class, add to a list of associations to be fetched /// by outerjoin /// - protected virtual void WalkEntityTree(IOuterJoinLoadable persister, string alias, string path, int currentDepth) + protected virtual void WalkEntityTree(IOuterJoinLoadable persister, string alias, string path, int currentDepth) { int n = persister.CountSubclassProperties(); for (int i = 0; i < n; i++) @@ -394,27 +432,37 @@ protected void WalkComponentTree(IAbstractComponentType componentType, int begin string subpath = SubPath(path, propertyNames[i]); bool[] propertyNullability = componentType.PropertyNullability; - - //Obtain related aliases to the current path - var subPathAliases = AliasByPath(subpath,alias); - foreach (var subPathAliase in subPathAliases) + + // Obtain related aliases to the current path + var subPathAliases = GetChildAliases(alias, subpath); + foreach (var subPathAlias in subPathAliases) { - JoinType joinType = GetJoinType(associationType, componentType.GetFetchMode(i), subpath, subPathAliase, lhsTable, + var joinType = GetJoinType( + associationType, + componentType.GetFetchMode(i), + subpath, + subPathAlias, + lhsTable, lhsColumns, - propertyNullability == null || propertyNullability[i], currentDepth, + propertyNullability == null || propertyNullability[i], + currentDepth, componentType.GetCascadeStyle(i)); - AddAssociationToJoinTreeIfNecessary(associationType, aliasedLhsColumns, alias, subpath, subPathAliase, + AddAssociationToJoinTreeIfNecessary( + associationType, + aliasedLhsColumns, + alias, + subpath, + subPathAlias, currentDepth, joinType); } - } else if (types[i].IsComponentType) { string subpath = SubPath(path, propertyNames[i]); - WalkComponentTree((IAbstractComponentType) types[i], begin, alias, subpath, currentDepth, associationTypeSQLInfo); + WalkComponentTree((IAbstractComponentType) types[i], begin, alias, subpath, currentDepth, associationTypeSQLInfo); } begin += types[i].GetColumnSpan(Factory); } @@ -444,17 +492,37 @@ private void WalkCompositeElementTree(IAbstractComponentType compositeType, stri string subpath = SubPath(path, propertyNames[i]); bool[] propertyNullability = compositeType.PropertyNullability; - JoinType joinType = - GetJoinType(associationType, compositeType.GetFetchMode(i), subpath, alias, persister.TableName, lhsColumns, - propertyNullability == null || propertyNullability[i], currentDepth, compositeType.GetCascadeStyle(i)); + var joinType = + GetJoinType( + associationType, + compositeType.GetFetchMode(i), + subpath, + alias, + persister.TableName, + lhsColumns, + propertyNullability == null || propertyNullability[i], + currentDepth, + compositeType.GetCascadeStyle(i)); - AddAssociationToJoinTreeIfNecessary(associationType, aliasedLhsColumns, alias, subpath, subPathAlias, currentDepth, + AddAssociationToJoinTreeIfNecessary( + associationType, + aliasedLhsColumns, + alias, + subpath, + subPathAlias, + currentDepth, joinType); } else if (types[i].IsComponentType) { string subpath = SubPath(path, propertyNames[i]); - WalkCompositeElementTree((IAbstractComponentType) types[i], lhsColumns, persister, alias, subpath, subPathAlias, + WalkCompositeElementTree( + (IAbstractComponentType) types[i], + lhsColumns, + persister, + alias, + subpath, + subPathAlias, currentDepth); } begin += length; @@ -492,13 +560,16 @@ protected virtual JoinType GetJoinType(IAssociationType type, FetchMode config, return GetJoinType(nullable, currentDepth); } + // By default, multiple aliases for a child are not supported. There is only one and its value + // does not matter for default implementation. + private static readonly IReadOnlyCollection DefaultChildAliases = new[] { string.Empty }; + /// - /// Returns the "work" alias on the SQL alias and the path to the query field (entity) - /// - protected virtual IList AliasByPath(string path, string sqlAlias) + /// Returns the child criteria aliases for a parent SQL alias and a child path. + /// + protected virtual IReadOnlyCollection GetChildAliases(string parentSqlAlias, string childPath) { - //Returns an empty string to start the default behavior - return new List() {string.Empty}; + return DefaultChildAliases; } /// From a5c9ad74e1d96263e1c4a9038040c28ef7d49b49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Wed, 21 Nov 2018 18:21:39 +0100 Subject: [PATCH 4/8] Generate async --- .../Async/NHSpecificTest/NH3472/Fixture.cs | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 src/NHibernate.Test/Async/NHSpecificTest/NH3472/Fixture.cs diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH3472/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH3472/Fixture.cs new file mode 100644 index 00000000000..08552d68121 --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH3472/Fixture.cs @@ -0,0 +1,80 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by AsyncGenerator. +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + + +using System.Collections.Generic; +using NHibernate.Criterion; +using NHibernate.SqlCommand; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.NH3472 +{ + using System.Threading.Tasks; + [TestFixture] + public class FixtureAsync : BugTestCase + { + protected override void OnSetUp() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var c = new Cat + { + Age = 4, + Children = new List + { + new Cat { Color = "Ginger", Age = 1 }, + new Cat { Color = "Black", Age = 3 } + } + }; + s.Save(c); + t.Commit(); + } + } + + protected override void OnTearDown() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + s.Delete("from Cat"); + t.Commit(); + } + } + + [Test] + public async Task CriteriaQueryWithMultipleJoinsToSameAssociationAsync() + { + using (var s = OpenSession()) + { + var list = + await (s + .CreateCriteria("cat") + .CreateAlias( + "cat.Children", + "gingerCat", + JoinType.LeftOuterJoin, + Restrictions.Eq("Color", "Ginger")) + .CreateAlias( + "cat.Children", + "blackCat", + JoinType.LeftOuterJoin, + Restrictions.Eq("Color", "Black")) + .SetProjection( + Projections.Alias(Projections.Property("gingerCat.Age"), "gingerCatAge"), + Projections.Alias(Projections.Property("blackCat.Age"), "blackCatAge") + ).AddOrder(new Order(Projections.Property("Age"), true)).ListAsync()); + Assert.That(list, Has.Count.EqualTo(3)); + Assert.That(list[0], Is.EqualTo(new object[] { null, null })); + Assert.That(list[1], Is.EqualTo(new object[] { null, null })); + Assert.That(list[2], Is.EqualTo(new object[] { 1, 3 })); + } + } + } +} From 6cf6fc8f6abdad4c17b066b7ef0b6899b3abd5b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Thu, 22 Nov 2018 13:12:28 +0100 Subject: [PATCH 5/8] Avoid binary breaking changes --- .../Criteria/CriteriaQueryTranslator.cs | 44 +++++++++++++++++++ src/NHibernate/Loader/JoinWalker.cs | 44 +++++++++++++++++-- 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs b/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs index 5df1d77a221..929fa5f92b0 100644 --- a/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs +++ b/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs @@ -328,6 +328,13 @@ private ICriteria GetAliasedCriteria(string alias) return result; } + // Since v5.2 + [Obsolete("Use overload with a critAlias additional parameter", true)] + public bool IsJoin(string path) + { + return associationPathCriteriaMap.Keys.Any(k => k.Path == path); + } + public bool IsJoin(string path, string critAlias) { return associationPathCriteriaMap.ContainsKey(new AliasKey(critAlias, path)); @@ -355,6 +362,17 @@ public IReadOnlyCollection GetChildAliases(string parentSqlAlias, string return alias; } + // Since v5.2 + [Obsolete("Use overload with a critAlias additional parameter", true)] + public JoinType GetJoinType(string path) + { + return + associationPathJoinTypesMap + .Where(kv => kv.Key.Path == path) + .Select(kv => (JoinType?) kv.Value) + .SingleOrDefault() ?? JoinType.InnerJoin; + } + public JoinType GetJoinType(string path, string critAlias) { if (associationPathJoinTypesMap.TryGetValue(new AliasKey(critAlias, path), out var result)) @@ -362,6 +380,19 @@ public JoinType GetJoinType(string path, string critAlias) return JoinType.InnerJoin; } + // Since v5.2 + [Obsolete("Use overload with a critAlias additional parameter", true)] + public ICriteria GetCriteria(string path) + { + var result = + associationPathCriteriaMap + .Where(kv => kv.Key.Path == path) + .Select(kv => kv.Value) + .SingleOrDefault(); + logger.Debug("getCriteria for path {0}: crit={1}", path, result); + return result; + } + public ICriteria GetCriteria(string path, string critAlias) { var key = new AliasKey(critAlias, path); @@ -850,6 +881,19 @@ public string GetPropertyName(string propertyName) return propertyName; } + // Since v5.2 + [Obsolete("Use overload with a critAlias additional parameter", true)] + public SqlString GetWithClause(string path) + { + var crit = + withClauseMap + .Where(kv => kv.Key.Path == path) + .Select(kv => kv.Value) + .SingleOrDefault(); + + return crit?.ToSqlString(GetCriteria(path), this); + } + public SqlString GetWithClause(string path, string pathAlias) { var key = new AliasKey(pathAlias, path); diff --git a/src/NHibernate/Loader/JoinWalker.cs b/src/NHibernate/Loader/JoinWalker.cs index f5b2237dafe..160fd39ea32 100644 --- a/src/NHibernate/Loader/JoinWalker.cs +++ b/src/NHibernate/Loader/JoinWalker.cs @@ -142,11 +142,21 @@ private void AddAssociationToJoinTreeIfNecessary(IAssociationType type, string[] } } - protected virtual SqlString GetWithClause(string path, string pathAlias) + // Since v5.2 + [Obsolete("Use or override the overload taking a pathAlias additional parameter")] + protected virtual SqlString GetWithClause(string path) { return SqlString.Empty; } + protected virtual SqlString GetWithClause(string path, string pathAlias) + { + // 6.0 TODO: inline the call +#pragma warning disable 618 + return GetWithClause(path); +#pragma warning restore 618 + } + /// /// Add on association (one-to-one, many-to-one, or a collection) to a list /// of associations to be fetched by outerjoin @@ -544,8 +554,10 @@ protected static string SubPath(string path, string property) /// association should not be joined. Override on /// subclasses. /// - protected virtual JoinType GetJoinType(IAssociationType type, FetchMode config, string path, string pathAlias, - string lhsTable, string[] lhsColumns, bool nullable, int currentDepth, CascadeStyle cascadeStyle) + // Since v5.2 + [Obsolete("Use or override the overload taking a pathAlias additional parameter")] + protected virtual JoinType GetJoinType(IAssociationType type, FetchMode config, string path, string lhsTable, + string[] lhsColumns, bool nullable, int currentDepth, CascadeStyle cascadeStyle) { if (!IsJoinedFetchEnabled(type, config, cascadeStyle)) return JoinType.None; @@ -560,6 +572,20 @@ protected virtual JoinType GetJoinType(IAssociationType type, FetchMode config, return GetJoinType(nullable, currentDepth); } + /// + /// Get the join type (inner, outer, etc) or -1 if the + /// association should not be joined. Override on + /// subclasses. + /// + protected virtual JoinType GetJoinType(IAssociationType type, FetchMode config, string path, string pathAlias, + string lhsTable, string[] lhsColumns, bool nullable, int currentDepth, CascadeStyle cascadeStyle) + { + // 6.0 TODO: inline the call +#pragma warning disable 618 + return GetJoinType(type, config, path, lhsTable, lhsColumns, nullable, currentDepth, cascadeStyle); +#pragma warning restore 618 + } + // By default, multiple aliases for a child are not supported. There is only one and its value // does not matter for default implementation. private static readonly IReadOnlyCollection DefaultChildAliases = new[] { string.Empty }; @@ -639,11 +665,21 @@ protected virtual bool IsJoinedFetchEnabled(IAssociationType type, FetchMode con return type.IsEntityType && IsJoinedFetchEnabledInMapping(config, type); } - protected virtual string GenerateTableAlias(int n, string path, string pathAlias, IJoinable joinable) + // Since v5.2 + [Obsolete("Use or override the overload taking a pathAlias additional parameter")] + protected virtual string GenerateTableAlias(int n, string path, IJoinable joinable) { return StringHelper.GenerateAlias(joinable.Name, n); } + protected virtual string GenerateTableAlias(int n, string path, string pathAlias, IJoinable joinable) + { + // 6.0 TODO: inline the call +#pragma warning disable 618 + return GenerateTableAlias(n, path, joinable); +#pragma warning restore 618 + } + protected virtual string GenerateRootAlias(string description) { return StringHelper.GenerateAlias(description, 0); From 5cdb8904dc92d701aa9c6e2cf93ed50fc50750cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Fri, 23 Nov 2018 17:54:56 +0100 Subject: [PATCH 6/8] Do additional cleanup --- src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs b/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs index 929fa5f92b0..63d89cd42a2 100644 --- a/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs +++ b/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs @@ -681,7 +681,7 @@ private void CreateCriteriaSQLAliasMap() } var sqlAlias = StringHelper.GenerateAlias(alias, i++); criteriaSQLAliasMap[crit] = sqlAlias; - logger.Debug("put criteria={0} alias={1}", crit, criteriaSQLAliasMap[crit]); + logger.Debug("put criteria={0} alias={1}", crit, sqlAlias); if (!string.IsNullOrEmpty(crit.Alias)) sqlAliasToCriteriaAliasMap[sqlAlias] = alias; } From 18e10adde26db7ef08c2a1297db21662c5487463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Fri, 23 Nov 2018 18:53:27 +0100 Subject: [PATCH 7/8] Add some more tests --- .../Async/NHSpecificTest/NH3472/Fixture.cs | 82 +++++++++++++++++-- .../NHSpecificTest/NH3472/Domain.cs | 3 +- .../NHSpecificTest/NH3472/Fixture.cs | 82 +++++++++++++++++-- .../NHSpecificTest/NH3472/Mappings.hbm.xml | 5 +- 4 files changed, 159 insertions(+), 13 deletions(-) diff --git a/src/NHibernate.Test/Async/NHSpecificTest/NH3472/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/NH3472/Fixture.cs index 08552d68121..fb8d95d5d49 100644 --- a/src/NHibernate.Test/Async/NHSpecificTest/NH3472/Fixture.cs +++ b/src/NHibernate.Test/Async/NHSpecificTest/NH3472/Fixture.cs @@ -26,11 +26,18 @@ protected override void OnSetUp() { var c = new Cat { - Age = 4, - Children = new List + Age = 6, + Children = new HashSet { - new Cat { Color = "Ginger", Age = 1 }, - new Cat { Color = "Black", Age = 3 } + new Cat + { + Age = 4, + Children = new HashSet + { + new Cat { Color = "Ginger", Age = 1 }, + new Cat { Color = "Black", Age = 3 } + } + } } }; s.Save(c); @@ -70,10 +77,75 @@ public async Task CriteriaQueryWithMultipleJoinsToSameAssociationAsync() Projections.Alias(Projections.Property("gingerCat.Age"), "gingerCatAge"), Projections.Alias(Projections.Property("blackCat.Age"), "blackCatAge") ).AddOrder(new Order(Projections.Property("Age"), true)).ListAsync()); - Assert.That(list, Has.Count.EqualTo(3)); + Assert.That(list, Has.Count.EqualTo(4)); Assert.That(list[0], Is.EqualTo(new object[] { null, null })); Assert.That(list[1], Is.EqualTo(new object[] { null, null })); Assert.That(list[2], Is.EqualTo(new object[] { 1, 3 })); + Assert.That(list[3], Is.EqualTo(new object[] { null, null })); + } + } + + [Test] + public async Task QueryWithFetchesAndAliasDoNotDuplicateJoinAsync() + { + using (var s = OpenSession()) + { + Cat parent = null; + using (var spy = new SqlLogSpy()) + { + var list = + await (s + .QueryOver() + .Fetch(SelectMode.Fetch, o => o.Parent) + .Fetch(SelectMode.Fetch, o => o.Parent.Parent) + .JoinAlias(o => o.Parent, () => parent) + .Where(x => parent.Age == 4) + .ListAsync()); + + // Two joins to Cat are expected: one for the immediate parent, and a second for the grand-parent. + // So checking if it does not contain three joins or more. (The regex uses "[\s\S]" instead of "." + // because the SQL is formatted by default and contains "\n" which are not matched by ".".) + Assert.That(spy.GetWholeLog(), Does.Not.Match(@"(?:\bjoin\s*Cat\b[\s\S]*){3,}").IgnoreCase); + Assert.That(list, Has.Count.EqualTo(2)); + Assert.That( + NHibernateUtil.IsInitialized(list[0].Parent), + Is.True, + "first cat parent initialization status"); + Assert.That( + NHibernateUtil.IsInitialized(list[1].Parent), + Is.True, + "second cat parent initialization status"); + Assert.That( + NHibernateUtil.IsInitialized(list[0].Parent.Parent), + Is.True, + "first cat parent parent initialization status"); + Assert.That( + NHibernateUtil.IsInitialized(list[1].Parent.Parent), + Is.True, + "second cat parent parent initialization status"); + } + } + } + + [Test, Explicit("Debatable use case")] + public async Task QueryWithFetchesAndMultipleJoinsToSameAssociationAsync() + { + using (var s = OpenSession()) + { + Cat ginger = null; + Cat black = null; + var list = + await (s + .QueryOver() + .Fetch(SelectMode.Fetch, o => o.Children) + .JoinAlias(o => o.Children, () => ginger) + .Where(x => ginger.Color == "Ginger") + .JoinAlias(o => o.Children, () => black) + .Where(x => black.Color == "Black") + .ListAsync()); + + Assert.That(list, Has.Count.EqualTo(1)); + Assert.That(list[0].Children, Has.Count.EqualTo(2)); } } } diff --git a/src/NHibernate.Test/NHSpecificTest/NH3472/Domain.cs b/src/NHibernate.Test/NHSpecificTest/NH3472/Domain.cs index 4e03e1bd45b..14a655d43ae 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH3472/Domain.cs +++ b/src/NHibernate.Test/NHSpecificTest/NH3472/Domain.cs @@ -8,7 +8,8 @@ public class Cat public virtual string Color { get; set; } public virtual int Age { get; set; } + public virtual Cat Parent { get; set; } - public virtual IList Children { get; set; } = new List(); + public virtual ISet Children { get; set; } = new HashSet(); } } diff --git a/src/NHibernate.Test/NHSpecificTest/NH3472/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/NH3472/Fixture.cs index be5da48050c..aee93d374b2 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH3472/Fixture.cs +++ b/src/NHibernate.Test/NHSpecificTest/NH3472/Fixture.cs @@ -15,11 +15,18 @@ protected override void OnSetUp() { var c = new Cat { - Age = 4, - Children = new List + Age = 6, + Children = new HashSet { - new Cat { Color = "Ginger", Age = 1 }, - new Cat { Color = "Black", Age = 3 } + new Cat + { + Age = 4, + Children = new HashSet + { + new Cat { Color = "Ginger", Age = 1 }, + new Cat { Color = "Black", Age = 3 } + } + } } }; s.Save(c); @@ -59,10 +66,75 @@ public void CriteriaQueryWithMultipleJoinsToSameAssociation() Projections.Alias(Projections.Property("gingerCat.Age"), "gingerCatAge"), Projections.Alias(Projections.Property("blackCat.Age"), "blackCatAge") ).AddOrder(new Order(Projections.Property("Age"), true)).List(); - Assert.That(list, Has.Count.EqualTo(3)); + Assert.That(list, Has.Count.EqualTo(4)); Assert.That(list[0], Is.EqualTo(new object[] { null, null })); Assert.That(list[1], Is.EqualTo(new object[] { null, null })); Assert.That(list[2], Is.EqualTo(new object[] { 1, 3 })); + Assert.That(list[3], Is.EqualTo(new object[] { null, null })); + } + } + + [Test] + public void QueryWithFetchesAndAliasDoNotDuplicateJoin() + { + using (var s = OpenSession()) + { + Cat parent = null; + using (var spy = new SqlLogSpy()) + { + var list = + s + .QueryOver() + .Fetch(SelectMode.Fetch, o => o.Parent) + .Fetch(SelectMode.Fetch, o => o.Parent.Parent) + .JoinAlias(o => o.Parent, () => parent) + .Where(x => parent.Age == 4) + .List(); + + // Two joins to Cat are expected: one for the immediate parent, and a second for the grand-parent. + // So checking if it does not contain three joins or more. (The regex uses "[\s\S]" instead of "." + // because the SQL is formatted by default and contains "\n" which are not matched by ".".) + Assert.That(spy.GetWholeLog(), Does.Not.Match(@"(?:\bjoin\s*Cat\b[\s\S]*){3,}").IgnoreCase); + Assert.That(list, Has.Count.EqualTo(2)); + Assert.That( + NHibernateUtil.IsInitialized(list[0].Parent), + Is.True, + "first cat parent initialization status"); + Assert.That( + NHibernateUtil.IsInitialized(list[1].Parent), + Is.True, + "second cat parent initialization status"); + Assert.That( + NHibernateUtil.IsInitialized(list[0].Parent.Parent), + Is.True, + "first cat parent parent initialization status"); + Assert.That( + NHibernateUtil.IsInitialized(list[1].Parent.Parent), + Is.True, + "second cat parent parent initialization status"); + } + } + } + + [Test, Explicit("Debatable use case")] + public void QueryWithFetchesAndMultipleJoinsToSameAssociation() + { + using (var s = OpenSession()) + { + Cat ginger = null; + Cat black = null; + var list = + s + .QueryOver() + .Fetch(SelectMode.Fetch, o => o.Children) + .JoinAlias(o => o.Children, () => ginger) + .Where(x => ginger.Color == "Ginger") + .JoinAlias(o => o.Children, () => black) + .Where(x => black.Color == "Black") + .List(); + + Assert.That(list, Has.Count.EqualTo(1)); + Assert.That(list[0].Children, Has.Count.EqualTo(2)); } } } diff --git a/src/NHibernate.Test/NHSpecificTest/NH3472/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/NH3472/Mappings.hbm.xml index bce993f76b0..cd40636acdc 100644 --- a/src/NHibernate.Test/NHSpecificTest/NH3472/Mappings.hbm.xml +++ b/src/NHibernate.Test/NHSpecificTest/NH3472/Mappings.hbm.xml @@ -8,11 +8,12 @@ + - + - + From 78f900f06efa2e0f699162302243e7b00c28d8be Mon Sep 17 00:00:00 2001 From: Roman Artiukhin Date: Sat, 24 Nov 2018 11:23:17 +0300 Subject: [PATCH 8/8] Remove unrelated changes --- .../Loader/Criteria/CriteriaJoinWalker.cs | 9 ++------- .../Loader/Criteria/CriteriaQueryTranslator.cs | 14 +++----------- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs b/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs index b9c9a805d41..4ec1d841976 100644 --- a/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs +++ b/src/NHibernate/Loader/Criteria/CriteriaJoinWalker.cs @@ -80,15 +80,10 @@ protected override void AddAssociations() foreach (var entityJoinInfo in translator.GetEntityJoins().Values) { var tableAlias = translator.GetSQLAlias(entityJoinInfo.Criteria); - var criteriaPath = entityJoinInfo.Criteria.Path; + var criteriaPath = entityJoinInfo.Criteria.Alias; //path for entity join is equal to alias var criteriaAlias = entityJoinInfo.Criteria.Alias; var persister = entityJoinInfo.Persister as IOuterJoinLoadable; - AddExplicitEntityJoinAssociation( - persister, - tableAlias, - translator.GetJoinType(criteriaPath, criteriaAlias), - criteriaPath, - criteriaAlias); + AddExplicitEntityJoinAssociation(persister, tableAlias, translator.GetJoinType(criteriaPath, criteriaAlias), criteriaPath, criteriaAlias); IncludeInResultIfNeeded(persister, entityJoinInfo.Criteria, tableAlias, criteriaPath); //collect mapped associations for entity join WalkEntityTree(persister, tableAlias, criteriaPath, 1); diff --git a/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs b/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs index 63d89cd42a2..05749b3fdce 100644 --- a/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs +++ b/src/NHibernate/Loader/Criteria/CriteriaQueryTranslator.cs @@ -18,20 +18,12 @@ namespace NHibernate.Loader.Criteria { public class CriteriaQueryTranslator : ICriteriaQuery, ISupportEntityProjectionCriteriaQuery { - // Since v5.2 - [Obsolete("This class has no usage and will be removed")] public class EntityJoinInfo { public ICriteria Criteria; public IQueryable Persister; } - internal class EntityJoinInformation - { - internal CriteriaImpl.Subcriteria Criteria; - internal IQueryable Persister; - } - public static readonly string RootSqlAlias = CriteriaSpecification.RootAlias + '_'; private static readonly INHibernateLogger logger = NHibernateLogger.For(typeof(CriteriaQueryTranslator)); @@ -65,7 +57,7 @@ internal class EntityJoinInformation private readonly ICollection namedParameters; private readonly ISet subQuerySpaces = new HashSet(); - private Dictionary entityJoins = new Dictionary(); + private Dictionary entityJoins = new Dictionary(); private readonly IQueryable rootPersister; //Key for the dictionary sub-criteria @@ -174,7 +166,7 @@ public CriteriaImpl RootCriteria ICriteria ISupportEntityProjectionCriteriaQuery.RootCriteria => rootCriteria; - internal IReadOnlyDictionary GetEntityJoins() + internal IReadOnlyDictionary GetEntityJoins() { return entityJoins; } @@ -531,7 +523,7 @@ private void CreateEntityJoinMap() if (criteria.IsEntityJoin) { var entityJoinPersister = GetQueryablePersister(criteria.JoinEntityName); - entityJoins[criteria.Alias] = new EntityJoinInformation + entityJoins[criteria.Alias] = new EntityJoinInfo { Persister = entityJoinPersister, Criteria = criteria,