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

Request: The switch from System.Data.SQLite.Core to Microsoft.Data.SQLite.Core is a pretty big breaking change #497

Closed
FutureTD opened this issue Aug 2, 2020 · 13 comments
Assignees
Labels
deployed Feature or bug is deployed at the current release fixed The bug, issue, incident has been fixed. priority Top priority feature or things to do request A request from the community. todo Things to be done in the future

Comments

@FutureTD
Copy link

FutureTD commented Aug 2, 2020

As soon as I tried to upgrade to RepoDb 1.0.15 and had to downgrade as I started to break stuff. Microsoft.Data.SQLite.Core is not as mature as System.Data.SQLite.Core and does not support some of its features at the moment such as (Case-sensitive parameter names) dotnet/efcore#18861 nor does it support (Connection Pooling) dotnet/efcore#13837 among other things.

Its just missing things that I and others are used to having, System.Data.SQLite.Core is just more mature and supports things not implemented in Microsoft.Data.SQLite.Core yet.

Was just wondering the reason for this change and If there was a way I could use System.Data.SQLite.Core without having a dependency on Microsoft.Data.SQLite.Core?

@mikependon mikependon self-assigned this Aug 2, 2020
@mikependon
Copy link
Owner

The old version is using only the System.Data.SQLite, not the System.Data.SQLite.Core. So that's something that we need to remove from the RepoDb versions specially that it is still referring the old EF library.

Yes, you may be able to use the System.Data.SQLite.Core still, but using your own bootstrap. Though I have not yet tested it, but that would still be the same.

But since the RepoDb version will be defaulted to Microsoft.Data.SQLite.Core, then there is always a reference towards that library. I hope you can live with it.

How to initialize the System.Data.SQLite.Core?

By default, you use to call the RepoDb.SqLiteBootstrap.Initialize(). This will only be defaulted to Microsoft.Data.SQLite.Core, to support the System.Data.SQLite.Core, you would need to the code below.

// Map the DbSetting
DbSettingMapper.Add(typeof(SQLiteConnection), new SqLiteDbSetting(), true);

// Map the DbHelper
DbHelperMapper.Add(typeof(SQLiteConnection), new SqLiteDbHelper(), true);

// Map the Statement Builder
StatementBuilderMapper.Add(typeof(SQLiteConnection), new SqLiteStatementBuilder(), true);

Such calls equate the bootstrapper calls.

@FutureTD
Copy link
Author

FutureTD commented Aug 2, 2020

Yes I am fine with that as dependency as long as I am still able to use System.Data.SQLite.Core which seems to be the case however, I am getting a exception on this line:

StatementBuilderMapper.Add(typeof(SQLiteConnection), new SqLiteStatementBuilder(), true);

Exception: System.NullReferenceException: 'The database setting cannot be null.'

I do have a custom SqliteDbSetting class

    public sealed class SqLiteDbSetting: BaseDbSetting
    {
        public SqLiteDbSetting()
        {
            AreTableHintsSupported = false;
            AverageableType = typeof(double);
            ClosingQuote = "]";
            DefaultSchema = null;
            IsDirectionSupported = false;
            IsExecuteReaderDisposable = true;
            IsMultiStatementExecutable = true;
            IsPreparable = true;
            IsUseUpsert = true;
            OpeningQuote = "[";
            ParameterPrefix = "@";
            SchemaSeparator = ".";
        }
    }

@mikependon
Copy link
Owner

@FutureTD - I am now simulating your use-case. Few moment please :) Thanks!

@mikependon
Copy link
Owner

@FutureTD - I think, it requires you to call the RepoDb.SqLiteBootstrap.Initialize() method first. That makes it a bug on the initialization.

@mikependon
Copy link
Owner

I think, I need to revert to use the System.Data.SQLite.Core instead as the other user has opened up the same issue as you. They also have referencing problem with the latest release. Would you be able to wait for the new release?

@FutureTD
Copy link
Author

FutureTD commented Aug 2, 2020

Ah ok I just changed the line to:

StatementBuilderMapper.Add(typeof(SQLiteConnection), new SqLiteStatementBuilder(new SqLiteDbSetting()), true);

and the exception was gone

@FutureTD
Copy link
Author

FutureTD commented Aug 2, 2020

Yes, I would be able to wait, thank you

@mikependon
Copy link
Owner

I saw that, but the problem is MDS made the breaking changes on the .NET CLR Type mapping. So there are coercion problem still. Unless your solution works, then I am fine retaining with MDS, otherwise I have to revert back to SDS.

@FutureTD
Copy link
Author

FutureTD commented Aug 2, 2020

My project seems to work based on the little testing I did, However SDS is probably used more MDS so I don't know. However, Im not sure how switching to MDS will effect other users.

@mikependon mikependon added priority Top priority feature or things to do request A request from the community. todo Things to be done in the future labels Aug 2, 2020
@mikependon
Copy link
Owner

A new release has been pushed. Please install the RepoDb.SqLite (v1.0.16).

> Install-Package RepoDb.SqLite -version 1.0.16

Both the 'SDS' and 'MDS' are referenced on the package. The same strategy has been made with RepoDb.SqlServer package. Up until the community decided in the near future, one may be eliminated soon. That's still unclear when.

@mikependon mikependon added deployed Feature or bug is deployed at the current release fixed The bug, issue, incident has been fixed. labels Aug 2, 2020
@mikependon mikependon changed the title The switch from System.Data.SQLite.Core to Microsoft.Data.SQLite.Core is a pretty big breaking change Request: The switch from System.Data.SQLite.Core to Microsoft.Data.SQLite.Core is a pretty big breaking change Aug 3, 2020
@mikependon
Copy link
Owner

Have you tested your solution based on the latest release? Planning to close this ticket soon. Thanks

@FutureTD
Copy link
Author

FutureTD commented Aug 5, 2020

Yes it works fine, Thanks for your hard work.

@FutureTD FutureTD closed this as completed Aug 5, 2020
@mikependon
Copy link
Owner

@FutureTD , thank you for verifying the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed Feature or bug is deployed at the current release fixed The bug, issue, incident has been fixed. priority Top priority feature or things to do request A request from the community. todo Things to be done in the future
Projects
None yet
Development

No branches or pull requests

2 participants