Skip to content

Commit

Permalink
Fix to #19178 - Invalid SQL with CROSS APPLY generated on SQLite
Browse files Browse the repository at this point in the history
Adding logic that checks for APPLY and throws meaningful exception

Fixes #19178
  • Loading branch information
maumar committed Jul 10, 2020
1 parent 0a0a455 commit d5112aa
Show file tree
Hide file tree
Showing 18 changed files with 343 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Linq.Expressions;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
using Microsoft.EntityFrameworkCore.Utilities;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public static IServiceCollection AddEntityFrameworkSqlite([NotNull] this IServic
.TryAdd<IQuerySqlGeneratorFactory, SqliteQuerySqlGeneratorFactory>()
.TryAdd<IQueryableMethodTranslatingExpressionVisitorFactory, SqliteQueryableMethodTranslatingExpressionVisitorFactory>()
.TryAdd<IRelationalSqlTranslatingExpressionVisitorFactory, SqliteSqlTranslatingExpressionVisitorFactory>()
.TryAdd<IQueryTranslationPostprocessorFactory, SqliteQueryTranslationPostprocessorFactory>()
.TryAddProviderSpecificServices(
b => b.TryAddScoped<ISqliteRelationalConnection, SqliteRelationalConnection>());

Expand Down
6 changes: 6 additions & 0 deletions src/EFCore.Sqlite.Core/Properties/SqliteStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/EFCore.Sqlite.Core/Properties/SqliteStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -187,4 +187,7 @@
<data name="AggregateOperationNotSupported" xml:space="preserve">
<value>SQLite cannot apply aggregate operator '{aggregateOperator}' on expression of type '{type}'. Convert the values to a supported type or use LINQ to Objects to aggregate the results.</value>
</data>
<data name="ApplyNotSupported" xml:space="preserve">
<value>Translating this query requires APPLY operation which is not supported on SQLite.</value>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Linq;
using System.Linq.Expressions;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
using Microsoft.EntityFrameworkCore.Sqlite.Internal;

namespace Microsoft.EntityFrameworkCore.Sqlite.Query.Internal
{
/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public class SqliteQueryTranslationPostprocessor : RelationalQueryTranslationPostprocessor
{
/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public SqliteQueryTranslationPostprocessor(
QueryTranslationPostprocessorDependencies dependencies,
RelationalQueryTranslationPostprocessorDependencies relationalDependencies,
QueryCompilationContext queryCompilationContext)
: base(dependencies, relationalDependencies, queryCompilationContext)
{
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public override Expression Process(Expression query)
{
var result = base.Process(query);

if (result is ShapedQueryExpression shapedQueryExpression)
{
var applyValidator = new ApplyValidatingVisitor();
applyValidator.Visit(shapedQueryExpression.QueryExpression);
if (!applyValidator.ApplyFound)
{
applyValidator.Visit(shapedQueryExpression.ShaperExpression);
}

if (applyValidator.ApplyFound)
{
throw new InvalidOperationException(SqliteStrings.ApplyNotSupported);
}
}

return result;
}

private class ApplyValidatingVisitor : ExpressionVisitor
{
public bool ApplyFound { get; private set; } = false;

protected override Expression VisitExtension(Expression extensionExpression)
{
if (extensionExpression is SelectExpression selectExpression
&& selectExpression.Tables.Any(t => t is CrossApplyExpression || t is OuterApplyExpression))
{
ApplyFound = true;

return extensionExpression;
}

return base.VisitExtension(extensionExpression);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Utilities;

namespace Microsoft.EntityFrameworkCore.Sqlite.Query.Internal
{
/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public class SqliteQueryTranslationPostprocessorFactory : IQueryTranslationPostprocessorFactory
{
private readonly QueryTranslationPostprocessorDependencies _dependencies;
private readonly RelationalQueryTranslationPostprocessorDependencies _relationalDependencies;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public SqliteQueryTranslationPostprocessorFactory(
[NotNull] QueryTranslationPostprocessorDependencies dependencies,
[NotNull] RelationalQueryTranslationPostprocessorDependencies relationalDependencies)
{
_dependencies = dependencies;
_relationalDependencies = relationalDependencies;
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual QueryTranslationPostprocessor Create([NotNull] QueryCompilationContext queryCompilationContext)
{
Check.NotNull(queryCompilationContext, nameof(queryCompilationContext));

return new SqliteQueryTranslationPostprocessor(
_dependencies,
_relationalDependencies,
queryCompilationContext);
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ public SqliteQueryableMethodTranslatingExpressionVisitorFactory(
[NotNull] QueryableMethodTranslatingExpressionVisitorDependencies dependencies,
[NotNull] RelationalQueryableMethodTranslatingExpressionVisitorDependencies relationalDependencies)
{
Check.NotNull(dependencies, nameof(dependencies));
Check.NotNull(relationalDependencies, nameof(relationalDependencies));

_dependencies = dependencies;
_relationalDependencies = relationalDependencies;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.Sqlite.Internal;
using Xunit;

namespace Microsoft.EntityFrameworkCore.Query
Expand All @@ -16,9 +18,17 @@ public ManyToManyNoTrackingQuerySqliteTest(ManyToManyQuerySqliteFixture fixture)

// Sqlite does not support Apply operations

public override Task Skip_navigation_order_by_single_or_default(bool async) => Task.CompletedTask;
public override async Task Skip_navigation_order_by_single_or_default(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Skip_navigation_order_by_single_or_default(async))).Message);

public override Task Filtered_include_skip_navigation_order_by_skip_take_then_include_skip_navigation_where(bool async) => Task.CompletedTask;
public override async Task Filtered_include_skip_navigation_order_by_skip_take_then_include_skip_navigation_where(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Filtered_include_skip_navigation_order_by_skip_take_then_include_skip_navigation_where(async))).Message);

[ConditionalTheory(Skip = "Issue#21541")]
public override Task Left_join_with_skip_navigation(bool async) => base.Left_join_with_skip_navigation(async);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.Sqlite.Internal;
using Xunit;

namespace Microsoft.EntityFrameworkCore.Query
Expand All @@ -13,19 +15,26 @@ public ManyToManyQuerySqliteTest(ManyToManyQuerySqliteFixture fixture)
{
}

// Sqlite does not support Apply operations
public override async Task Skip_navigation_order_by_single_or_default(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Skip_navigation_order_by_single_or_default(async))).Message);

public override Task Skip_navigation_order_by_single_or_default(bool async)
=> Task.CompletedTask;
public override async Task Filtered_include_skip_navigation_order_by_skip_take_then_include_skip_navigation_where(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Filtered_include_skip_navigation_order_by_skip_take_then_include_skip_navigation_where(async))).Message);

public override Task Filtered_include_skip_navigation_order_by_skip_take_then_include_skip_navigation_where(bool async)
=> Task.CompletedTask;
public override async Task Include_skip_navigation_then_include_inverse_throws_in_no_tracking(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Include_skip_navigation_then_include_inverse_throws_in_no_tracking(async))).Message);

[ConditionalTheory(Skip = "Issue#21541")]
public override Task Left_join_with_skip_navigation(bool async)
=> Task.CompletedTask;

public override Task Include_skip_navigation_then_include_inverse_throws_in_no_tracking(bool async)
=> Task.CompletedTask;
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.Sqlite.Internal;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;
using Xunit.Abstractions;

namespace Microsoft.EntityFrameworkCore.Query
Expand All @@ -15,13 +18,28 @@ public NorthwindIncludeNoTrackingQuerySqliteTest(NorthwindQuerySqliteFixture<Noo
//TestSqlLoggerFactory.CaptureOutput(testOutputHelper);
}

// Sqlite does not support Apply operations
public override Task Include_collection_with_cross_apply_with_filter(bool async) => Task.CompletedTask;
public override async Task Include_collection_with_cross_apply_with_filter(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Include_collection_with_cross_apply_with_filter(async))).Message);

public override Task Include_collection_with_outer_apply_with_filter(bool async) => Task.CompletedTask;
public override async Task Include_collection_with_outer_apply_with_filter(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Include_collection_with_outer_apply_with_filter(async))).Message);

public override Task Filtered_include_with_multiple_ordering(bool async) => Task.CompletedTask;
public override async Task Filtered_include_with_multiple_ordering(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Filtered_include_with_multiple_ordering(async))).Message);

public override Task Include_collection_with_outer_apply_with_filter_non_equality(bool async) => Task.CompletedTask;
public override async Task Include_collection_with_outer_apply_with_filter_non_equality(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Include_collection_with_outer_apply_with_filter_non_equality(async))).Message);
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.Sqlite.Internal;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;
using Xunit.Abstractions;

namespace Microsoft.EntityFrameworkCore.Query
Expand All @@ -15,12 +18,28 @@ public NorthwindIncludeQuerySqliteTest(NorthwindQuerySqliteFixture<NoopModelCust
//TestSqlLoggerFactory.CaptureOutput(testOutputHelper);
}

// Sqlite does not support Apply operations
public override Task Include_collection_with_cross_apply_with_filter(bool async) => Task.CompletedTask;
public override async Task Include_collection_with_cross_apply_with_filter(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Include_collection_with_cross_apply_with_filter(async))).Message);

public override Task Include_collection_with_outer_apply_with_filter(bool async) => Task.CompletedTask;
public override async Task Include_collection_with_outer_apply_with_filter(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Include_collection_with_outer_apply_with_filter(async))).Message);

public override Task Filtered_include_with_multiple_ordering(bool async) => Task.CompletedTask;
public override Task Include_collection_with_outer_apply_with_filter_non_equality(bool async) => Task.CompletedTask;
public override async Task Filtered_include_with_multiple_ordering(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Filtered_include_with_multiple_ordering(async))).Message);

public override async Task Include_collection_with_outer_apply_with_filter_non_equality(bool async)
=> Assert.Equal(
SqliteStrings.ApplyNotSupported,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Include_collection_with_outer_apply_with_filter_non_equality(async))).Message);
}
}
Loading

0 comments on commit d5112aa

Please sign in to comment.