aarondfrancis / fast-paginate

A fast implementation of offset/limit pagination for Laravel.
MIT License
1.21k stars 56 forks source link

Unnecessary condition in the last query #41

Closed crazynds closed 1 year ago

crazynds commented 1 year ago

Problem

The last query of fast paginations has unnecessary conditions. The last query will search the rows by id, so all the others conditions is redundant.

My case

I have a query with the fast pagination that do the follow sql:

select count(*) as aggregate from "questions" where "title"::text LIKE ?
select "questions"."id" from "questions" where "title"::text LIKE ? limit 20 offset 0 
select * from "questions" where "title"::text LIKE ? and "questions"."id" in (2) limit 21 offset 0 

My controller function:

public function index(IndexRequest $request)
{
  $query = Question::query();

  $query->where('title','LIKE','%fav%');

  return QuestionResource::collection($query->fastPaginate($qtd));
}

The last query that in the where condition is redundant because the previous query already returned the ids with the filter applied. So the part where "title"::text LIKE ? in the last query needs to be removed.

Why this change

The performance inpact will be irrelevant, but for debuging the sql this repeated where is so much confusing and makes you think the redundant code was made by you. In the end a lot of time is lost to find out that the redundant part is from the library.

I also think that queries are much more readable by removing these irrelevant conditions. I have a log of my database, and i was wondering why for each index he made 3 queries with all the conditions.

aarondfrancis commented 1 year ago

Thanks for the feedback! I don't think I'll be making this change. I can see it being confusing regardless of which way we do it. I.e. if I remove the where condition on the outer query people may think that something has gone wrong. I prefer the less interventional approach.