Skip to content

Commit

Permalink
Correct LTree type mapping logic
Browse files Browse the repository at this point in the history 
Fixes #2487

(cherry picked from commit ddc63fc)
  • Loading branch information
@roji
roji committed Aug 31, 2022
1 parent 6780f7e commit 9b2a90c
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 21 deletions.
48 changes: 31 additions & 17 deletions src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlLTreeTranslator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,32 +70,32 @@ public NpgsqlLTreeTranslator(
nameof(LTree.IsAncestorOf)
=> new PostgresBinaryExpression(
PostgresExpressionType.Contains,
_sqlExpressionFactory.ApplyTypeMapping(instance!, _ltreeTypeMapping),
_sqlExpressionFactory.ApplyTypeMapping(arguments[0], _ltreeTypeMapping),
ApplyTypeMappingOrConvert(instance!, _ltreeTypeMapping),
ApplyTypeMappingOrConvert(arguments[0], _ltreeTypeMapping),
typeof(bool),
_boolTypeMapping),

nameof(LTree.IsDescendantOf)
=> new PostgresBinaryExpression(
PostgresExpressionType.ContainedBy,
_sqlExpressionFactory.ApplyTypeMapping(instance!, _ltreeTypeMapping),
_sqlExpressionFactory.ApplyTypeMapping(arguments[0], _ltreeTypeMapping),
ApplyTypeMappingOrConvert(instance!, _ltreeTypeMapping),
ApplyTypeMappingOrConvert(arguments[0], _ltreeTypeMapping),
typeof(bool),
_boolTypeMapping),

nameof(LTree.MatchesLQuery)
=> new PostgresBinaryExpression(
PostgresExpressionType.LTreeMatches,
_sqlExpressionFactory.ApplyTypeMapping(instance!, _ltreeTypeMapping),
_sqlExpressionFactory.ApplyTypeMapping(arguments[0], _lqueryTypeMapping),
ApplyTypeMappingOrConvert(instance!, _ltreeTypeMapping),
ApplyTypeMappingOrConvert(arguments[0], _lqueryTypeMapping),
typeof(bool),
_boolTypeMapping),

nameof(LTree.MatchesLTxtQuery)
=> new PostgresBinaryExpression(
PostgresExpressionType.LTreeMatches,
_sqlExpressionFactory.ApplyTypeMapping(instance!, _ltreeTypeMapping),
_sqlExpressionFactory.ApplyTypeMapping(arguments[0], _ltxtqueryTypeMapping),
ApplyTypeMappingOrConvert(instance!, _ltreeTypeMapping),
ApplyTypeMappingOrConvert(arguments[0], _ltxtqueryTypeMapping),
typeof(bool),
_boolTypeMapping),

Expand Down Expand Up @@ -185,7 +185,7 @@ arguments[1] is LambdaExpression wherePredicate &&
{
return new PostgresBinaryExpression(
PostgresExpressionType.LTreeMatchesAny,
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateInstance), _ltreeTypeMapping),
ApplyTypeMappingOrConvert(Visit(predicateInstance), _ltreeTypeMapping),
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _lqueryArrayTypeMapping),
typeof(bool),
_boolTypeMapping);
Expand All @@ -198,7 +198,7 @@ arguments[1] is LambdaExpression wherePredicate &&
return new PostgresBinaryExpression(
PostgresExpressionType.Contains,
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _ltreeTypeMapping),
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _ltreeTypeMapping),
typeof(bool),
_boolTypeMapping);
}
Expand All @@ -210,7 +210,7 @@ arguments[1] is LambdaExpression wherePredicate &&
return new PostgresBinaryExpression(
PostgresExpressionType.ContainedBy,
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _ltreeTypeMapping),
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _ltreeTypeMapping),
typeof(bool),
_boolTypeMapping);
}
Expand All @@ -222,7 +222,7 @@ arguments[1] is LambdaExpression wherePredicate &&
return new PostgresBinaryExpression(
PostgresExpressionType.LTreeMatches,
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _lqueryTypeMapping),
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _lqueryTypeMapping),
typeof(bool),
_boolTypeMapping);
}
Expand All @@ -234,7 +234,7 @@ arguments[1] is LambdaExpression wherePredicate &&
return new PostgresBinaryExpression(
PostgresExpressionType.LTreeMatches,
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _ltxtqueryTypeMapping),
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _ltxtqueryTypeMapping),
typeof(bool),
_boolTypeMapping);
}
Expand Down Expand Up @@ -281,7 +281,7 @@ arguments[1] is LambdaExpression wherePredicate &&
return new PostgresBinaryExpression(
PostgresExpressionType.LTreeFirstAncestor,
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _ltreeTypeMapping),
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _ltreeTypeMapping),
typeof(LTree),
_ltreeTypeMapping);
}
Expand All @@ -293,7 +293,7 @@ arguments[1] is LambdaExpression wherePredicate &&
return new PostgresBinaryExpression(
PostgresExpressionType.LTreeFirstDescendent,
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _ltreeTypeMapping),
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _ltreeTypeMapping),
typeof(LTree),
_ltreeTypeMapping);
}
Expand All @@ -305,7 +305,7 @@ arguments[1] is LambdaExpression wherePredicate &&
return new PostgresBinaryExpression(
PostgresExpressionType.LTreeFirstMatches,
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _lqueryTypeMapping),
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _lqueryTypeMapping),
typeof(LTree),
_ltreeTypeMapping);
}
Expand All @@ -317,7 +317,7 @@ arguments[1] is LambdaExpression wherePredicate &&
return new PostgresBinaryExpression(
PostgresExpressionType.LTreeFirstMatches,
_sqlExpressionFactory.ApplyTypeMapping(Visit(array), _ltreeArrayTypeMapping),
_sqlExpressionFactory.ApplyTypeMapping(Visit(predicateArguments[0]), _ltxtqueryTypeMapping),
ApplyTypeMappingOrConvert(Visit(predicateArguments[0]), _ltxtqueryTypeMapping),
typeof(string),
_ltreeTypeMapping);
}
Expand All @@ -329,4 +329,18 @@ arguments[1] is LambdaExpression wherePredicate &&
SqlExpression Visit(Expression expression)
=> (SqlExpression)sqlTranslatingExpressionVisitor.Visit(expression);
}

// Applying e.g. the LQuery type mapping on a function operator is a bit tricky.
// If it's a constant, we can just apply the mapping: the constant will get rendered as an untyped string literal, and PG will
// coerce it as the function parameter.
// If it's a parameter, we can also just apply the mapping (which causes NpgsqlDbType to be set to LQuery).
// For anything else, we may need an explicit cast to LQuery, e.g. a plain text column or a concatenation between strings;
// apply the default type mapping and then apply an additional Convert node if the resulting mapping isn't what we need.
private SqlExpression ApplyTypeMappingOrConvert(SqlExpression sqlExpression, RelationalTypeMapping typeMapping)
=> sqlExpression is SqlConstantExpression or SqlParameterExpression
? _sqlExpressionFactory.ApplyTypeMapping(sqlExpression, typeMapping)
: _sqlExpressionFactory.ApplyDefaultTypeMapping(sqlExpression) is var expressionWithDefaultTypeMapping
&& expressionWithDefaultTypeMapping.TypeMapping!.StoreType == typeMapping.StoreType
? expressionWithDefaultTypeMapping
: _sqlExpressionFactory.Convert(expressionWithDefaultTypeMapping, typeMapping.ClrType, typeMapping);
}
39 changes: 35 additions & 4 deletions test/EFCore.PG.FunctionalTests/Query/LTreeQueryTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,39 @@ public void LTree_matches_LQuery()

Assert.Equal(4, entity.Id);
AssertSql(
@"SELECT l.""Id"", l.""Path"", l.""PathAsString""
@"SELECT l.""Id"", l.""Path"", l.""PathAsString"", l.""SomeString""
FROM ""LTreeEntities"" AS l
WHERE l.""Path"" ~ '*.Astrophysics'
LIMIT 2");
}

[ConditionalFact] // #2487
public void LTree_matches_LQuery_with_string_column()
{
using var ctx = CreateContext();
var entity = ctx.LTreeEntities.Single(l => l.Path.MatchesLQuery(l.SomeString));

Assert.Equal(4, entity.Id);
AssertSql(
@"SELECT l.""Id"", l.""Path"", l.""PathAsString"", l.""SomeString""
FROM ""LTreeEntities"" AS l
WHERE l.""Path"" ~ l.""SomeString""::lquery
LIMIT 2");
}

[ConditionalFact] // #2487
public void LTree_matches_LQuery_with_concat()
{
using var ctx = CreateContext();
var count = ctx.LTreeEntities.Count(l => l.Path.MatchesLQuery("*.Astrophysics." + l.Id));

Assert.Equal(0, count);
AssertSql(
@"SELECT COUNT(*)::INT
FROM ""LTreeEntities"" AS l
WHERE l.""Path"" ~ CAST(('*.Astrophysics.' || l.""Id""::text) AS lquery)");
}

[ConditionalFact]
public void LTree_matches_any_LQuery()
{
Expand All @@ -137,7 +164,7 @@ public void LTree_matches_any_LQuery()
AssertSql(
@"@__lqueries_0={ '*.Astrophysics', '*.Geology' } (DbType = Object)
SELECT l.""Id"", l.""Path"", l.""PathAsString""
SELECT l.""Id"", l.""Path"", l.""PathAsString"", l.""SomeString""
FROM ""LTreeEntities"" AS l
WHERE l.""Path"" ? @__lqueries_0
LIMIT 2");
Expand All @@ -164,7 +191,7 @@ public void LTree_concat()

Assert.Equal(2, entity.Id);
AssertSql(
@"SELECT l.""Id"", l.""Path"", l.""PathAsString""
@"SELECT l.""Id"", l.""Path"", l.""PathAsString"", l.""SomeString""
FROM ""LTreeEntities"" AS l
WHERE (l.""Path""::text || '.Astronomy') = 'Top.Science.Astronomy'
LIMIT 2");
Expand Down Expand Up @@ -357,7 +384,7 @@ public void Subpath2()

Assert.Equal(4, result.Id);
AssertSql(
@"SELECT l.""Id"", l.""Path"", l.""PathAsString""
@"SELECT l.""Id"", l.""Path"", l.""PathAsString"", l.""SomeString""
FROM ""LTreeEntities"" AS l
WHERE (nlevel(l.""Path"") > 2) AND (subpath(l.""Path"", 2) = 'Astronomy.Astrophysics')
LIMIT 2");
Expand Down Expand Up @@ -451,6 +478,7 @@ public static void Seed(LTreeQueryContext context)
foreach (var ltreeEntity in ltreeEntities)
{
ltreeEntity.PathAsString = ltreeEntity.Path;
ltreeEntity.SomeString = "*.Astrophysics";
}

context.LTreeEntities.AddRange(ltreeEntities);
Expand All @@ -468,6 +496,9 @@ public class LTreeEntity
[Required]
[Column(TypeName = "ltree")]
public string PathAsString { get; set; }

[Required]
public string SomeString { get; set; }
}

public class LTreeQueryFixture : SharedStoreFixtureBase<LTreeQueryContext>
Expand Down

0 comments on commit 9b2a90c

Please sign in to comment.