Skip to content
This repository was archived by the owner on Dec 24, 2022. It is now read-only.

Commit d1e6504

Browse files
mythzmythz
authored
Merge pull request #515 from shift-evgeny/SqlExpressionJoinFixes
Fixed bug in .GroupBy() introduced in PR #514
2 parents 4c60114 + e385f3e commit d1e6504

File tree

9 files changed

+82
-44
lines changed

9 files changed

+82
-44
lines changed

‎src/ServiceStack.OrmLite/Expressions/SqlExpression.cs

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -461,9 +461,12 @@ public virtual SqlExpression<T> GroupBy<TKey>(Expression<Func<T, TKey>> keySelec
461461
{
462462
sep = string.Empty;
463463
useFieldName = true;
464-
return GroupBy(Visit(keySelector).ToString());
465-
}
466464

465+
var groupByKey = Visit(keySelector);
466+
StripAliases(groupByKey as SelectList); // No "AS ColumnAlias" in GROUP BY, just the column names/expressions
467+
468+
return GroupBy(groupByKey.ToString());
469+
}
467470

468471
public virtual SqlExpression<T> Having()
469472
{
@@ -888,7 +891,8 @@ public virtual SqlExpression<T> Insert<TKey>(Expression<Func<T, TKey>> fields)
888891
{
889892
sep = string.Empty;
890893
useFieldName = false;
891-
InsertFields = Visit(fields).ToString().Split(',').ToList();
894+
var fieldList = Visit(fields);
895+
InsertFields = fieldList.ToString().Split(',').Select(f => f.Trim()).ToList();
892896
return this;
893897
}
894898

@@ -1450,17 +1454,12 @@ protected virtual object VisitNew(NewExpression nex)
14501454
{
14511455
var exprs = VisitExpressionList(nex.Arguments);
14521456

1453-
var r = StringBuilderCache.Allocate();
14541457
for (var i = 0; i < exprs.Count; ++i)
14551458
{
14561459
exprs[i] = SetAnonTypePropertyNamesForSelectExpression(exprs[i], nex.Arguments[i], nex.Members[i]);
1457-
1458-
if (i > 0)
1459-
r.Append(",");
1460-
1461-
r.Append(exprs[i]);
14621460
}
1463-
return StringBuilderCache.ReturnAndFree(r);
1461+
1462+
return new SelectList(exprs);
14641463
}
14651464

14661465
return CachedExpressionCompiler.Evaluate(nex);
@@ -1485,16 +1484,20 @@ private object SetAnonTypePropertyNamesForSelectExpression(object expr, Expressi
14851484
{
14861485
foreach (var item in selectList.Items)
14871486
{
1488-
if (!string.IsNullOrEmpty(item.Alias))
1487+
var selectItem = item as SelectItem;
1488+
if (selectItem != null)
14891489
{
1490-
item.Alias = member.Name + item.Alias;
1491-
}
1492-
else
1493-
{
1494-
var columnItem = item as SelectItemColumn;
1495-
if (columnItem != null)
1490+
if (!string.IsNullOrEmpty(selectItem.Alias))
1491+
{
1492+
selectItem.Alias = member.Name + selectItem.Alias;
1493+
}
1494+
else
14961495
{
1497-
columnItem.Alias = member.Name + columnItem.ColumnName;
1496+
var columnItem = item as SelectItemColumn;
1497+
if (columnItem != null)
1498+
{
1499+
columnItem.Alias = member.Name + columnItem.ColumnName;
1500+
}
14981501
}
14991502
}
15001503
}
@@ -1503,11 +1506,26 @@ private object SetAnonTypePropertyNamesForSelectExpression(object expr, Expressi
15031506
return expr;
15041507
}
15051508

1506-
class SelectList
1509+
private static void StripAliases(SelectList selectList)
15071510
{
1508-
public readonly SelectItem[] Items;
1511+
if (selectList == null)
1512+
return;
15091513

1510-
public SelectList(SelectItem[] items)
1514+
foreach (var item in selectList.Items)
1515+
{
1516+
var selectItem = item as SelectItem;
1517+
if (selectItem != null)
1518+
{
1519+
selectItem.Alias = null;
1520+
}
1521+
}
1522+
}
1523+
1524+
private class SelectList
1525+
{
1526+
public readonly IEnumerable<object> Items;
1527+
1528+
public SelectList(IEnumerable<object> items)
15111529
{
15121530
this.Items = items;
15131531
}
@@ -2162,7 +2180,10 @@ public SelectItemExpression(IOrmLiteDialectProvider dialectProvider, string sele
21622180

21632181
public override string ToString()
21642182
{
2165-
return SelectExpression + " AS " + DialectProvider.GetQuotedName(Alias); // Alias is required for a non-column expression
2183+
var text = SelectExpression;
2184+
if (!string.IsNullOrEmpty(Alias)) // Note that even though Alias must be non-empty in the constructor it may be set to null/empty later
2185+
text += " AS " + DialectProvider.GetQuotedName(Alias);
2186+
return text;
21662187
}
21672188
}
21682189

‎src/ServiceStack.OrmLite/OrmLiteDialectProviderBase.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -558,11 +558,7 @@ public virtual string GetColumnNames(ModelDefinition modelDef)
558558

559559
public virtual SelectItem[] GetColumnNames(ModelDefinition modelDef, bool tableQualified)
560560
{
561-
var tablePrefix = "";
562-
if (tableQualified)
563-
{
564-
tablePrefix = GetQuotedTableName(modelDef);
565-
}
561+
var tablePrefix = tableQualified ? GetQuotedTableName(modelDef) : "";
566562

567563
var sqlColumns = new SelectItem[modelDef.FieldDefinitions.Count];
568564
for (var i = 0; i < sqlColumns.Length; ++i)

‎src/ServiceStack.OrmLite/OrmLiteUtils.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ public static string GetColumnNames(this ModelDefinition modelDef, IOrmLiteDiale
289289
return dialect.GetColumnNames(modelDef);
290290
}
291291

292-
public static string ToSelectString(this SelectItem[] items)
292+
public static string ToSelectString<TItem>(this IEnumerable<TItem> items)
293293
{
294294
var sb = StringBuilderCache.Allocate();
295295

‎tests/ServiceStack.OrmLite.Tests/ApiSqlServerTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,13 +165,13 @@ public void API_SqlServer_Examples()
165165
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Age FROM Person WHERE Age < @age"));
166166

167167
db.Lookup<int, string>(db.From<Person>().Select(x => new { x.Age, x.LastName }).Where(q => q.Age < 50));
168-
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Age\",\"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
168+
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Age\", \"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
169169

170170
db.Lookup<int, string>("SELECT Age, LastName FROM Person WHERE Age < @age", new { age = 50 });
171171
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Age, LastName FROM Person WHERE Age < @age"));
172172

173173
db.Dictionary<int, string>(db.From<Person>().Select(x => new { x.Id, x.LastName }).Where(x => x.Age < 50));
174-
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Id\",\"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
174+
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Id\", \"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
175175

176176
db.Dictionary<int, string>("SELECT Id, LastName FROM Person WHERE Age < @age", new { age = 50 });
177177
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Id, LastName FROM Person WHERE Age < @age"));

‎tests/ServiceStack.OrmLite.Tests/ApiSqliteTests.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,19 +163,19 @@ public void API_Sqlite_Examples()
163163
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Age FROM Person WHERE Age < @age"));
164164

165165
db.Lookup<int, string>(db.From<Person>().Select(x => new { x.Age, x.LastName }).Where(q => q.Age < 50));
166-
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Age\",\"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
166+
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Age\", \"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
167167

168168
db.Lookup<int, string>("SELECT Age, LastName FROM Person WHERE Age < @age", new { age = 50 });
169169
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Age, LastName FROM Person WHERE Age < @age"));
170170

171171
db.Dictionary<int, string>(db.From<Person>().Select(x => new { x.Id, x.LastName }).Where(x => x.Age < 50));
172-
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Id\",\"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
172+
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Id\", \"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
173173

174174
db.Dictionary<int, string>(db.From<Person>().Select(x => new { x.Id, x.LastName }).Where(x => x.Age < 50));
175-
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Id\",\"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
175+
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Id\", \"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
176176

177177
db.Dictionary<int, string>(db.From<Person>().Select(x => new { x.Id, x.LastName }).Where(x => x.Age < 50));
178-
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Id\",\"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
178+
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Id\", \"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
179179

180180
db.Dictionary<int, string>("SELECT Id, LastName FROM Person WHERE Age < @age", new { age = 50 });
181181
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Id, LastName FROM Person WHERE Age < @age"));

‎tests/ServiceStack.OrmLite.Tests/Expression/SqlExpressionTests.cs

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ public void Can_select_Dictionary_with_SqlExpression()
6969
using (var db = OpenDbConnection())
7070
{
7171
InitLetters(db);
72+
var expected = new Dictionary<string, int> {
73+
{ "A", 1 }, { "B", 2 }, { "C", 3 },
74+
};
7275

7376
var query = db.From<LetterFrequency>()
7477
.Select(x => new { x.Letter, count = Sql.Count("*") })
@@ -77,10 +80,28 @@ public void Can_select_Dictionary_with_SqlExpression()
7780

7881
query.ToSelectStatement().Print();
7982

80-
var map = new SortedDictionary<string, int>(db.Dictionary<string, int>(query));
81-
Assert.That(map.EquivalentTo(new Dictionary<string, int> {
82-
{ "A", 1 }, { "B", 2 }, { "C", 3 },
83-
}));
83+
var map = db.Dictionary<string, int>(query);
84+
Assert.That(map.EquivalentTo(expected));
85+
86+
// Same, but group by an anonymous type using an alias - this should not translate to "GROUP BY TheLetter AS Letter", which is invalid SQL
87+
88+
query = db.From<LetterFrequency>()
89+
.Select(x => new { x.Letter, count = Sql.Count("*") })
90+
.Where(q => q.Letter != "D")
91+
.GroupBy(x => new { TheLetter = x.Letter });
92+
93+
map = db.Dictionary<string, int>(query);
94+
Assert.That(map.EquivalentTo(expected));
95+
96+
// Now group by all columns without listing them - effectively "SELECT DISTINCT *"
97+
98+
query = db.From<LetterFrequency>()
99+
.Select(x => new { x.Id })
100+
.Where(q => q.Letter != "D")
101+
.GroupBy(x => new { x });
102+
103+
var list = db.SqlList<int>(query);
104+
Assert.That(list, Is.EquivalentTo(new[] { 1, 2, 3, 4, 5, 6 }));
84105
}
85106
}
86107

‎tests/ServiceStack.OrmLiteV45.Tests/ApiPostgreSqlTestsAsync.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ public async Task API_PostgreSql_Examples_Async()
148148
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT age FROM person WHERE age < :Age"));
149149

150150
await db.LookupAsync<int, string>(db.From<Person>().Select(x => new { x.Age, x.LastName }).Where(q => q.Age < 50));
151-
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"age\",\"last_name\" \nFROM \"person\"\nWHERE (\"age\" < :0)"));
151+
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"age\", \"last_name\" \nFROM \"person\"\nWHERE (\"age\" < :0)"));
152152

153153
await db.LookupAsync<int, string>("SELECT age, last_name FROM person WHERE age < :Age", new { age = 50 });
154154
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT age, last_name FROM person WHERE age < :Age"));
@@ -157,7 +157,7 @@ public async Task API_PostgreSql_Examples_Async()
157157
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT age, last_name FROM person WHERE age < :Age"));
158158

159159
await db.DictionaryAsync<int, string>(db.From<Person>().Select(x => new { x.Id, x.LastName }).Where(x => x.Age < 50));
160-
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"id\",\"last_name\" \nFROM \"person\"\nWHERE (\"age\" < :0)"));
160+
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"id\", \"last_name\" \nFROM \"person\"\nWHERE (\"age\" < :0)"));
161161

162162
await db.DictionaryAsync<int, string>("SELECT id, last_name FROM person WHERE age < :Age", new { age = 50 });
163163
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT id, last_name FROM person WHERE age < :Age"));

‎tests/ServiceStack.OrmLiteV45.Tests/ApiSqlServerTestsAsync.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ public async Task API_SqlServer_Examples_Async()
149149
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Age FROM Person WHERE Age < @age"));
150150

151151
await db.LookupAsync<int, string>(db.From<Person>().Select(x => new { x.Age, x.LastName }).Where(q => q.Age < 50));
152-
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Age\",\"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
152+
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Age\", \"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
153153

154154
await db.LookupAsync<int, string>("SELECT Age, LastName FROM Person WHERE Age < @age", new { age = 50 });
155155
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Age, LastName FROM Person WHERE Age < @age"));
@@ -158,7 +158,7 @@ public async Task API_SqlServer_Examples_Async()
158158
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Age, LastName FROM Person WHERE Age < @age"));
159159

160160
await db.DictionaryAsync<int, string>(db.From<Person>().Select(x => new { x.Id, x.LastName }).Where(x => x.Age < 50));
161-
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Id\",\"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
161+
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Id\", \"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
162162

163163
await db.DictionaryAsync<int, string>("SELECT Id, LastName FROM Person WHERE Age < @age", new { age = 50 });
164164
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Id, LastName FROM Person WHERE Age < @age"));

‎tests/ServiceStack.OrmLiteV45.Tests/ApiSqliteTestsAsync.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ public async Task API_Sqlite_Examples_Async()
148148
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Age FROM Person WHERE Age < @age"));
149149

150150
await db.LookupAsync<int, string>(db.From<Person>().Select(x => new { x.Age, x.LastName }).Where(q => q.Age < 50));
151-
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Age\",\"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
151+
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Age\", \"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
152152

153153
await db.LookupAsync<int, string>("SELECT Age, LastName FROM Person WHERE Age < @age", new { age = 50 });
154154
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Age, LastName FROM Person WHERE Age < @age"));
@@ -157,7 +157,7 @@ public async Task API_Sqlite_Examples_Async()
157157
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Age, LastName FROM Person WHERE Age < @age"));
158158

159159
await db.DictionaryAsync<int, string>(db.From<Person>().Select(x => new { x.Id, x.LastName }).Where(x => x.Age < 50));
160-
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Id\",\"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
160+
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT \"Id\", \"LastName\" \nFROM \"Person\"\nWHERE (\"Age\" < @0)"));
161161

162162
await db.DictionaryAsync<int, string>("SELECT Id, LastName FROM Person WHERE Age < @age", new { age = 50 });
163163
Assert.That(db.GetLastSql(), Is.EqualTo("SELECT Id, LastName FROM Person WHERE Age < @age"));

0 commit comments

Comments
 (0)