Skip to content

Commit 2281916

Browse files
ChrisJollyAUChrisJollyAU
committed
Fix generating sets (union, intersect). The code was originally executing and returning before it could handle adding the SQL for the Alias in (including wrapping the section in brackets). This realigns with how the parent VisitSelect works
1 parent d78256f commit 2281916

File tree

7 files changed

+5543
-5629
lines changed

7 files changed

+5543
-5629
lines changed

‎src/EFCore.Jet/Query/Sql/Internal/JetQuerySqlGenerator.cs

Lines changed: 152 additions & 150 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,6 @@ protected override Expression VisitSelect(SelectExpression selectExpression)
7272
// workaround.
7373
// Should be kept in sync with the base class.
7474

75-
if (IsNonComposedSetOperation(selectExpression))
76-
{
77-
// Naked set operation
78-
GenerateSetOperation((SetOperationBase)selectExpression.Tables[0]);
79-
80-
return selectExpression;
81-
}
82-
8375
IDisposable? subQueryIndent = null;
8476

8577
if (selectExpression.Alias != null)
@@ -88,202 +80,212 @@ protected override Expression VisitSelect(SelectExpression selectExpression)
8880
subQueryIndent = Sql.Indent();
8981
}
9082

91-
Sql.Append("SELECT ");
92-
93-
if (selectExpression.IsDistinct)
83+
if (!TryGenerateWithoutWrappingSelect(selectExpression))
9484
{
95-
Sql.Append("DISTINCT ");
96-
}
9785

98-
if (selectExpression.Tags.Contains("DeepSkip"))
99-
{
86+
Sql.Append("SELECT ");
10087

101-
}
102-
else
103-
{
104-
GenerateTop(selectExpression);
105-
}
106-
107-
108-
if (selectExpression.Projection.Any())
109-
{
110-
GenerateList(selectExpression.Projection, e => Visit(e));
111-
}
112-
else
113-
{
114-
Sql.Append("1");
115-
}
88+
if (selectExpression.IsDistinct)
89+
{
90+
Sql.Append("DISTINCT ");
91+
}
11692

117-
List<ColumnExpression> colexp = new List<ColumnExpression>();
118-
// Implement Jet's non-standard JOIN syntax and DUAL table workaround.
119-
// TODO: This does not properly handle all cases (especially when cross joins are involved).
120-
if (selectExpression.Tables.Any())
121-
{
122-
Sql.AppendLine()
123-
.Append("FROM ");
93+
if (selectExpression.Tags.Contains("DeepSkip"))
94+
{
12495

125-
const int maxTablesWithoutBrackets = 2;
96+
}
97+
else
98+
{
99+
GenerateTop(selectExpression);
100+
}
126101

127-
Sql.Append(
128-
new string(
129-
'(',
130-
Math.Max(
131-
0,
132-
selectExpression
133-
.Tables
134-
.Count(t => !(t is CrossJoinExpression || t is CrossApplyExpression)) -
135-
maxTablesWithoutBrackets)));
136102

137-
for (var index = 0; index < selectExpression.Tables.Count; index++)
103+
if (selectExpression.Projection.Any())
138104
{
139-
var tableExpression = selectExpression.Tables[index];
105+
GenerateList(selectExpression.Projection, e => Visit(e));
106+
}
107+
else
108+
{
109+
Sql.Append("1");
110+
}
140111

141-
var isApplyExpression = tableExpression is CrossApplyExpression ||
142-
tableExpression is OuterApplyExpression;
112+
List<ColumnExpression> colexp = new List<ColumnExpression>();
113+
// Implement Jet's non-standard JOIN syntax and DUAL table workaround.
114+
// TODO: This does not properly handle all cases (especially when cross joins are involved).
115+
if (selectExpression.Tables.Any())
116+
{
117+
Sql.AppendLine()
118+
.Append("FROM ");
119+
120+
const int maxTablesWithoutBrackets = 2;
121+
122+
Sql.Append(
123+
new string(
124+
'(',
125+
Math.Max(
126+
0,
127+
selectExpression
128+
.Tables
129+
.Count(t => !(t is CrossJoinExpression || t is CrossApplyExpression)) -
130+
maxTablesWithoutBrackets)));
131+
132+
for (var index = 0; index < selectExpression.Tables.Count; index++)
133+
{
134+
var tableExpression = selectExpression.Tables[index];
143135

144-
var isCrossExpression = tableExpression is CrossJoinExpression ||
145-
tableExpression is CrossApplyExpression;
136+
var isApplyExpression = tableExpression is CrossApplyExpression ||
137+
tableExpression is OuterApplyExpression;
146138

147-
if (isApplyExpression)
148-
{
149-
throw new InvalidOperationException(
150-
"Jet does not support APPLY statements. Switch to client evaluation explicitly by inserting a call to either AsEnumerable(), AsAsyncEnumerable(), ToList(), or ToListAsync() if needed.");
151-
}
139+
var isCrossExpression = tableExpression is CrossJoinExpression ||
140+
tableExpression is CrossApplyExpression;
152141

153-
if (index > 0)
154-
{
155-
if (isCrossExpression)
142+
if (isApplyExpression)
156143
{
157-
Sql.Append(",");
158-
}
159-
else if (index >= maxTablesWithoutBrackets)
160-
{
161-
Sql.Append(")");
144+
throw new InvalidOperationException(
145+
"Jet does not support APPLY statements. Switch to client evaluation explicitly by inserting a call to either AsEnumerable(), AsAsyncEnumerable(), ToList(), or ToListAsync() if needed.");
162146
}
163147

164-
Sql.AppendLine();
165-
}
166-
List<ColumnExpression> tempcolexp;
167-
if (tableExpression is InnerJoinExpression expression)
168-
{
169-
SqlBinaryExpression? binaryJoin = expression.JoinPredicate as SqlBinaryExpression;
170-
tempcolexp = ExtractColumnExpressions(binaryJoin!);
171-
bool refrencesfirsttable = false;
172-
foreach (ColumnExpression col in tempcolexp)
148+
if (index > 0)
173149
{
174-
if (col.Table == selectExpression.Tables[0])
150+
if (isCrossExpression)
175151
{
176-
refrencesfirsttable = true;
177-
break;
152+
Sql.Append(",");
153+
}
154+
else if (index >= maxTablesWithoutBrackets)
155+
{
156+
Sql.Append(")");
178157
}
179-
}
180158

181-
if (refrencesfirsttable)
182-
{
183-
Visit(tableExpression);
184-
continue;
159+
Sql.AppendLine();
185160
}
186-
else
187-
{
188-
colexp.AddRange(tempcolexp);
189-
}
190-
/*if (expression.JoinPredicate is SqlBinaryExpression { Left: ColumnExpression left, Right: ColumnExpression right })
161+
162+
List<ColumnExpression> tempcolexp;
163+
if (tableExpression is InnerJoinExpression expression)
191164
{
192-
var lt = left.Table == selectExpression.Tables[0];
193-
var rt = right.Table == selectExpression.Tables[0];
194-
if (lt || rt)
165+
SqlBinaryExpression? binaryJoin = expression.JoinPredicate as SqlBinaryExpression;
166+
tempcolexp = ExtractColumnExpressions(binaryJoin!);
167+
bool refrencesfirsttable = false;
168+
foreach (ColumnExpression col in tempcolexp)
169+
{
170+
if (col.Table == selectExpression.Tables[0])
171+
{
172+
refrencesfirsttable = true;
173+
break;
174+
}
175+
}
176+
177+
if (refrencesfirsttable)
195178
{
196179
Visit(tableExpression);
197180
continue;
198181
}
199182
else
200183
{
201-
colexp.Add(left);
202-
colexp.Add(right);
184+
colexp.AddRange(tempcolexp);
203185
}
204-
}*/
205-
Sql.Append("LEFT JOIN ");
206-
Visit(expression.Table);
207-
Sql.Append(" ON ");
208-
Visit(expression.JoinPredicate);
209-
}
210-
else
211-
{
212-
Visit(tableExpression);
186+
187+
/*if (expression.JoinPredicate is SqlBinaryExpression { Left: ColumnExpression left, Right: ColumnExpression right })
188+
{
189+
var lt = left.Table == selectExpression.Tables[0];
190+
var rt = right.Table == selectExpression.Tables[0];
191+
if (lt || rt)
192+
{
193+
Visit(tableExpression);
194+
continue;
195+
}
196+
else
197+
{
198+
colexp.Add(left);
199+
colexp.Add(right);
200+
}
201+
}*/
202+
Sql.Append("LEFT JOIN ");
203+
Visit(expression.Table);
204+
Sql.Append(" ON ");
205+
Visit(expression.JoinPredicate);
206+
}
207+
else
208+
{
209+
Visit(tableExpression);
210+
}
213211
}
214212
}
215-
}
216-
else
217-
{
218-
GeneratePseudoFromClause();
219-
}
220-
221-
if (selectExpression.Predicate != null || colexp.Count > 0)
222-
{
223-
Sql.AppendLine()
224-
.Append("WHERE ");
225-
226-
if (selectExpression.Predicate != null)
213+
else
227214
{
228-
if (colexp.Count > 0) Sql.Append("(");
229-
Visit(selectExpression.Predicate);
230-
if (colexp.Count > 0) Sql.Append(")");
215+
GeneratePseudoFromClause();
231216
}
232217

233-
if (selectExpression.Predicate != null && colexp.Count > 0)
218+
if (selectExpression.Predicate != null || colexp.Count > 0)
234219
{
235-
Sql.Append(" AND (");
236-
}
220+
Sql.AppendLine()
221+
.Append("WHERE ");
237222

238-
if (colexp.Count > 0)
239-
{
240-
int ct = 0;
241-
foreach (var exp in colexp)
223+
if (selectExpression.Predicate != null)
242224
{
243-
if (!string.IsNullOrEmpty(exp.TableAlias))
244-
{
245-
Sql.Append($"`{exp.TableAlias}`.");
246-
}
247-
Sql.Append($"`{exp.Name}` IS NOT NULL");
248-
if (ct < colexp.Count - 1)
225+
if (colexp.Count > 0) Sql.Append("(");
226+
Visit(selectExpression.Predicate);
227+
if (colexp.Count > 0) Sql.Append(")");
228+
}
229+
230+
if (selectExpression.Predicate != null && colexp.Count > 0)
231+
{
232+
Sql.Append(" AND (");
233+
}
234+
235+
if (colexp.Count > 0)
236+
{
237+
int ct = 0;
238+
foreach (var exp in colexp)
249239
{
250-
ct++;
251-
Sql.Append(" AND ");
240+
if (!string.IsNullOrEmpty(exp.TableAlias))
241+
{
242+
Sql.Append($"`{exp.TableAlias}`.");
243+
}
244+
245+
Sql.Append($"`{exp.Name}` IS NOT NULL");
246+
if (ct < colexp.Count - 1)
247+
{
248+
ct++;
249+
Sql.Append(" AND ");
250+
}
252251
}
253252
}
253+
254+
if (selectExpression.Predicate != null && colexp.Count > 0)
255+
{
256+
Sql.Append(")");
257+
}
254258
}
255259

256-
if (selectExpression.Predicate != null && colexp.Count > 0)
260+
if (selectExpression.GroupBy.Count > 0)
257261
{
258-
Sql.Append(")");
262+
Sql.AppendLine()
263+
.Append("GROUP BY ");
264+
265+
GenerateList(selectExpression.GroupBy, e => Visit(e));
259266
}
260-
}
261267

262-
if (selectExpression.GroupBy.Count > 0)
263-
{
264-
Sql.AppendLine()
265-
.Append("GROUP BY ");
268+
if (selectExpression.Having != null)
269+
{
270+
Sql.AppendLine()
271+
.Append("HAVING ");
266272

267-
GenerateList(selectExpression.GroupBy, e => Visit(e));
268-
}
273+
Visit(selectExpression.Having);
274+
}
269275

270-
if (selectExpression.Having != null)
271-
{
272-
Sql.AppendLine()
273-
.Append("HAVING ");
276+
GenerateOrderings(selectExpression);
277+
GenerateLimitOffset(selectExpression);
274278

275-
Visit(selectExpression.Having);
276279
}
277280

278-
GenerateOrderings(selectExpression);
279-
GenerateLimitOffset(selectExpression);
280-
281281
if (selectExpression.Alias != null)
282282
{
283283
subQueryIndent!.Dispose();
284284

285285
Sql.AppendLine()
286-
.Append(")" + AliasSeparator + _sqlGenerationHelper.DelimitIdentifier(selectExpression.Alias));
286+
.Append(")")
287+
.Append(AliasSeparator)
288+
.Append(_sqlGenerationHelper.DelimitIdentifier(selectExpression.Alias));
287289
}
288290

289291
return selectExpression;

‎test/EFCore.Jet.FunctionalTests/Query/SimpleQueryJetTest.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -519,15 +519,15 @@ public override async Task Hierarchy_query_with_abstract_type_sibling_TPC(bool a
519519

520520
AssertSql(
521521
"""
522-
SELECT [t].[Id], [t].[Species], [t].[Name], [t].[EdcuationLevel], [t].[FavoriteToy], [t].[Discriminator]
522+
SELECT `t`.`Id`, `t`.`Species`, `t`.`Name`, `t`.`EdcuationLevel`, `t`.`FavoriteToy`, `t`.`Discriminator`
523523
FROM (
524-
SELECT [c].[Id], [c].[Species], [c].[Name], [c].[EdcuationLevel], NULL AS [FavoriteToy], N'Cat' AS [Discriminator]
525-
FROM [Cats] AS [c]
524+
SELECT `c`.`Id`, `c`.`Species`, `c`.`Name`, `c`.`EdcuationLevel`, NULL AS `FavoriteToy`, 'Cat' AS `Discriminator`
525+
FROM `Cats` AS `c`
526526
UNION ALL
527-
SELECT [d].[Id], [d].[Species], [d].[Name], NULL AS [EdcuationLevel], [d].[FavoriteToy], N'Dog' AS [Discriminator]
528-
FROM [Dogs] AS [d]
529-
) AS [t]
530-
WHERE ([t].[Species] IS NOT NULL) AND ([t].[Species] LIKE N'F%')
527+
SELECT `d`.`Id`, `d`.`Species`, `d`.`Name`, NULL AS `EdcuationLevel`, `d`.`FavoriteToy`, 'Dog' AS `Discriminator`
528+
FROM `Dogs` AS `d`
529+
) AS `t`
530+
WHERE `t`.`Species` LIKE 'F%'
531531
""");
532532
}
533533

0 commit comments

Comments
 (0)