Skip to content

Commit

Permalink
Use -> operator for SQLite JsonScalarExpression
Browse files Browse the repository at this point in the history 
Closes dotnet#30334
  • Loading branch information
@roji
roji committed Apr 25, 2023
1 parent 6f77314 commit df4b06c
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 43 deletions.
18 changes: 9 additions & 9 deletions src/EFCore.Relational/Query/QuerySqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -592,32 +592,32 @@ protected virtual void CheckComposableSqlTrimmed(ReadOnlySpan<char> sql)
/// <inheritdoc />
protected override Expression VisitSqlBinary(SqlBinaryExpression sqlBinaryExpression)
{
var requiresBrackets = RequiresParentheses(sqlBinaryExpression, sqlBinaryExpression.Left);
var requiresParentheses = RequiresParentheses(sqlBinaryExpression, sqlBinaryExpression.Left);

if (requiresBrackets)
if (requiresParentheses)
{
_relationalCommandBuilder.Append("(");
}

Visit(sqlBinaryExpression.Left);

if (requiresBrackets)
if (requiresParentheses)
{
_relationalCommandBuilder.Append(")");
}

_relationalCommandBuilder.Append(GetOperator(sqlBinaryExpression));

requiresBrackets = RequiresParentheses(sqlBinaryExpression, sqlBinaryExpression.Right);
requiresParentheses = RequiresParentheses(sqlBinaryExpression, sqlBinaryExpression.Right);

if (requiresBrackets)
if (requiresParentheses)
{
_relationalCommandBuilder.Append("(");
}

Visit(sqlBinaryExpression.Right);

if (requiresBrackets)
if (requiresParentheses)
{
_relationalCommandBuilder.Append(")");
}
Expand Down Expand Up @@ -800,14 +800,14 @@ protected override Expression VisitSqlUnary(SqlUnaryExpression sqlUnaryExpressio
case ExpressionType.Convert:
{
_relationalCommandBuilder.Append("CAST(");
var requiresBrackets = RequiresParentheses(sqlUnaryExpression, sqlUnaryExpression.Operand);
if (requiresBrackets)
var requiresParentheses = RequiresParentheses(sqlUnaryExpression, sqlUnaryExpression.Operand);
if (requiresParentheses)
{
_relationalCommandBuilder.Append("(");
}

Visit(sqlUnaryExpression.Operand);
if (requiresBrackets)
if (requiresParentheses)
{
_relationalCommandBuilder.Append(")");
}
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Query/SqlExpressions/JsonScalarExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ internal JsonScalarExpression(
}

/// <summary>
/// The column containg JSON value.
/// The column containing the JSON value.
/// </summary>
public virtual ColumnExpression JsonColumn { get; }

Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1611,7 +1611,7 @@ static Dictionary<JsonQueryExpression, JsonScalarExpression> BuildJsonProjection
{
// force reference comparison for this one, even if we implement custom equality for JsonQueryExpression in the future
var deduplicationMap = new Dictionary<JsonQueryExpression, JsonScalarExpression>(ReferenceEqualityComparer.Instance);
if (projections.Count() > 0)
if (projections.Any())
{
var ordered = projections
.OrderBy(x => $"{x.JsonColumn.TableAlias}.{x.JsonColumn.Name}")
Expand Down
109 changes: 80 additions & 29 deletions src/EFCore.Sqlite.Core/Query/Internal/SqliteQuerySqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,56 +126,106 @@ private Expression VisitRegexp(RegexpExpression regexpExpression)
/// </summary>
protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExpression)
{
if (jsonScalarExpression.Path.Count == 1
&& jsonScalarExpression.Path[0].ToString() == "$")
// TODO: This trims the leading $ PathSegment, which isn't actually part of the path (but rather path of the JSONPATH language
// used to generate the path in SQL for *some* databases).
// Instead, we should not be producing JsonScalarExpression at all with the leading $.
var path = jsonScalarExpression.Path is [{ PropertyName: "$" }, ..]
? jsonScalarExpression.Path.Skip(1).ToArray()
: jsonScalarExpression.Path;

if (path.Count == 0)
{
Visit(jsonScalarExpression.JsonColumn);

return jsonScalarExpression;
}

Sql.Append("json_extract(");

Visit(jsonScalarExpression.JsonColumn);

Sql.Append(", '");
foreach (var pathSegment in jsonScalarExpression.Path)
var inJsonpathString = false;

for (var i = 0; i < path.Count; i++)
{
if (pathSegment.PropertyName != null)
{
Sql.Append((pathSegment.PropertyName == "$" ? "" : ".") + pathSegment.PropertyName);
}
var pathSegment = path[i];
var isLast = i == path.Count - 1;

if (pathSegment.ArrayIndex != null)
switch (pathSegment)
{
Sql.Append("[");
case { PropertyName: string propertyName }:
if (inJsonpathString)
{
Sql.Append(".").Append(propertyName);
continue;
}

if (pathSegment.ArrayIndex is SqlConstantExpression)
{
Visit(pathSegment.ArrayIndex);
}
else
{
Sql.Append("' || ");
if (pathSegment.ArrayIndex is SqlParameterExpression)
Sql.Append(" ->> ");

// No need to start a $. JSONPATH string if we're the last segment or the next segment isn't a constant
if (isLast || path[i + 1] is { ArrayIndex: not null and not SqlConstantExpression })
{
Sql.Append("'").Append(propertyName).Append("'");
continue;
}

Sql.Append("'$.").Append(propertyName);
inJsonpathString = true;
continue;

case { ArrayIndex: SqlConstantExpression arrayIndex }:
if (inJsonpathString)
{
Sql.Append("[");
Visit(pathSegment.ArrayIndex);
Sql.Append("]");
continue;
}
else

Sql.Append(" ->> ");

// No need to start a $. JSONPATH string if we're the last segment or the next segment isn't a constant
if (isLast || path[i + 1] is { ArrayIndex: not null and not SqlConstantExpression })
{
Visit(arrayIndex);
continue;
}

Sql.Append("'$[");
Visit(arrayIndex);
Sql.Append("]");
inJsonpathString = true;
continue;

default:
if (inJsonpathString)
{
Sql.Append("'");
inJsonpathString = false;
}

Sql.Append(" ->> ");

Check.DebugAssert(pathSegment.ArrayIndex is not null, "pathSegment.ArrayIndex is not null");

var requiresParentheses = RequiresParentheses(jsonScalarExpression, pathSegment.ArrayIndex);
if (requiresParentheses)
{
Sql.Append("(");
Visit(pathSegment.ArrayIndex);
Sql.Append(")");
}

Sql.Append(" || '");
}
Visit(pathSegment.ArrayIndex);

if (requiresParentheses)
{
Sql.Append(")");
}

Sql.Append("]");
continue;
}
}

Sql.Append("')");
if (inJsonpathString)
{
Sql.Append("'");
}

return jsonScalarExpression;
}
Expand All @@ -196,7 +246,7 @@ protected override bool TryGetOperatorInfo(SqlExpression expression, out int pre
ExpressionType.Multiply => (900, true),
ExpressionType.Divide => (900, false),
ExpressionType.Modulo => (900, false),
ExpressionType.Add when sqlBinaryExpression.Type == typeof(string) => (1100, true),
ExpressionType.Add when sqlBinaryExpression.Type == typeof(string) => (1000, true),
ExpressionType.Add when sqlBinaryExpression.Type != typeof(string) => (800, true),
ExpressionType.Subtract => (800, false),
ExpressionType.And => (600, true),
Expand Down Expand Up @@ -227,6 +277,7 @@ protected override bool TryGetOperatorInfo(SqlExpression expression, out int pre

CollateExpression => (1100, false),
LikeExpression => (500, false),
JsonScalarExpression => (1000, true),

_ => default,
};
Expand Down
43 changes: 40 additions & 3 deletions test/EFCore.Sqlite.FunctionalTests/Query/JsonQuerySqliteTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,29 @@ public JsonQuerySqliteTest(JsonQuerySqliteFixture fixture, ITestOutputHelper tes
//Fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
}

public override async Task Json_scalar_length(bool async)
{
await base.Json_scalar_length(async);

AssertSql(
"""
SELECT "j"."Name"
FROM "JsonEntitiesBasic" AS "j"
WHERE length("j"."OwnedReferenceRoot" ->> 'Name') > 2
""");
}

public override async Task Basic_json_projection_enum_inside_json_entity(bool async)
{
await base.Basic_json_projection_enum_inside_json_entity(async);

AssertSql(
"""
SELECT "j"."Id", "j"."OwnedReferenceRoot" ->> '$.OwnedReferenceBranch.Enum' AS "Enum"
FROM "JsonEntitiesBasic" AS "j"
""");
}

public override async Task Project_json_entity_FirstOrDefault_subquery(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
Expand Down Expand Up @@ -67,6 +90,20 @@ await AssertQuery(
""");
}

public override async Task Json_collection_element_access_in_predicate_nested_mix(bool async)
{
await base.Json_collection_element_access_in_predicate_nested_mix(async);

AssertSql(
"""
@__prm_0='0'

SELECT "j"."Id", "j"."EntityBasicId", "j"."Name", "j"."OwnedCollectionRoot", "j"."OwnedReferenceRoot"
FROM "JsonEntitiesBasic" AS "j"
WHERE "j"."OwnedCollectionRoot" ->> '$[1].OwnedCollectionBranch' ->> @__prm_0 ->> 'OwnedCollectionLeaf' ->> ("j"."Id" - 1) ->> 'SomethingSomething' = 'e1_c2_c1_c1'
""");
}

public override async Task Json_predicate_on_bool_converted_to_int_zero_one(bool async)
{
await base.Json_predicate_on_bool_converted_to_int_zero_one(async);
Expand All @@ -75,7 +112,7 @@ public override async Task Json_predicate_on_bool_converted_to_int_zero_one(bool
"""
SELECT "j"."Id", "j"."Reference"
FROM "JsonEntitiesConverters" AS "j"
WHERE json_extract("j"."Reference", '$.BoolConvertedToIntZeroOne') = 1
WHERE "j"."Reference" ->> 'BoolConvertedToIntZeroOne' = 1
""");
}

Expand All @@ -87,7 +124,7 @@ public override async Task Json_predicate_on_bool_converted_to_string_True_False
"""
SELECT "j"."Id", "j"."Reference"
FROM "JsonEntitiesConverters" AS "j"
WHERE json_extract("j"."Reference", '$.BoolConvertedToStringTrueFalse') = 'True'
WHERE "j"."Reference" ->> 'BoolConvertedToStringTrueFalse' = 'True'
""");
}

Expand All @@ -99,7 +136,7 @@ public override async Task Json_predicate_on_bool_converted_to_string_Y_N(bool a
"""
SELECT "j"."Id", "j"."Reference"
FROM "JsonEntitiesConverters" AS "j"
WHERE json_extract("j"."Reference", '$.BoolConvertedToStringYN') = 'Y'
WHERE "j"."Reference" ->> 'BoolConvertedToStringYN' = 'Y'
""");
}

Expand Down

0 comments on commit df4b06c

Please sign in to comment.