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

Bug when using with sqlite library that is compiled with SQLITE_DSQ=0 #12

Closed
Blackclaws opened this issue Dec 20, 2022 · 6 comments
Closed

Comments

@Blackclaws
Copy link

Sqlite has the oddity that it treats double quoted strings that don't evaluate to an identifier as strings. This is not sql standard behavior. Since in the initialization routine values are inserted into the meta table that contain strings that are double quoted. Those will fail with a rather cryptic message 'created is not a column name' when the underlying sqlite does not support this odd configuration option.

Since its recommended to compile sqlite with DSQ=0 (i.e. always treat double quotes as identifiers) it would be nice if this could be reflected in the sql statements by replacing double quoted strings by single quoted strings.

@mqudsi
Copy link
Member

mqudsi commented Jan 27, 2023

Thanks for pointing this out. I'll put out a release with this fix shortly.

@mqudsi mqudsi closed this as completed in 9f38c2d Jan 30, 2023
@mqudsi
Copy link
Member

mqudsi commented Jan 30, 2023

Hi @Blackclaws

Sorry for the delay but I just pushed out a commit that should fix this and will be making a final 6.x release with this fix soon. Are you able to test this?

I can compile SQLite with SQLITE_DSQ=0 but I'm not sure how you were actually using your copy of the SQLite library? The nuget has a dependency on SQLitePCLRaw.bundle_green which ships with its own copy of the SQLite library on all supported platforms - how did you bypass that? Are you including this package with private assets set to true then using your own SQLite backfill?

@mqudsi
Copy link
Member

mqudsi commented Jan 30, 2023

To make it easier to test, I published the latest build with these changes on nuget as NeoSmart.Cache.SqliteCache version 6.1.0-preview1.

@Blackclaws
Copy link
Author

Hi @Blackclaws

Sorry for the delay but I just pushed out a commit that should fix this and will be making a final 6.x release with this fix soon. Are you able to test this?

I can compile SQLite with SQLITE_DSQ=0 but I'm not sure how you were actually using your copy of the SQLite library? The nuget has a dependency on SQLitePCLRaw.bundle_green which ships with its own copy of the SQLite library on all supported platforms - how did you bypass that? Are you including this package with private assets set to true then using your own SQLite backfill?

I bypassed it by compiling the library myself excluding bundle_green. We're using the cache on Android via Unity where bundle_green doesn't work. In general I'm not sure if forcing this specific version is in general a good idea.

Basically what I did was remove bundle_green and manually linked against sqlite:

        SQLitePCL.raw.SetProvider(new SQLite3Provider_sqlite3());

@mqudsi
Copy link
Member

mqudsi commented Jan 31, 2023

Yeah, I'm not sure what the best thing for me to do would be. I think you don't need to compile the library yourself - if you add the right dependency then call SQLitePCL.raw.SetProvider(new SQLite3Provider_sqlite3()); before the static SqliteCache() constructor calls SQLitePCL.Batteries.Init();, I believe your call should make mine a no-op. Just make sure you run it before the SqliteCache type is loaded and see if that works?

I could remove the bundle_green dependency from the main nuget and replace it with just SQLitePCLRaw.core; ASP.NET Core users will be OK because the web app will initialize the provider for them but anyone using it framework-free will always need to add a dependency on one of the SQLitePCLRaw.provider.xxx libraries and most likely also on the matching SQLitePCLRaw.lib.xxx dependency as well.

Or I could ship a separate meta package NeoSmart.Caching.Sqlite.ProviderFree or something that basically excludes the bundle_green dependency by doing <PrivateAssets>SQLitePCLRaw.bundle_green</PrivateAssets>. Or I could just add a note to the README that anyone that wants to exclude the bundle_green dependency to bring their own SQLitePCLRaw provider should just use

<PackageReference Include="NeoSmart.Caching.Sqlite" Version="x.y.z" PrivateAssets="SQLitePCLRaw.bundle_green"  />

which I think should accomplish the same? You could try this approach as well.

@Blackclaws
Copy link
Author

I personally think that removing the explicit dependency on bundle_green makes sense given that its not the only way you can use SQLitePCLRaw and it restricts the user in what they can use. The correct dependency should be SQLitePCLRaw.core and a section in the docs under installation basically telling the user to go ahead and install one of the providers and initialize it should be fine as well? You could also split it into two packages, where one is the core cache without the dependency and the other one keeps the original name including the dependency (not breaking backwards compatibility).

The problem is that bundle_green itself ships with a couple of libraries that I'd rather like to avoid pulling into the project in the first place as they are unnecessary ballast (they don't work on iOS or Android anyway, at least not in the context we are in).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants