-
-
Notifications
You must be signed in to change notification settings - Fork 110
WIP: api!: add LIMIT arg to get_chat_media
#7325
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
base: main
Are you sure you want to change the base?
Conversation
In Delta Chat desktop we show the 3 recently used WebXDC apps, which relies on `get_chat_media`, which is quite expensive. Hopefully adding `LIMIT 3` makes it faster. Marking this as a breaking change because it's breaking TypeScript-wise, but shouldn't be breaking behavior-wise, because not providing the argument in JSON-RPC should be equivalent to providing `null` (which gets converted to `None`). TODO: - [ ] Add to CFFI? - [ ] Docs. Both the core fn and the JSON-RPC.
e3728b2 to
781de46
Compare
|
related to the need to provide null in typescript for |
| chat_id.unwrap_or_else(|| ChatId::new(0)), | ||
| DC_CHAT_ID_TRASH, | ||
| Viewtype::Webxdc, | ||
| limit.map(|l| l.to_string()).unwrap_or("ALL".to_string()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SQLite doesn't support ALL. Looks like we need to omit the parameter somehow. Maybe using format!
|
Alternatively there could be another jsonrpc api just for the purpose of returning the last 3 webxdc apps. jsonrpc apis are easy to create compared to cffi api. (Could potentially be optimized even more than adding an option to the existing api, because it could be implemented differently under the hood) |
|
please do some measurements on some slow chats before merging or diving into details. it is not always the case that a LIMIT makes things significantly faster. eg. ORDER BY and SELECT may still needed to be done on whole data set; LIMIT is more often useful when it comes to remote connections, less often on local connections. but as said, measurement will tell |
In Delta Chat desktop we show the 3 recently used WebXDC apps,
which relies on
get_chat_media, which is quite expensive.Hopefully adding
LIMIT 3makes it faster.https://github.com/deltachat/deltachat-desktop/blob/d32e85ec1353730a512cbdfff6c9c4dba5816dd4/packages/frontend/src/components/screens/MainScreen/MainScreen.tsx#L170-L181
Marking this as a breaking change
because it's breaking TypeScript-wise,
but shouldn't be breaking behavior-wise,
because not providing the argument in JSON-RPC
should be equivalent to providing
null(which gets converted to
None).TODO: