Modify toQueryString to chunk large list of ConditionParam #2565
Conversation
LZRS
force-pushed
the
2561-fix-sqlite-crash
branch
from
ae8187f
to
6b3d4c4
Compare
| "(${it.condition})" | ||
| } else { | ||
| it.condition | ||
| this.chunked(50) { conditionParams -> |
There was a problem hiding this comment.
@MJ1998 do you have thoughts on what the chunk size should be?
| * This is to prevent SQLite expression tree exceeding max depth of 1000 See | ||
| * https://www.sqlite.org/limits.html for Maximum Depth Of An Expression Tree | ||
| */ | ||
| const val CONDITION_PARAMS_CHUNK_SIZE = 50 |
There was a problem hiding this comment.
another question on this value, don't we want this to be as high as possible? I'd think fewer chunks are easier to process, and fewer checks definitely means fewer iterations. Why not set this to the max of 1000 as a default?
There was a problem hiding this comment.
Yes, you're right, the chunk size could be higher. Most of the filters for FhirEngine#search are implemented in subqueries, it seems subqueries add to the depth expression tree generated. Will try to figure out a higher number that could be suitable...
There was a problem hiding this comment.
It might be hard to find a number that might be suitable for most applications because of joins and subqueries that depend on the search. I'm thinking of setting the size to a lower number and maybe allow it to be configurable
LZRS
force-pushed
the
2561-fix-sqlite-crash
branch
from
80c50de
to
0264816
Compare
LZRS
force-pushed
the
2561-fix-sqlite-crash
branch
from
d9e6d03
to
d764115
Compare
https://stackoverflow.com/a/17032196
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #2561
Description
Chunks large expression list to limit 50 within parantheses to avoid crashing with
Expression tree is too large (maximum depth 1000), as described hereAlternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
Type
Choose one: (Bug fix | Feature | Documentation | Testing | Code health | Builds | Releases | Other)
Screenshots (if applicable)
Checklist
./gradlew spotlessApplyand./gradlew spotlessCheckto check my code follows the style guide of this project../gradlew checkand./gradlew connectedCheckto test my changes locally.