Skip to content

Commit 67f05f3

Browse files
maca88maca88
committed
Code review changes.
1 parent e80ee97 commit 67f05f3

File tree

5 files changed

+157
-129
lines changed

5 files changed

+157
-129
lines changed

‎src/NHibernate/Hql/Ast/ANTLR/Tree/FromElement.cs‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -584,8 +584,12 @@ public IType GetPropertyType(string propertyName, string propertyPath)
584584
public virtual string GetIdentityColumn()
585585
{
586586
var cols = GetIdentityColumns();
587-
string result = string.Join(", ", cols);
587+
if (cols == null)
588+
{
589+
return null;
590+
}
588591

592+
string result = string.Join(", ", cols);
589593
if (cols.Length > 1 && Walker.IsComparativeExpressionClause)
590594
{
591595
return "(" + result + ")";
@@ -603,10 +607,14 @@ internal string[] GetIdentityColumns()
603607
{
604608
throw new InvalidOperationException("No table alias for node " + this);
605609
}
606-
string[] cols;
610+
611+
return GetIdentityColumns(table);
612+
}
613+
614+
internal virtual string[] GetIdentityColumns(string alias)
615+
{
607616
string propertyName;
608-
if (EntityPersister != null && EntityPersister.EntityMetamodel != null
609-
&& EntityPersister.EntityMetamodel.HasNonIdentifierPropertyNamedId)
617+
if (EntityPersister?.EntityMetamodel?.HasNonIdentifierPropertyNamedId == true)
610618
{
611619
propertyName = EntityPersister.IdentifierPropertyName;
612620
}
@@ -619,11 +627,9 @@ internal string[] GetIdentityColumns()
619627
propertyName = NHibernate.Persister.Entity.EntityPersister.EntityID;
620628
}
621629

622-
cols = UseTableAliases
623-
? GetPropertyMapping(propertyName).ToColumns(table, propertyName)
630+
return UseTableAliases
631+
? GetPropertyMapping(propertyName).ToColumns(alias, propertyName)
624632
: GetPropertyMapping(propertyName).ToColumns(propertyName);
625-
626-
return cols;
627633
}
628634

629635
internal bool UseTableAliases => Walker.StatementType == HqlSqlWalker.SELECT || Walker.IsSubQuery;

‎src/NHibernate/Hql/Ast/ANTLR/Tree/JoinSubqueryFromElement.cs‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
using System.Linq;
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
24
using Antlr.Runtime;
35
using NHibernate.Engine;
46
using NHibernate.Persister.Entity;
@@ -19,9 +21,10 @@ public JoinSubqueryFromElement(FromClause fromClause, QueryNode queryNode, JoinT
1921
QueryNode = queryNode;
2022
DataType = dataType;
2123
foreach (var fromElement in querySelectClause.SelectExpressions.Select(o => o.FromElement)
22-
.Union(querySelectClause.NonScalarExpressions?.Select(o => o.FromElement) ?? Enumerable.Empty<FromElement>())
23-
.Union(querySelectClause.CollectionFromElements ?? Enumerable.Empty<FromElement>())
24-
.Where(o => o != null))
24+
.Concat(querySelectClause.NonScalarExpressions?.Select(o => o.FromElement) ?? Enumerable.Empty<FromElement>())
25+
.Concat(querySelectClause.CollectionFromElements ?? Enumerable.Empty<FromElement>())
26+
.Where(o => o != null)
27+
.Distinct())
2528
{
2629
fromElement.ParentFromElement = this;
2730
}
@@ -65,16 +68,24 @@ public SqlString RenderText(SqlString subQuery, ISessionFactoryImplementor sessi
6568
renderText.Substring(index + 7));
6669
}
6770

68-
public override string GetIdentityColumn()
71+
internal List<ISelectExpression> GetRelatedSelectExpressions(DotNode dotNode, out SelectClause selectClause)
6972
{
70-
// Return null for a scalar subquery instead of throwing an exception.
71-
// The node will be removed in the SelectClause
72-
if (DataType is SubqueryComponentType && PropertyMapping.GetIdentifiersColumns(TableAlias).Count == 0)
73+
return PropertyMapping.GetRelatedSelectExpressions(dotNode.PropertyPath, out selectClause);
74+
}
75+
76+
internal override string[] GetIdentityColumns(string alias)
77+
{
78+
if (DataType is SubqueryComponentType)
7379
{
74-
return null;
80+
var idColumns = PropertyMapping.GetIdentifiersColumns(alias);
81+
// Return null for a scalar subquery instead of throwing an exception.
82+
// The node will be removed in the SelectClause
83+
return idColumns.Count == 0
84+
? null
85+
: idColumns.ToArray();
7586
}
7687

77-
return base.GetIdentityColumn();
88+
return base.GetIdentityColumns(alias);
7889
}
7990
}
8091
}

‎src/NHibernate/Hql/Ast/ANTLR/Tree/SelectClause.cs‎

Lines changed: 103 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,15 @@ public void InitializeDerivedSelectClause(FromClause fromClause)
6767
_derivedSelectExpressions = new HashSet<ISelectExpression>();
6868
foreach (FromElement fromElement in fromElements)
6969
{
70-
if (fromElement.SelectType == null || fromElement.IsFetch || fromElement.IsCollectionOfValuesOrComponents)
70+
IType type;
71+
if (fromElement.IsFetch || fromElement.IsCollectionOfValuesOrComponents || ((type = fromElement.SelectType) == null))
7172
{
7273
continue;
7374
}
7475

7576
var node = (IdentNode)appender.Append(HqlSqlWalker.IDENT, fromElement.ClassAlias ?? "", true);
7677
node.FromElement = fromElement;
77-
node.DataType = fromElement.SelectType;
78+
node.DataType = type;
7879
_derivedSelectExpressions.Add(node);
7980
}
8081

@@ -112,69 +113,22 @@ public void InitializeExplicitSelectClause(FromClause fromClause)
112113
{
113114
_constructorNode = (ConstructorNode) expr;
114115
_scalarSelect = true;
115-
NonScalarExpressions?.AddRange(_constructorNode.GetSelectExpressions(true, o => !o.IsScalar));
116+
SelectExpressions.RemoveAt(i);
117+
NonScalarExpressions.AddRange(_constructorNode.GetSelectExpressions(true, o => !o.IsScalar));
116118
foreach (var argumentExpression in _constructorNode.GetSelectExpressions())
117119
{
118120
SelectExpressions.Insert(i, argumentExpression);
119121
i++;
120122
AddExpression(argumentExpression, queryReturnTypeList);
121123
}
122124

123-
SelectExpressions.Remove(expr);
124-
length = SelectExpressions.Count;
125125
i--;
126+
length = SelectExpressions.Count;
126127
}
127-
else if (expr.FromElement is JoinSubqueryFromElement joinSubquery)
128+
else if (expr.FromElement is JoinSubqueryFromElement joinSubquery &&
129+
TryProcessSubqueryExpressions(expr, joinSubquery, out var selectClause, out var subqueryExpressions))
128130
{
129-
SelectClause selectClause;
130-
List<ISelectExpression> subqueryExpressions;
131-
if (expr is IdentNode)
132-
{
133-
selectClause = joinSubquery.QueryNode.GetSelectClause();
134-
subqueryExpressions = selectClause.SelectExpressions;
135-
NonScalarExpressions.Add(expr);
136-
}
137-
else if (expr is DotNode dotNode)
138-
{
139-
var relatedExpressions = joinSubquery.PropertyMapping.GetRelatedSelectExpressions(dotNode.PropertyPath, out selectClause);
140-
if (relatedExpressions == null)
141-
{
142-
if (!expr.IsScalar)
143-
{
144-
NonScalarExpressions.Add(expr);
145-
}
146-
147-
AddExpression(expr, queryReturnTypeList);
148-
continue;
149-
}
150-
151-
if (!selectClause.IsScalarSelect)
152-
{
153-
RemoveChildAndUnsetParent((IASTNode) expr);
154-
}
155-
156-
subqueryExpressions = new List<ISelectExpression>();
157-
foreach (var relatedExpression in relatedExpressions)
158-
{
159-
if (!relatedExpression.IsScalar)
160-
{
161-
NonScalarExpressions.Add(relatedExpression);
162-
}
163-
164-
subqueryExpressions.Add(relatedExpression);
165-
}
166-
}
167-
else
168-
{
169-
if (!expr.IsScalar)
170-
{
171-
NonScalarExpressions?.Add(expr);
172-
}
173-
174-
AddExpression(expr, queryReturnTypeList);
175-
continue;
176-
}
177-
131+
SelectExpressions.RemoveAt(i);
178132
var indexes = new List<int>(subqueryExpressions.Count);
179133
foreach (var expression in subqueryExpressions)
180134
{
@@ -185,16 +139,15 @@ public void InitializeExplicitSelectClause(FromClause fromClause)
185139
AddExpression(expression, queryReturnTypeList);
186140
}
187141

188-
_replacedExpressions.Add(expr, indexes);
189-
SelectExpressions.Remove(expr);
190-
length = SelectExpressions.Count;
191142
i--;
143+
length = SelectExpressions.Count;
144+
_replacedExpressions.Add(expr, indexes);
192145
}
193146
else
194147
{
195148
if (!expr.IsScalar)
196149
{
197-
NonScalarExpressions?.Add(expr);
150+
NonScalarExpressions.Add(expr);
198151
}
199152

200153
AddExpression(expr, queryReturnTypeList);
@@ -214,6 +167,49 @@ public void InitializeExplicitSelectClause(FromClause fromClause)
214167
FinishInitialization();
215168
}
216169

170+
private bool TryProcessSubqueryExpressions(
171+
ISelectExpression selectExpression,
172+
JoinSubqueryFromElement joinSubquery,
173+
out SelectClause selectClause,
174+
out List<ISelectExpression> subqueryExpressions)
175+
{
176+
if (selectExpression is IdentNode)
177+
{
178+
selectClause = joinSubquery.QueryNode.GetSelectClause();
179+
subqueryExpressions = selectClause.SelectExpressions;
180+
NonScalarExpressions.Add(selectExpression);
181+
}
182+
else if (selectExpression is DotNode dotNode)
183+
{
184+
subqueryExpressions = joinSubquery.GetRelatedSelectExpressions(dotNode, out selectClause);
185+
if (subqueryExpressions == null)
186+
{
187+
return false;
188+
}
189+
190+
if (!selectClause.IsScalarSelect)
191+
{
192+
RemoveChildAndUnsetParent((IASTNode) selectExpression);
193+
}
194+
195+
foreach (var expression in subqueryExpressions)
196+
{
197+
if (!expression.IsScalar)
198+
{
199+
NonScalarExpressions.Add(expression);
200+
}
201+
}
202+
}
203+
else
204+
{
205+
selectClause = null;
206+
subqueryExpressions = null;
207+
return false;
208+
}
209+
210+
return true;
211+
}
212+
217213
private void Render(
218214
FromClause fromClause,
219215
Dictionary<ISelectExpression, SelectClause> inheritedExpressions)
@@ -454,6 +450,46 @@ private void RenderNonScalarSelects(
454450
var appender = new ASTAppender(ASTFactory, this);
455451
var combinedFromElements = new List<FromElement>();
456452
var processedElements = new HashSet<FromElement>();
453+
RenderNonScalarIdentifiers(appender, processedElements, combinedFromElements, inheritedExpressions);
454+
if (Walker.IsShallowQuery)
455+
{
456+
return;
457+
}
458+
459+
// Append fetched elements
460+
RenderFetchedNonScalarIdentifiers(appender, fetchedFromElements, processedElements, combinedFromElements);
461+
if (currentFromClause.IsScalarSubQuery)
462+
{
463+
return;
464+
}
465+
466+
// Generate the property select tokens.
467+
foreach (var fromElement in combinedFromElements)
468+
{
469+
RenderNonScalarProperties(appender, fromElement);
470+
}
471+
472+
// Generate properties for fetched collections of components or values
473+
var fromElements = currentFromClause.GetAllProjectionListTyped();
474+
foreach (var fromElement in fromElements)
475+
{
476+
if (fromElement.IsCollectionOfValuesOrComponents &&
477+
fromElement.IsFetch &&
478+
processedElements.Add(fromElement))
479+
{
480+
var suffix = Walker.GetSuffix(fromElement);
481+
var fragment = fromElement.GetValueCollectionSelectFragment(suffix);
482+
Append(appender, HqlSqlWalker.SQL_TOKEN, fragment);
483+
}
484+
}
485+
}
486+
487+
private void RenderNonScalarIdentifiers(
488+
ASTAppender appender,
489+
HashSet<FromElement> processedElements,
490+
List<FromElement> combinedFromElements,
491+
Dictionary<ISelectExpression, SelectClause> inheritedExpressions)
492+
{
457493
foreach (var e in NonScalarExpressions)
458494
{
459495
var fromElement = e.FromElement;
@@ -473,13 +509,14 @@ private void RenderNonScalarSelects(
473509
RemoveChildAndUnsetParent(node);
474510
}
475511
}
512+
}
476513

477-
if (Walker.IsShallowQuery)
478-
{
479-
return;
480-
}
481-
482-
// Append fetched elements
514+
private void RenderFetchedNonScalarIdentifiers(
515+
ASTAppender appender,
516+
IList<FromElement> fetchedFromElements,
517+
HashSet<FromElement> processedElements,
518+
List<FromElement> combinedFromElements)
519+
{
483520
foreach (var fetchedFromElement in fetchedFromElements)
484521
{
485522
if (!processedElements.Add(fetchedFromElement))
@@ -493,39 +530,14 @@ private void RenderNonScalarSelects(
493530
if (fragment == null)
494531
{
495532
// When a subquery join has a scalar select only
496-
continue;
533+
continue;
497534
}
498535

499536
var generatedExpr = (SelectExpressionImpl) Append(appender, HqlSqlWalker.SELECT_EXPR, fragment);
500537
generatedExpr.FromElement = fetchedFromElement;
501538
generatedExpr.DataType = fetchedFromElement.DataType;
502539
NonScalarExpressions.Add(generatedExpr);
503540
}
504-
505-
if (currentFromClause.IsScalarSubQuery)
506-
{
507-
return;
508-
}
509-
510-
// Generate the property select tokens.
511-
foreach (var fromElement in combinedFromElements)
512-
{
513-
RenderNonScalarProperties(appender, fromElement);
514-
}
515-
516-
// Generate properties for fetched collections of components or values
517-
var fromElements = currentFromClause.GetAllProjectionListTyped();
518-
foreach (var fromElement in fromElements)
519-
{
520-
if (fromElement.IsCollectionOfValuesOrComponents &&
521-
fromElement.IsFetch &&
522-
processedElements.Add(fromElement))
523-
{
524-
var suffix = Walker.GetSuffix(fromElement);
525-
var fragment = fromElement.GetValueCollectionSelectFragment(suffix);
526-
Append(appender, HqlSqlWalker.SQL_TOKEN, fragment);
527-
}
528-
}
529541
}
530542

531543
private IASTNode Append(ASTAppender appender, int type, SelectFragment fragment)

‎src/NHibernate/Persister/Collection/IQueryableCollection.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,18 +119,18 @@ public static SelectFragment GetSelectFragment(this IQueryableCollection queryab
119119
var identifierAlias = queryable.GetIdentifierColumnAlias(null);
120120
var indexAliases = queryable.GetIndexColumnAliases(null);
121121
var columnAliases = queryable.GetKeyColumnAliases(null)
122-
.Union(queryable.GetElementColumnAliases(null));
122+
.Concat(queryable.GetElementColumnAliases(null));
123123
if (indexAliases != null)
124124
{
125-
columnAliases = columnAliases.Union(indexAliases);
125+
columnAliases = columnAliases.Concat(indexAliases);
126126
}
127127

128128
if (identifierAlias != null)
129129
{
130-
columnAliases = columnAliases.Union(new[] {identifierAlias});
130+
columnAliases = columnAliases.Concat(new[] {identifierAlias});
131131
}
132132

133-
return new SelectFragment(queryable.Factory.Dialect, renderedText, columnAliases.ToList())
133+
return new SelectFragment(queryable.Factory.Dialect, renderedText, columnAliases.Distinct().ToList())
134134
.SetSuffix(columnSuffix);
135135
}
136136
}

0 commit comments

Comments
 (0)