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

Microsoft.Data.Sqlite: Persist functions, PRAGMAs, etc. between close and re-open #13826

Closed
bricelam opened this issue Apr 6, 2017 · 25 comments · Fixed by #14789
Closed

Microsoft.Data.Sqlite: Persist functions, PRAGMAs, etc. between close and re-open #13826

bricelam opened this issue Apr 6, 2017 · 25 comments · Fixed by #14789
Assignees
Labels
area-adonet-sqlite breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@bricelam
Copy link
Contributor

bricelam commented Apr 6, 2017

This was previously discussed in aspnet/Microsoft.Data.Sqlite#61.

There are quite a few things in SQLite that need to be configured every time a connection is opened. (E.g. certain PRAGMAs, user-defined functions, collations, etc.)

The idea would be that you pass some configuration into your SqliteConnection instances, and every time the connection is opened, it would be configured appropriately.

@AlexanderTaeschner
Copy link
Contributor

For the PRAGMAs we could take the same route as System.Data.SQLite does it: use certain keywords in the ConnectionString. Looking at the ones from System.Data.SQLite one could go with:

  • Foreign Keys: True, False
  • Journal Mode: Delete, Truncate, Persist, Memory, Wal, Off
  • Recursive Triggers: True, False
  • Synchronous: Off, Normal, Full, Extra

Additional PRAGMA settings of interest could be:

  • Auto Vacuum: None, Full, Incremental
  • Automatic Index: True, False

For collations or user defined functions we would need an additional configuration object, but the information storage in this object will be tricky. One could add the same functions as defined in #14 and #19 inside this SqliteConnectionConfiguration class and use them to store the given information in lists which would be parsed later in the Open call of the collation.

@bricelam
Copy link
Contributor Author

bricelam commented Apr 12, 2017

I'm not a big fan of the things System.Data.SQlite added to the connection string. In my mind, things in the connection string should be things you can't change after the connection is opened.

You ideas about SqliteConnectionConfiguration match mine.

var config = new SqliteConfiguration();
config.AddFunction("func", myDelegate);

var connection = new SqliteConnection(connectionString, config);

connection.Open(); // Automatically calls connection.AddFunction("func", myDelegate);

@AlexanderTaeschner
Copy link
Contributor

AlexanderTaeschner commented Apr 12, 2017

Okay, then the keywords mentioned above would be properties of the SqliteConnectionConfiguration.
Proposal would than be:

public class SqliteConnectionConfiguration
{
    public bool ForeignKeys {get; set;} = false; // default?
    public bool RecursiveTriggers {get; set;} = false; // default?
    public JournalMode JournalMode {get; set;} = JournalMode.Wal;// enum name? default?
    public SynchronousMode Synchronous {get; set;} = SynchronousMode.Full; // default?
    public AutoVacuumMode AutoVacuum {get; set;} = AutoVacuumMode.None; // default?
    public AutomaticIndex {get; set;} = true; // default?
    
    public void AddCollation(
        string name,
        Comparison<string> comparison);

    public void AddCollation<T>(
        string name,
        T state,
        Func<T, string, string, int> comparison);

    public virtual void AddFunction<TResult>(
        string name,
        Func<TResult> function);

    public virtual void AddFunction<T1..16, TResult>(
        string name,
        Func<T1..16, TResult> function);

    public virtual void AddFunction<TState, TResult>(
        string name,
        TState state,
        Func<TState, TResult> function);

    public virtual void AddFunction<TState, T1..15, TResult>(
        string name,
        TState state,
        Func<TState, T1..15, TResult> function);

    public virtual void AddAggregate<TContext>(
        string name,
        Func<TContext, TContext> func);

    public virtual void AddAggregate<TContext, T1..15>(
        string name,
        Func<TContext, T1..15, TContext> func);

    public virtual void AddAggregate<TContext>(
        string name,
        TContext seed,
        Func<TContext, TContext> func);

    public virtual void AddAggregate<TContext, T1..15>(
        string name,
        TContext seed,
        Func<TContext, T1..15, TContext> func);

    public virtual void AddAggregate<TContext, TResult>(
        string name,
        TContext seed,
        Func<TContext, TContext> func,
        Func<TContext, TResult> resultSelector);

    public virtual void AddAggregate<TContext, T1..15, TResult>(
        string name,
        TContext seed,
        Func<TContext, T1..15, TContext> function,
        Func<TContext, TResult> resultSelector);
}

@bricelam
Copy link
Contributor Author

If SQLite doesn't have a default (or it can be changed with a compile flag), we should probably make the properties nullable. null would mean "don't send a pragma".

@bricelam
Copy link
Contributor Author

bricelam commented Apr 12, 2017

We also need to define what happens when you mutate the configuration after a connection is open. Options:

  1. Throw; not allowed
  2. Immediately applied to any open connections
  3. Applied after the connection is closed and re-opened

@bricelam
Copy link
Contributor Author

We should also look at alternatives to this feature. Using the SqliteConnection.StateChanged event isn't too bad.

Maybe adding properties directly to SqliteConnection would be better along with making calls to AddFunction, etc. re-apply themselves whenever the connection is re-opened.

@AlexanderTaeschner
Copy link
Contributor

In my opinion it would be good to have a seperate object which stores the configuration, so that one can initialize different connections with the same configuration.

Regarding mutation after initialization, the question is if this is really necessary. At the moment I see no real use case for this. In order to prevent the mutation we could also package the properties and methods proposed above in some kind of SqliteConnectionConfigurationCreator class and define the SqliteConnectionConfiguration as completely immutable. In order to create the configuration one can add a SqliteConnectionConfigurationCreator.CreateConfiguration() method and/or an constructor SqliteConnectionConfiguration(SqliteConnectionConfigurationCreator creator).

@evil-shrike
Copy link

Why not support all CS keyword from System.Data.SQLite?
Support Foreign Keys=True (especially if it's not default) is a must.
Adding corresponding properties (get/set) to connection object is needed as well. Boolean SqliteConnection.ForeignKeys {get;set;}

Could you tell me how can I enable Foreign Keys currently on 2.0?

@AlexanderTaeschner
Copy link
Contributor

@evil-shrike: You can enable foreign keys using the sqlite PRAGMA commands (https://www.sqlite.org/pragma.html):

using (var command = connection.CreateCommand())
{
    command.CommandText = "PRAGMA foreign_keys = true;";
    command.ExecuteNonQuery();
}

@bricelam
Copy link
Contributor Author

My latest thinking on this is that we should probably add a connection pool in version 3.0, and when we do that, we can make the calls to CreateFunction(), etc. persistent.

@jonreis
Copy link

jonreis commented Jul 19, 2018

I am missing the ability to add the items in the connection string that allow you to fully initialize access to a particular database. I do not know of any other driver where you cannot access the database after configuring a proper connection string.

In Microsoft.Data.Sqlite one has to execute a number of PRAGMAs before the connection is actually valid. This breaks many tools such as RoundhousE (http://projectroundhouse.org) which takes a connection string and a set of sql scripts and migrates the database. There is no provision in these tools to allow commands to be run every time a connection is made. For example, in System.Data.Sqlite one can set the Password, Pooling, and many other things that fully set up the connection to the database so that it is ready for action.

It seems that the idea of a connection string is to fully initialize a connection so that work can be performed on the database. Having to execute PRAGMA statements complicates initialization and prevents existing tools from working.

Also I believe from a pooling implementation perspective in other databases, that connections are pooled based on matching connection strings. If the connect string doesn't fully describe the settings of the connection will it not be difficult to determine if a connection can be reused in a particular context?

@bricelam
Copy link
Contributor Author

@jonreis Can you provide specific examples of what breaks in the current implementation?

@bricelam
Copy link
Contributor Author

See #401 for why we didn't add Password.

@bricelam bricelam changed the title Connection configuration Connection configuration (persist between close and re-open) Sep 25, 2018
@ajcvickers ajcvickers transferred this issue from aspnet/Microsoft.Data.Sqlite Oct 31, 2018
@ajcvickers ajcvickers added this to the 3.0.0 milestone Oct 31, 2018
@ajcvickers ajcvickers changed the title Connection configuration (persist between close and re-open) Microsoft.Data.Sqlite: Connection configuration (persist between close and re-open) Oct 31, 2018
@bricelam bricelam self-assigned this Nov 29, 2018
@bricelam
Copy link
Contributor Author

bricelam commented Nov 29, 2018

We've been thinking about this again. Our current thinking is to make the following calls persist between closing and reopening a connection.

  • CreateFunction
  • CreateAggregate
  • CreateCollation
  • EnableExtensions

If performance is significantly degraded after this, we'll consider adding a connection pool (#13837).

We'll consider adding methods/properties/connection string keywords for things like the following based on feedback.

  • SELECT load_extension() (System.Data.SQLite has a method for this.)
  • PRAGMA recursive_triggers (Can this be enabled by default in e_sqlite3? If so, why turn it off? System.Data.SQLite has a connection string keyword for this.)
  • PRAGMA case_sensitive_like (Why turn this on?)
  • PRAGMA foreign_keys (Enabled by default in e_sqlite3. Why turn this off outside of rebuilds? System.Data.SQLite has a connection string keyword for this.)

We're adding a connection string option for PRAGMA key in #13828

@bricelam
Copy link
Contributor Author

bricelam commented Nov 30, 2018

I'm opening up to the idea of adding more connection string keywords like Foreign Keys, Journal Mode, and Recursive Triggers since you generally don't change them over the lifetime of the connection and you want to set them the same for every connection.

@bricelam bricelam changed the title Microsoft.Data.Sqlite: Connection configuration (persist between close and re-open) Microsoft.Data.Sqlite: Persist functions, PRAGMAs, etc. between close and re-open Nov 30, 2018
@bricelam
Copy link
Contributor Author

bricelam commented Nov 30, 2018

Tentative plan:

Add Foreign Keys, Password (#13828), and Recursive Triggers as connection string keywords. Send corresponding PRAGMA statements on connection open.

Add LoadExtension method to SqliteConnection. Keep track of calls to this, CreateFunction, CreateAggregate, CreateCollation, and EnableExtensions. Call corresponding APIs on connection open.

Need to think more about the interaction between EnableExtensions, LoadExtension, Close, and Open.

@bricelam
Copy link
Contributor Author

bricelam commented Jan 2, 2019

WIP branch for allowing Foreign Keys=True and Recursive Triggers=True in the connection string: bricelam:keywords

bricelam added a commit that referenced this issue Feb 22, 2019
…teFunction & EnableExtensions between close and re-open

Part of #13826
@bricelam bricelam added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Feb 22, 2019
@bricelam
Copy link
Contributor Author

The breaking change here is unlikely to affect many people: If you override a built-in collation or function, that override now remains in effect after closing and re-opening the connection which may affect behavior. But that's more likely to fix you application than break it. 😉

bricelam added a commit that referenced this issue Feb 28, 2019
This also updates EFCore.Sqlite to use the new method.

There is a breaking change in SpatialiteLoader. The methods used to take a DbConnection (I guess 'cause we could) but now take a SqliteConnection.

You no longer need to call SqliteConnection.EnableExtensions() before using SpatialiteLoader since the new LoadExtension method will bypass the connection setting. This greatly improves usability and makes EnableExtensions only apply to the load_extension SQL function. Given this, EnableExtensions should probably have been a connection string keyword.

In fact, SpatialiteLoader isn't really needed anymore since calling UseNetTopologySuite now loads the extension for open external connections. Filed #14821 to decide its fate.

LoadExtension has some unusual behavior when the connection is closed. Any exceptions will be delayed until the connection is opened. There is no way to remove an extension you tried to load, so you'll have to create a new connection object to get back in a good state.

Resolves #13826
@bricelam
Copy link
Contributor Author

I forgot to mention this here:

Since setting the Journal Mode to Write-Ahead Logging (WAL) is persisted, I decided it would be better for EF just to set it for databases it creates. (See #14059) No need for a connection string keyword yet since this probably covers most scenarios where you want to change it from the default.

@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview4 Apr 15, 2019
@vhking
Copy link

vhking commented Apr 24, 2019

@bricelam Is it possible to set Synchronous in the connection string in the new .net 3.0.0 preview 4? If so, how would you do that?

@bricelam
Copy link
Contributor Author

No, we haven't added that one. What is your scenario? Maybe what you really want is for e_sqlite3 to define SQLITE_DEFAULT_WAL_SYNCHRONOUS=1?

@vhking
Copy link

vhking commented Apr 25, 2019

We are running on an ARM/iMX6 SBC with Linux/debian where inserts is very slow (2-3 sec). We were hoping to get better performance by doing some optimization on SQLite.

@bricelam
Copy link
Contributor Author

bricelam commented Apr 25, 2019

Have you seen this post? It's a good place to start. Issue #14044 will make perf even better with higher-level libraries like EF. Also if you're using encryption, make sure you're not closing a re-openeing the connection a lot (issue #13837 will help this)--one open connection per thread is a good strategy.

@ajcvickers ajcvickers modified the milestones: 3.0.0-preview4, 3.0.0 Nov 11, 2019
@darkguy2008
Copy link

So is this done? I'm getting Keyword not supported: "synchronous" when trying to set this PRAGMA. Any ideas?

@bricelam
Copy link
Contributor Author

@darkguy2008 Can you submit a new issue and clarify what you are doing when you get this error?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-adonet-sqlite breaking-change closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants