sqlite{,-analyzer}: 3.28.0 -> 3.29.0 #65402
Conversation
|
perl's dbd sqlite will probably need updating, there's activity so hopefully it's available soon? |
| @@ -41,6 +41,9 @@ stdenv.mkDerivation rec { | |||
| "-DSQLITE_SECURE_DELETE" | |||
| "-DSQLITE_MAX_VARIABLE_NUMBER=250000" | |||
| "-DSQLITE_MAX_EXPR_DEPTH=10000" | |||
| # Release notes for 3.29.0 say this is recommended | |||
| # https://www.sqlite.org/compile.html#rcmd | |||
| "-DSQLITE_DQS=0" | |||
There was a problem hiding this comment.
I wonder about the comment here (should go into the commit message rather) since our settings don't follow all the recommendations, for instance they recommnad DSQLITE_MAX_EXPR_DEPTH=0 Shouldn't we update others as well ?
There was a problem hiding this comment.
Good question, the reasoning for these particular settings (where deviating from upstream) is unknown :/. Perhaps answers could be found with some git history spelunking but even better would be if we documented the purpose/scope/validation-still-needed (perhaps these parameters help one guy's use case but are terrible for other folks'? who can say....).
In short I suppose we should push the technical debt further down the road and leave the options as-is? :(
There was a problem hiding this comment.
If we change it then better early than right before the release.
SQLITE_MAX_VARIABLE_NUMBER and SQLITE_MAX_EXPR_DEPTH can be explained via #36293 .
As for other recommended defaults. Maybe we could just add them one by one across updates in order not to break too many things ?
|
Well we're behind 2 revs now. 3.30 came out. |
|
This PR should be closed or the good parts picked and updated, as 3.30 went into master in #70593 (comment) |
|
Thanks all, @teto and @d-goldin ! Sorry for not seeing this through, closing since seems like at least the version portion made it in. If anyone thinks it's worth addressing the issue discussed above re:justifying our config choices (esp those conflicting with sqlite's recommendations and/or other distro builds) please speak up -- even if collectively folks don't have gusto to tackle it just yet :). Otherwise let's hope those commits were made with wisdom and hope for the best.... |
Motivation for this change
https://www.sqlite.org/releaselog/3_29_0.html
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nix-review --run "nix-review wip"./result/bin/)nix path-info -Sbefore and after)