-
-
Notifications
You must be signed in to change notification settings - Fork 237
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
feat: improve pagination performance #879
base: master
Are you sure you want to change the base?
Conversation
Improve pagination performance by using ROW_NUMBER() and handling default orderBys.
test: pagination improvement
Co-authored-by: StyleCI Bot <[email protected]>
Requesting for review @alexc-jeromes and @mixaster, TIA! |
So, @yajra I implemented this in our app to check, and got some new errors on previously working queries, specifically an "Invalid Identifier" in the outer query on the "tablename.*" column. Also, the initial query that was giving me trouble still took more than 30 seconds. |
Is there a reason why there's a wrapper query? if I take my regular query and just add something like "FETCH FIRST 20 ROWS ONLY", it comes back in 0.28 seconds on a table of 281K records (in SQL Developer). Honest question, I really don't know too much about the inner workings of Oracle. |
@yajra I did some experimenting.... I replaced
and then
It seems to work throughout my app in initial testing. Looking for a sanity check. I could make a PR if you'd like. |
Unfortunately, there is a need to support a lower Oracle version down to 10g which is why I can't use I considered making this configurable and pagination will depend on the stated Oracle version but I couldn't execute it yet. |
Can you provide a snippet to reproduce the invalid identifier error? Based on my testing, some of my queries only improved by a few milliseconds. But I will still have to check on a bigger application. |
@yajra This QueryBuilder call:
Produced this SQL:
and the error was |
I adjusted the table alias to use the original table in I will also add some tests using Eloquent with relation count when I can. Thanks! |
Initial tests result vs 1.1k records: Before: select t2.* from ( select rownum AS "rn", t1.* from (select "ROLES".*, (select count(*) from "USERS" inner join "ROLE_USER" on "USERS"."ID" = "ROLE_USER"."USER_ID" where "ROLES"."ID" = "ROLE_USER"."ROLE_ID") as "USERS_COUNT", (select count(*) from "PERMISSIONS" inner join "PERMISSION_ROLE" on "PERMISSIONS"."ID" = "PERMISSION_ROLE"."PERMISSION_ID" where "ROLES"."ID" = "PERMISSION_ROLE"."ROLE_ID") as "PERMISSIONS_COUNT" from "ROLES" order by "ID" desc) t1 ) t2 where t2."rn" between 1 and 10
After: select "ROLES".*, (select count(*) from "USERS" inner join "ROLE_USER" on "USERS"."ID" = "ROLE_USER"."USER_ID" where "ROLES"."ID" = "ROLE_USER"."ROLE_ID") as "USERS_COUNT", (select count(*) from "PERMISSIONS" inner join "PERMISSION_ROLE" on "PERMISSIONS"."ID" = "PERMISSION_ROLE"."PERMISSION_ID" where "ROLES"."ID" = "PERMISSION_ROLE"."ROLE_ID") as "PERMISSIONS_COUNT" from (select "ROLES".*, row_number() over (order by "ID" desc) as rn from (select "ROLES".*, (select count(*) from "USERS" inner join "ROLE_USER" on "USERS"."ID" = "ROLE_USER"."USER_ID" where "ROLES"."ID" = "ROLE_USER"."ROLE_ID") as "USERS_COUNT", (select count(*) from "PERMISSIONS" inner join "PERMISSION_ROLE" on "PERMISSIONS"."ID" = "PERMISSION_ROLE"."PERMISSION_ID" where "ROLES"."ID" = "PERMISSION_ROLE"."ROLE_ID") as "PERMISSIONS_COUNT" from "ROLES" order by "ID" desc) "ROLES") "ROLES" where rn between 1 and 10
|
I had to wait until we had our dev environment available to test... unfortunately, the pagination was still slow with this version. This query:
with 800K rows took 23 seconds. |
@mixaster do you have indexes on that table? Curious to see if/how it changes things. Our tables were 5M or so. |
I do have indexes. it's the inner select I think that's causing the slowdown. |
Oic, we could trim down the query to a single inner select. Something like: select *
from (select users.*, row_number() over (order by rowid) as rn from USERS) users
where rn between 1 and 10 I will try to implement it. @mixaster, can you confirm if this trimmed query will improve your use case? |
There's still a full scan, so in my test of the raw query it took over 14 seconds. |
Apologies, I accidentally tagged v11.6.1 on this branch. On the good side, this PR got tested and seemed to break #891. |
Improve pagination performance by using ROW_NUMBER() and handling default orders.
fix: #878, #870
test: pagination improvement SQL changes
fix: subSelect where in query
fix: pagination when using fromSub