Set SQLITE_LIMIT_FUNCTION_ARG to SQLite's normal default.#2493
Conversation
People keep legitimately running up against this limit. It's not really clear what value it is providing, so let's just match the default. Fixes #2412
|
Just to add color here, according to https://www.sqlite.org/limits.html (search for "Maximum Number Of Arguments On A Function"), the default is 100 and absolute maximum is 127 because this value is sometimes stored in an unsigned char. Sure enough, sqliteLimit.h has: I can't seem to figure out where the default of 100 is set. |
| sqlite3_limit(db, SQLITE_LIMIT_COMPOUND_SELECT, 5); | ||
| sqlite3_limit(db, SQLITE_LIMIT_VDBE_OP, 25000); | ||
| sqlite3_limit(db, SQLITE_LIMIT_FUNCTION_ARG, 32); | ||
| // For SQLITE_LIMIT_FUNCTION_ARG we use the default instead of the "security" recommendation |
There was a problem hiding this comment.
nit: s/default/maximum/ ?
|
Weird, https://www.sqlite.org/security.html says that the default is 127, not 100. My intent was to use the default. I guess I can try to find out what sqlite actually reports its own default as being... |
|
Digging more: the default is 127 and https://www.sqlite.org/limits.html is buggy. Here's what my The limits from the compile-time constants are initialized into a So it's the default and maximum. |
| sqlite3_limit(db, SQLITE_LIMIT_FUNCTION_ARG, 32); | ||
| // For SQLITE_LIMIT_FUNCTION_ARG we use the default instead of the "security" recommendation | ||
| // because there are too many valid use cases for large argument lists, especially json_object. | ||
| sqlite3_limit(db, SQLITE_LIMIT_FUNCTION_ARG, 127); |
There was a problem hiding this comment.
Since this might change in the future, maybe we should add a link to the existing documentation?
PS: https://www.sqlite.org/c3ref/create_function.html Also mentions max 127 value. |
|
OK seems like this is correct as-is, just gonna merge. |
|
@kentonv Hi, when is this gonna be released up stream into production ? Unfortunately, i am hitting the upper limit of 32 (I have 36 params in json_object) |
People keep legitimately running up against this limit. It's not really clear what value it is providing, so let's just match the default.
Fixes #2412