Skip to content

Commit ddc63fc

Browse files
rojiroji
committed
Correct LTree type mapping logic
Fixes #2487
1 parent 07400f7 commit ddc63fc

2 files changed

Lines changed: 70 additions & 21 deletions

File tree

‎src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlLTreeTranslator.cs‎

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -57,32 +57,32 @@ public NpgsqlLTreeTranslator(
5757
nameof(LTree.IsAncestorOf)
5858
=> new PostgresBinaryExpression(
5959
PostgresExpressionType.Contains,
60-
_sqlExpressionFactory.ApplyTypeMapping(instance!, _ltreeTypeMapping),
61-
_sqlExpressionFactory.ApplyTypeMapping(arguments[0], _ltreeTypeMapping),
60+
ApplyTypeMappingOrConvert(instance!, _ltreeTypeMapping),
61+
ApplyTypeMappingOrConvert(arguments[0], _ltreeTypeMapping),
6262
typeof(bool),
6363
_boolTypeMapping),
6464

6565
nameof(LTree.IsDescendantOf)
6666
=> new PostgresBinaryExpression(
6767
PostgresExpressionType.ContainedBy,
68-
_sqlExpressionFactory.ApplyTypeMapping(instance!, _ltreeTypeMapping),
69-
_sqlExpressionFactory.ApplyTypeMapping(arguments[0], _ltreeTypeMapping),
68+
ApplyTypeMappingOrConvert(instance!, _ltreeTypeMapping),
69+
ApplyTypeMappingOrConvert(arguments[0], _ltreeTypeMapping),
7070
typeof(bool),
7171
_boolTypeMapping),
7272

7373
nameof(LTree.MatchesLQuery)
7474
=> new PostgresBinaryExpression(
7575
PostgresExpressionType.LTreeMatches,
76-
_sqlExpressionFactory.ApplyTypeMapping(instance!, _ltreeTypeMapping),
77-
_sqlExpressionFactory.ApplyTypeMapping(arguments[0], _lqueryTypeMapping),
76+
ApplyTypeMappingOrConvert(instance!, _ltreeTypeMapping),
77+
ApplyTypeMappingOrConvert(arguments[0], _lqueryTypeMapping),
7878
typeof(bool),
7979
_boolTypeMapping),
8080

8181
nameof(LTree.MatchesLTxtQuery)
8282
=> new PostgresBinaryExpression(
8383
PostgresExpressionType.LTreeMatches,
84-
_sqlExpressionFactory.ApplyTypeMapping(instance!, _ltreeTypeMapping),
85-
_sqlExpressionFactory.ApplyTypeMapping(arguments[0], _ltxtqueryTypeMapping),
84+
ApplyTypeMappingOrConvert(instance!, _ltreeTypeMapping),
85+
ApplyTypeMappingOrConvert(arguments[0], _ltxtqueryTypeMapping),
8686
typeof(bool),
8787
_boolTypeMapping),
8888

@@ -171,7 +171,7 @@ arguments[1] is LambdaExpression wherePredicate &&
171171
{
172172
return new PostgresBinaryExpression(
173173
PostgresExpressionType.LTreeMatchesAny,
174-
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateInstance), _ltreeTypeMapping),
174+
ApplyTypeMappingOrConvert(Visit(predicateInstance), _ltreeTypeMapping),
175175
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _lqueryArrayTypeMapping),
176176
typeof(bool),
177177
_boolTypeMapping);
@@ -184,7 +184,7 @@ arguments[1] is LambdaExpression wherePredicate &&
184184
return new PostgresBinaryExpression(
185185
PostgresExpressionType.Contains,
186186
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
187-
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _ltreeTypeMapping),
187+
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _ltreeTypeMapping),
188188
typeof(bool),
189189
_boolTypeMapping);
190190
}
@@ -196,7 +196,7 @@ arguments[1] is LambdaExpression wherePredicate &&
196196
return new PostgresBinaryExpression(
197197
PostgresExpressionType.ContainedBy,
198198
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
199-
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _ltreeTypeMapping),
199+
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _ltreeTypeMapping),
200200
typeof(bool),
201201
_boolTypeMapping);
202202
}
@@ -208,7 +208,7 @@ arguments[1] is LambdaExpression wherePredicate &&
208208
return new PostgresBinaryExpression(
209209
PostgresExpressionType.LTreeMatches,
210210
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
211-
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _lqueryTypeMapping),
211+
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _lqueryTypeMapping),
212212
typeof(bool),
213213
_boolTypeMapping);
214214
}
@@ -220,7 +220,7 @@ arguments[1] is LambdaExpression wherePredicate &&
220220
return new PostgresBinaryExpression(
221221
PostgresExpressionType.LTreeMatches,
222222
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
223-
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _ltxtqueryTypeMapping),
223+
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _ltxtqueryTypeMapping),
224224
typeof(bool),
225225
_boolTypeMapping);
226226
}
@@ -267,7 +267,7 @@ arguments[1] is LambdaExpression wherePredicate &&
267267
return new PostgresBinaryExpression(
268268
PostgresExpressionType.LTreeFirstAncestor,
269269
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
270-
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _ltreeTypeMapping),
270+
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _ltreeTypeMapping),
271271
typeof(LTree),
272272
_ltreeTypeMapping);
273273
}
@@ -279,7 +279,7 @@ arguments[1] is LambdaExpression wherePredicate &&
279279
return new PostgresBinaryExpression(
280280
PostgresExpressionType.LTreeFirstDescendent,
281281
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
282-
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _ltreeTypeMapping),
282+
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _ltreeTypeMapping),
283283
typeof(LTree),
284284
_ltreeTypeMapping);
285285
}
@@ -291,7 +291,7 @@ arguments[1] is LambdaExpression wherePredicate &&
291291
return new PostgresBinaryExpression(
292292
PostgresExpressionType.LTreeFirstMatches,
293293
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
294-
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _lqueryTypeMapping),
294+
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _lqueryTypeMapping),
295295
typeof(LTree),
296296
_ltreeTypeMapping);
297297
}
@@ -303,7 +303,7 @@ arguments[1] is LambdaExpression wherePredicate &&
303303
return new PostgresBinaryExpression(
304304
PostgresExpressionType.LTreeFirstMatches,
305305
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
306-
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _ltxtqueryTypeMapping),
306+
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _ltxtqueryTypeMapping),
307307
typeof(string),
308308
_ltreeTypeMapping);
309309
}
@@ -315,4 +315,18 @@ arguments[1] is LambdaExpression wherePredicate &&
315315
SqlExpression Visit(Expression expression)
316316
=> (SqlExpression)sqlTranslatingExpressionVisitor.Visit(expression);
317317
}
318+
319+
// Applying e.g. the LQuery type mapping on a function operator is a bit tricky.
320+
// If it's a constant, we can just apply the mapping: the constant will get rendered as an untyped string literal, and PG will
321+
// coerce it as the function parameter.
322+
// If it's a parameter, we can also just apply the mapping (which causes NpgsqlDbType to be set to LQuery).
323+
// For anything else, we may need an explicit cast to LQuery, e.g. a plain text column or a concatenation between strings;
324+
// apply the default type mapping and then apply an additional Convert node if the resulting mapping isn't what we need.
325+
private SqlExpression ApplyTypeMappingOrConvert(SqlExpression sqlExpression, RelationalTypeMapping typeMapping)
326+
=> sqlExpression is SqlConstantExpression or SqlParameterExpression
327+
? _sqlExpressionFactory.ApplyTypeMapping(sqlExpression, typeMapping)
328+
: _sqlExpressionFactory.ApplyDefaultTypeMapping(sqlExpression) is var expressionWithDefaultTypeMapping
329+
&& expressionWithDefaultTypeMapping.TypeMapping!.StoreType == typeMapping.StoreType
330+
? expressionWithDefaultTypeMapping
331+
: _sqlExpressionFactory.Convert(expressionWithDefaultTypeMapping, typeMapping.ClrType, typeMapping);
318332
}

‎test/EFCore.PG.FunctionalTests/Query/LTreeQueryTest.cs‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,44 @@ public void LTree_matches_LQuery()
128128
Assert.Equal(4, entity.Id);
129129
AssertSql(
130130
"""
131-
SELECT l."Id", l."Path", l."PathAsString"
131+
SELECT l."Id", l."Path", l."PathAsString", l."SomeString"
132132
FROM "LTreeEntities" AS l
133133
WHERE l."Path" ~ '*.Astrophysics'
134134
LIMIT 2
135135
""");
136136
}
137137

138+
[ConditionalFact] // #2487
139+
public void LTree_matches_LQuery_with_string_column()
140+
{
141+
using var ctx = CreateContext();
142+
var entity = ctx.LTreeEntities.Single(l => l.Path.MatchesLQuery(l.SomeString));
143+
144+
Assert.Equal(4, entity.Id);
145+
AssertSql(
146+
"""
147+
SELECT l."Id", l."Path", l."PathAsString", l."SomeString"
148+
FROM "LTreeEntities" AS l
149+
WHERE l."Path" ~ l."SomeString"::lquery
150+
LIMIT 2
151+
""");
152+
}
153+
154+
[ConditionalFact] // #2487
155+
public void LTree_matches_LQuery_with_concat()
156+
{
157+
using var ctx = CreateContext();
158+
var count = ctx.LTreeEntities.Count(l => l.Path.MatchesLQuery("*.Astrophysics." + l.Id));
159+
160+
Assert.Equal(0, count);
161+
AssertSql(
162+
"""
163+
SELECT count(*)::int
164+
FROM "LTreeEntities" AS l
165+
WHERE l."Path" ~ CAST(('*.Astrophysics.' || l."Id"::text) AS lquery)
166+
""");
167+
}
168+
138169
[ConditionalFact]
139170
public void LTree_matches_any_LQuery()
140171
{
@@ -147,7 +178,7 @@ public void LTree_matches_any_LQuery()
147178
"""
148179
@__lqueries_0={ '*.Astrophysics', '*.Geology' } (DbType = Object)
149180
150-
SELECT l."Id", l."Path", l."PathAsString"
181+
SELECT l."Id", l."Path", l."PathAsString", l."SomeString"
151182
FROM "LTreeEntities" AS l
152183
WHERE l."Path" ? @__lqueries_0
153184
LIMIT 2
@@ -178,7 +209,7 @@ public void LTree_concat()
178209
Assert.Equal(2, entity.Id);
179210
AssertSql(
180211
"""
181-
SELECT l."Id", l."Path", l."PathAsString"
212+
SELECT l."Id", l."Path", l."PathAsString", l."SomeString"
182213
FROM "LTreeEntities" AS l
183214
WHERE (l."Path"::text || '.Astronomy') = 'Top.Science.Astronomy'
184215
LIMIT 2
@@ -395,7 +426,7 @@ public void Subpath2()
395426
Assert.Equal(4, result.Id);
396427
AssertSql(
397428
"""
398-
SELECT l."Id", l."Path", l."PathAsString"
429+
SELECT l."Id", l."Path", l."PathAsString", l."SomeString"
399430
FROM "LTreeEntities" AS l
400431
WHERE nlevel(l."Path") > 2 AND subpath(l."Path", 2) = 'Astronomy.Astrophysics'
401432
LIMIT 2
@@ -495,6 +526,7 @@ public static void Seed(LTreeQueryContext context)
495526
foreach (var ltreeEntity in ltreeEntities)
496527
{
497528
ltreeEntity.PathAsString = ltreeEntity.Path;
529+
ltreeEntity.SomeString = "*.Astrophysics";
498530
}
499531

500532
context.LTreeEntities.AddRange(ltreeEntities);
@@ -512,6 +544,9 @@ public class LTreeEntity
512544
[Required]
513545
[Column(TypeName = "ltree")]
514546
public string PathAsString { get; set; }
547+
548+
[Required]
549+
public string SomeString { get; set; }
515550
}
516551

517552
public class LTreeQueryFixture : SharedStoreFixtureBase<LTreeQueryContext>

0 commit comments

Comments
 (0)