Skip to content
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

Optimisations for search queries #1082

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

discomrade
Copy link
Contributor

Disclaimer: I'm not experienced with SQL, so feel welcome to correct any mistaken assumptions I've made.

The first commit prevents tag queries from selecting all columns when only the id is being used, improving the speed of the join operations.
The second commit prevents all columns being fetched when only the total number of rows is relevant.

These are both improvements by themselves, although the first is also a useful step towards other optimisations, especially those needed to make INTERSECT and UNION operations efficient, which are necessary for #292

This makes future optimisations easier, such as only selecting specific columns to return and making disjunctive operations more efficient
@shish
Copy link
Owner

shish commented Feb 24, 2024

So I was looking at this and thinking "it's a great idea, something feels more complicated than needed though" - after much thinking and failing to put my finger on it, I opened up my editor and tried to figure out how I would write this code if I were doing it from scratch. But before I'd actually managed to write anything, Copilot had auto-completed what appears to be an even simpler and faster approach than either of our ideas :O The result is at #1090

a useful step towards other optimisations, especially those needed to make INTERSECT and UNION operations efficient, which are necessary for #292

I wonder about this though - if your approach is a little bit more complicated, but opens the door to useful features in a way that #1090 doesn't, then I would go with this approach 👀

@discomrade
Copy link
Contributor Author

discomrade commented Feb 24, 2024

I'm ready to upload my disjunctive ('OR') code as a draft pull request for reference, although be aware that I based it over the two commits in this merge request so I will need to force commit to it. (update: #1092)

By using images.id instead of images.* until a final JOIN INNER, I'm able to significantly improve the speed, so I believe the co-pilot approach can be adapted but it shouldn't be considered a replacement for the [core] only select id column on tag queries commit in this merge request until it uses .id.

Using .id only also allows me to avoid using INTERSECT, in favour of the much faster AND [x] IN ([y]), when factoring in disjunctive results. This reduces the query time by orders of magnitude, so I consider it necessary.

@discomrade discomrade marked this pull request as draft May 15, 2024 23:40
@discomrade
Copy link
Contributor Author

Marked as draft to avoid this being merged as-is:

This code should be rewritten and resubmitted after those pull requests are accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants