Fall back to non-RETURNING updates with old Sqlite

Fixes #28776

benchmark/EFCore.Sqlite.Benchmarks/EFCore.Sqlite.Benchmarks.csproj

@@ -16,7 +16,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3" Version="2.1.1-pre20220822172036" />
+    <PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3" Version="$(SqlitePCLRawVersion)" />
   </ItemGroup>
 
   <ItemGroup>

eng/Versions.props

@@ -30,5 +30,6 @@
   </PropertyGroup>
   <PropertyGroup Label="Other dependencies">
     <MicrosoftCodeAnalysisVersion>4.2.0</MicrosoftCodeAnalysisVersion>
+    <SqlitePCLRawVersion>2.1.1-pre20220822172036</SqlitePCLRawVersion>
   </PropertyGroup>
 </Project>

src/EFCore.Sqlite.Core/Extensions/SqliteServiceCollectionExtensions.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.ComponentModel;
+using Microsoft.Data.Sqlite;
 using Microsoft.EntityFrameworkCore.Sqlite.Diagnostics.Internal;
 using Microsoft.EntityFrameworkCore.Sqlite.Infrastructure.Internal;
 using Microsoft.EntityFrameworkCore.Sqlite.Metadata.Internal;
@@ -98,22 +99,29 @@ public static IServiceCollection AddEntityFrameworkSqlite(this IServiceCollectio
             .TryAdd<IRelationalAnnotationProvider, SqliteAnnotationProvider>()
             .TryAdd<IModelValidator, SqliteModelValidator>()
             .TryAdd<IProviderConventionSetBuilder, SqliteConventionSetBuilder>()
-            .TryAdd<IUpdateSqlGenerator, SqliteUpdateSqlGenerator>()
             .TryAdd<IModificationCommandBatchFactory, SqliteModificationCommandBatchFactory>()
             .TryAdd<IRelationalConnection>(p => p.GetRequiredService<ISqliteRelationalConnection>())
             .TryAdd<IMigrationsSqlGenerator, SqliteMigrationsSqlGenerator>()
             .TryAdd<IRelationalDatabaseCreator, SqliteDatabaseCreator>()
             .TryAdd<IHistoryRepository, SqliteHistoryRepository>()
             .TryAdd<IRelationalQueryStringFactory, SqliteQueryStringFactory>()
-
-            // New Query Pipeline
             .TryAdd<IMethodCallTranslatorProvider, SqliteMethodCallTranslatorProvider>()
             .TryAdd<IAggregateMethodCallTranslatorProvider, SqliteAggregateMethodCallTranslatorProvider>()
             .TryAdd<IMemberTranslatorProvider, SqliteMemberTranslatorProvider>()
             .TryAdd<IQuerySqlGeneratorFactory, SqliteQuerySqlGeneratorFactory>()
             .TryAdd<IQueryableMethodTranslatingExpressionVisitorFactory, SqliteQueryableMethodTranslatingExpressionVisitorFactory>()
             .TryAdd<IRelationalSqlTranslatingExpressionVisitorFactory, SqliteSqlTranslatingExpressionVisitorFactory>()
             .TryAdd<IQueryTranslationPostprocessorFactory, SqliteQueryTranslationPostprocessorFactory>()
+            .TryAdd<IUpdateSqlGenerator>(sp =>
+            {
+                // Support for the RETURNING clause on INSERT/UPDATE/DELETE was added in Sqlite 3.35.
+                // Detect which version we're using, and fall back to the older INSERT/UPDATE+SELECT behavior on legacy versions.
+                var dependencies = sp.GetRequiredService<UpdateSqlGeneratorDependencies>();
+
+                return new Version(new SqliteConnection().ServerVersion) < new Version(3, 35)
+                    ? new SqliteLegacyUpdateSqlGenerator(dependencies)
+                    : new SqliteUpdateSqlGenerator(dependencies);
+            })
             .TryAddProviderSpecificServices(
                 b => b.TryAddScoped<ISqliteRelationalConnection, SqliteRelationalConnection>());
 

src/EFCore.Sqlite.Core/Update/Internal/SqliteLegacyUpdateSqlGenerator.cs

@@ -0,0 +1,84 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Text;
+using Microsoft.EntityFrameworkCore.Sqlite.Internal;
+
+namespace Microsoft.EntityFrameworkCore.Sqlite.Update.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 SqliteLegacyUpdateSqlGenerator : UpdateAndSelectSqlGenerator
+{
+    /// <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 SqliteLegacyUpdateSqlGenerator(UpdateSqlGeneratorDependencies dependencies)
+        : base(dependencies)
+    {
+    }
+
+    /// <summary>
+    ///     Appends a <c>WHERE</c> condition for the identity (i.e. key value) of the given column.
+    /// </summary>
+    /// <param name="commandStringBuilder">The builder to which the SQL should be appended.</param>
+    /// <param name="columnModification">The column for which the condition is being generated.</param>
+    protected override void AppendIdentityWhereCondition(StringBuilder commandStringBuilder, IColumnModification columnModification)
+    {
+        Check.NotNull(commandStringBuilder, nameof(commandStringBuilder));
+        Check.NotNull(columnModification, nameof(columnModification));
+
+        SqlGenerationHelper.DelimitIdentifier(commandStringBuilder, "rowid");
+        commandStringBuilder.Append(" = ")
+            .Append("last_insert_rowid()");
+    }
+
+    /// <summary>
+    ///     Appends a SQL command for selecting the number of rows affected.
+    /// </summary>
+    /// <param name="commandStringBuilder">The builder to which the SQL should be appended.</param>
+    /// <param name="name">The name of the table.</param>
+    /// <param name="schema">The table schema, or <see langword="null" /> to use the default schema.</param>
+    /// <param name="commandPosition">The ordinal of the command for which rows affected it being returned.</param>
+    /// <returns>The <see cref="ResultSetMapping" /> for this command.</returns>
+    protected override ResultSetMapping AppendSelectAffectedCountCommand(StringBuilder commandStringBuilder, string name, string? schema, int commandPosition)
+    {
+        Check.NotNull(commandStringBuilder, nameof(commandStringBuilder));
+        Check.NotEmpty(name, nameof(name));
+
+        commandStringBuilder
+            .Append("SELECT changes()")
+            .AppendLine(SqlGenerationHelper.StatementTerminator)
+            .AppendLine();
+
+        return ResultSetMapping.LastInResultSet;
+    }
+
+    /// <summary>
+    ///     Appends a <c>WHERE</c> condition checking rows affected.
+    /// </summary>
+    /// <param name="commandStringBuilder">The builder to which the SQL should be appended.</param>
+    /// <param name="expectedRowsAffected">The expected number of rows affected.</param>
+    protected override void AppendRowsAffectedWhereCondition(StringBuilder commandStringBuilder, int expectedRowsAffected)
+    {
+        Check.NotNull(commandStringBuilder, nameof(commandStringBuilder));
+
+        commandStringBuilder.Append("changes() = ").Append(expectedRowsAffected);
+    }
+
+    /// <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 string GenerateNextSequenceValueOperation(string name, string? schema)
+        => throw new NotSupportedException(SqliteStrings.SequencesNotSupported);
+}

src/EFCore.Sqlite/EFCore.Sqlite.csproj

@@ -47,7 +47,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3" Version="2.1.1-pre20220822172036" />
+    <PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3" Version="$(SqlitePCLRawVersion)" />
   </ItemGroup>
 
   <ItemGroup>

src/Microsoft.Data.Sqlite.Core/Microsoft.Data.Sqlite.Core.csproj

@@ -39,7 +39,7 @@ Microsoft.Data.Sqlite.SqliteTransaction</Description>
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="SQLitePCLRaw.core" Version="2.1.1-pre20220822172036" />
+    <PackageReference Include="SQLitePCLRaw.core" Version="$(SqlitePCLRawVersion)" />
   </ItemGroup>
 
   <ItemGroup>

src/Microsoft.Data.Sqlite/Microsoft.Data.Sqlite.csproj

@@ -24,7 +24,7 @@ Microsoft.Data.Sqlite.SqliteTransaction</Description>
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3" Version="2.1.1-pre20220822172036" />
+    <PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3" Version="$(SqlitePCLRawVersion)" />
   </ItemGroup>
 
   <ItemGroup>

test/EFCore.Design.Tests/EFCore.Design.Tests.csproj

@@ -57,6 +57,7 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="$(MicrosoftCodeAnalysisVersion)" />
     <PackageReference Include="Microsoft.Extensions.DependencyModel" Version="$(MicrosoftExtensionsDependencyModelVersion)" />
+    <PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3" Version="$(SqlitePCLRawVersion)" />
   </ItemGroup>
 
 </Project>

test/EFCore.Sqlite.FunctionalTests/EFCore.Sqlite.FunctionalTests.csproj

@@ -53,7 +53,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3" Version="2.1.1-pre20220822172036" />
+    <PackageReference Include="SQLitePCLRaw.bundle_e_sqlite3" Version="$(SqlitePCLRawVersion)" />
   </ItemGroup>
 
 </Project>

test/EFCore.Sqlite.FunctionalTests/TestUtilities/SqliteVersionConditionAttribute.cs

@@ -6,32 +6,34 @@
 
 namespace Microsoft.EntityFrameworkCore.TestUtilities;
 
+#nullable enable
+
 [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class)]
 public sealed class SqliteVersionConditionAttribute : Attribute, ITestCondition
 {
-    private Version _min;
-    private Version _max;
-    private Version _skip;
+    private Version? _min;
+    private Version? _max;
+    private Version? _skip;
 
-    public string Min
+    public string? Min
     {
-        get => _min.ToString();
-        set => _min = new Version(value);
+        get => _min?.ToString();
+        set => _min = value is null ? null : new Version(value);
     }
 
-    public string Max
+    public string? Max
     {
-        get => _max.ToString();
-        set => _max = new Version(value);
+        get => _max?.ToString();
+        set => _max = value is null ? null : new Version(value);
     }
 
-    public string Skip
+    public string? Skip
     {
-        get => _skip.ToString();
-        set => _skip = new Version(value);
+        get => _skip?.ToString();
+        set => _skip = value is null ? null : new Version(value);
     }
 
-    private static Version Current
+    private static Version? Current
     {
         get
         {
@@ -61,7 +63,7 @@ public ValueTask<bool> IsMetAsync()
         return new ValueTask<bool>(_max == null ? Current >= _min : Current <= _max && Current >= _min);
     }
 
-    private string _skipReason;
+    private string? _skipReason;
 
     public string SkipReason
     {