Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EF7 should work with SQLite version that is included in Windows and older Ubuntu LTS #28776

Closed
bricelam opened this issue Aug 18, 2022 · 16 comments · Fixed by #28911
Closed

EF7 should work with SQLite version that is included in Windows and older Ubuntu LTS #28776

bricelam opened this issue Aug 18, 2022 · 16 comments · Fixed by #28911
Assignees
Labels
area-save-changes area-sqlite closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@bricelam
Copy link
Contributor

Issue #24835 raised the minimum required version of SQLite to 3.35. However, the latest version of SQLitePCLRaw.bundle_e_sqlcipher is based on SQLite 3.34.1.

A simple project like this...

<ItemGroup>
  <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite.Core" Version="7.0.0-*" />
  <PackageReference Include="SQLitePCLRaw.bundle_e_sqlcipher" Version="2.1.0" />
</ItemGroup>
using Microsoft.Data.Sqlite;
using Microsoft.EntityFrameworkCore;

using var db = new Context();
db.Database.EnsureDeleted();
db.Database.EnsureCreated();

db.Add(new Entity { Value = 36 });
db.SaveChanges();

class Entity
{
    public int Id { get; set; }
    public int Value { get; set; }
}

class Context : DbContext
{
    public DbSet<Entity> Entities { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder options)
        => options.UseSqlite("Data Source=ConsoleApp36.db");
}

...throws the following exception.

DbUpdateException: An error occurred while saving the entity changes. See the inner exception for details.
 ---> Microsoft.Data.Sqlite.SqliteException (0x80004005): SQLite Error 1: 'near "RETURNING": syntax error'.
   at Microsoft.Data.Sqlite.SqliteException.ThrowExceptionForRC(Int32 rc, sqlite3 db)
   at Microsoft.Data.Sqlite.SqliteCommand.PrepareAndEnumerateStatements(Stopwatch timer)+MoveNext()
   at Microsoft.Data.Sqlite.SqliteCommand.GetStatements(Stopwatch timer)+MoveNext()
   at Microsoft.Data.Sqlite.SqliteDataReader.NextResult()
   at Microsoft.Data.Sqlite.SqliteCommand.ExecuteReader(CommandBehavior behavior)
   at Microsoft.Data.Sqlite.SqliteCommand.ExecuteDbDataReader(CommandBehavior behavior)
   at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReader(RelationalCommandParameterObject parameterObject)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)
   --- End of inner exception stack trace ---
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Storage.RelationalDatabase.SaveChanges(IList`1 entries)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IList`1 entriesToSave)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(StateManager stateManager, Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.<>c.<SaveChanges>b__107_0(DbContext _, ValueTuple`2 t)
   at Microsoft.EntityFrameworkCore.Storage.NonRetryingExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges()
   at Program.<Main>$(String[] args)
@bricelam
Copy link
Contributor Author

@ericsink Any plans to update e_sqlicipher to SQLite 3.35 or newer in the near future?

@ericsink
Copy link

@bricelam Assuming the upstream code is more recent (very likely), I should be able to do that.

@bricelam
Copy link
Contributor Author

🤔 I wonder if we can update EFCore.CrossStore.FunctionalTests to run against e_sqlcipher (and sqlite3 and winsqlite3 too) to catch things like this earlier.

@bricelam
Copy link
Contributor Author

bricelam commented Aug 18, 2022

Looks like SQLitePCLRaw.bundle_winsqlite3 is also broken. The latest insiders build of Windows 11 only includes winsqlite3 version 3.34.1. The version hasn't changed since the May 2021 Update. I'll follow up with them offline to see what their plans are.

@ericsink
Copy link

FYI, my github action for building e_sqlite3 and e_sqlcipher is currently broken because it downloads toolchains from musl.cc and that site has blocked microsoft IP addresses:

https://github.com/orgs/community/discussions/27906

Workaround pending...

@ajcvickers ajcvickers changed the title Can't use SQLitePCLRaw.bundle_e_sqlcipher with EF7 EF7 should work with SQLite version that is included in Windows and older Ubuntu LTS Aug 22, 2022
@ajcvickers ajcvickers added this to the 7.0.0 milestone Aug 22, 2022
@ajcvickers
Copy link
Member

@roji Let's try and get this into rc2. Ideally, we would detect the SQLite version and automatically do the right thing. If that's not feasible, then we can have an AppContext switch that allows people to opt into the older way of doing things.

@roji
Copy link
Member

roji commented Aug 22, 2022

@ajcvickers do you mean provide some means to revert to the older update SQL for SQLite, that doesn't use RETURNING? Should we first see whether @ericsink can work around the build issue, or is there a problem even after that?

@ajcvickers
Copy link
Member

ajcvickers commented Aug 22, 2022

@roji If it was only sqlcipher, then yes. But apparently the SQLite that ships with Windows is too old to wok with RETURNING, as is the one that ships with LTS-1 of Ubuntu.

@roji
Copy link
Member

roji commented Aug 22, 2022

OK, I see. So is it correct that this is only for people who reference Microsoft.EntityFrameworkCore.Sqlite.Core, as opposed to M.EFC.Sqlite? Are many people doing that, and is it viable to just require people to upgrade their SQLite?

Am hoping it wouldn't be too hard to have an alternative code path for this, but it would be better to avoid the complexity if we can etc.

@ajcvickers
Copy link
Member

OK, I see. So is it correct that this is only for people who reference Microsoft.EntityFrameworkCore.Sqlite.Core, as opposed to M.EFC.Sqlite?

Yes.

Are many people doing that

We really have no idea.

and is it viable to just require people to upgrade their SQLite?

Possibly, but again we don't know.

Am hoping it wouldn't be too hard to have an alternative code path for this, but it would be better to avoid the complexity if we can etc.

There was a suggestion that we could not do this now and patch if we get enough feedback. But if we're going to take that approach, then I'd like to know that a patch is low-enough risk, so we should investigate now. But then, if it is low risk, then we may want to just do it for rc2--that would be my preference. And if it's not low-risk, well, then I guess it is what it is...

@ericsink
Copy link

I see this issue is shifting toward broader concerns, but... FWIW, I've got my sqlcipher updated and the build issue resolved. I should be able to release that in a build soon.

@roji
Copy link
Member

roji commented Aug 23, 2022

OK @ajcvickers understood, I'll do the work on this; if it seems like it becomes complex/risky I'll let you know.

@bricelam any way to find out at runtime which version of SQLite is being used? If it's easy/possible we can use that to switch the behavior.

@smitpatel
Copy link
Contributor

@roji
Copy link
Member

roji commented Aug 23, 2022

Thanks @smitpatel

@bricelam
Copy link
Contributor Author

@bricelam any way to find out at runtime which version of SQLite is being used? If it's easy/possible we can use that to switch the behavior.

Two ways. If you have access to the DbConnection, use the ServerVersion property. If not, use SQLitePCL.raw.sqlite3_libversion_number()

roji added a commit to roji/efcore that referenced this issue Aug 28, 2022
roji added a commit to roji/efcore that referenced this issue Aug 28, 2022
roji added a commit to roji/efcore that referenced this issue Aug 28, 2022
roji added a commit to roji/efcore that referenced this issue Aug 29, 2022
@roji roji added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 29, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0, 7.0.0-rc2 Aug 31, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-rc2, 7.0.0 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-save-changes area-sqlite closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants