-
Notifications
You must be signed in to change notification settings - Fork 6
Add index to message on receiver_id, read_ #20
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: master
Are you sure you want to change the base?
Conversation
Add mention of Timer requirement (mariodavid#19)
Hi, thanks for the PR. Unfortunately, as is I cannot merge it. In fact, CUBA does not use this JPA mechanism to define indexes. Instead, it uses the DB script update mechanism to generate SQL files for the different DBs. In Studio you find the section "Indexes" for an entity: The following things are missing:
If you want to dig into that a little deeper, feel free to do so. I will guide you wherever I can. In order to spin up the different DBMS systems, you can look at this markdown file, which explains on how to start the different docker containers: https://github.com/mariodavid/cuba-component-user-inbox/blob/master/create-databases.md Cheers |
Aha that makes total sense. I didn't know how that would work within the context of an add-on. I'll think about doing that. The index will pretty much be required in my usage anyway. |
I just tried on my own for the petclinic. And in fact you are partly right. Studio changes the table annotation (probably as you did). But when you go to Menu > CUBA > Generate DB Scrips - it will also additionally generate those (for the currently setup DB type). So you were probably already on the right track. |
Yeah... when you're working on a normal project, you can go the indexes tab of the entity and make the indexes, then CUBA --> Gen scripts and then update, and it all works. I didn't know how it would work in the context of an add-on though, since I've never made or worked on one. |
In fact, from this perspective, an addon is exactly the same as an application. In fact, every application can be transformed into an addon and it will just generate a XML descriptor. Even CUBA core itself is nothing more than an addon. It is a little cumbersome to generate the scripts for all DBs and test the changes. Therefore I was not super keen to just go ahead and do it. After thinking about the In order to make the query fast, we have to add all columns to the index. |
Interestingly (and oddly), the indexes automatically added by CUBA never include delete_ts either. I wonder if that's an oversight by the CUBA team or if they didn't figure it was needed since they assume people will periodically purge the soft-deleted records, making it not as much of an issue? |
I've been playing with a better way to handle the counter/etc entirely. Just for testing purposes I extended This gets rid of the need for an index and also doesn't hit the DB. What do you think? |
Hi, but if you only rely on the messages send around, how do you ensure that the users, that are not logged in at the time the message is received still get the information about their unread messages? I think this transient behavior is not really what is currently implemented via the menu badge. Or do I miss something? Cheers |
The first call to the original get of the unread messages is still in the init event, so a user logging in gets their count pulled that way. So yeah the index will still be needed. What do you make of the fact that none of the CUBA-generated indexes contain delete_ts? You seem to be pretty close with the team. Maybe they just assume people keep the soft-deleted records pruned? |
As discussed on the forum, the countUnreadMessagesForCurrentUser() method polls the DB by receiver_id and read_ in order to get said count. But there is currently no index to make that query perform well, so... adding it would be a good idea.