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

Increase bundled SQLite variables and depth #522

Merged
merged 3 commits into from
May 15, 2019
Merged

Conversation

pimeys
Copy link
Contributor

@pimeys pimeys commented May 15, 2019

We've been hitting the default MAX_VARIABLE_NUMBER and
MAX_EXPR_DEPTH with quite basic tests here in Prisma. I was able to
run the tests by using the Arch Linux packaged libsqlite3, but when
turning on the bundled version I was able to get my test to crash with
this test project:

https://github.com/pimeys/sqlite_parameter_test

Now taking a look how Arch Linux builds sqlite, I was able to find two
flags fixing the issue:

https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/sqlite#n35

I think it would be safe to include them in rusqlite.

We've been hitting the default `MAX_VARIABLE_NUMBER` and
`MAX_EXPR_DEPTH` with quite basic tests here in Prisma. I was able to
run the tests by using the Arch Linux packaged libsqlite3, but when
turning on the bundled version I was able to get my test to crash with
this test project:

https://github.com/pimeys/sqlite_parameter_test

Now taking a look how Arch Linux builds sqlite, I was able to find two
flags fixing the issue:

https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/sqlite#n35

I think it would be safe to include them in rusqlite.
@gwenn gwenn added the invalid label May 15, 2019
@gwenn
Copy link
Collaborator

gwenn commented May 15, 2019

Expected (or something like that):

diff --git a/libsqlite3-sys/build.rs b/libsqlite3-sys/build.rs
index e6942d0..66c6b8d 100644
--- a/libsqlite3-sys/build.rs
+++ b/libsqlite3-sys/build.rs
@@ -79,6 +79,9 @@ mod build_bundled {
         if cfg!(feature = "session") {
             cfg.flag("-DSQLITE_ENABLE_SESSION");
         }
+        if Some(limit) = env::var("SQLITE_MAX_VARIABLE_NUMBER") {
+            cfg.flag("-DSQLITE_MAX_VARIABLE_NUMBER="+limit);
+        }
         cfg.compile("libsqlite3.a");

         println!("cargo:lib_dir={}", out_dir);

@pimeys
Copy link
Contributor Author

pimeys commented May 15, 2019

Updated the pull request.

@gwenn gwenn added enhancement and removed invalid labels May 15, 2019
@gwenn
Copy link
Collaborator

gwenn commented May 15, 2019

Sorry,
I forgot:

        if Some(limit) = env::var("SQLITE_MAX_VARIABLE_NUMBER") {
            cfg.flag("-DSQLITE_MAX_VARIABLE_NUMBER="+limit);
        } 
        println!("cargo:rerun-if-env-changed=SQLITE_MAX_VARIABLE_NUMBER");

Thanks.

@pimeys
Copy link
Contributor Author

pimeys commented May 15, 2019

Done.

@gwenn
Copy link
Collaborator

gwenn commented May 15, 2019

Perfect.

@gwenn gwenn merged commit c87c49c into rusqlite:master May 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants