Negative padding returns 0 objects if number of objects is less than negative padding value

[yo-gen]

For eg. if I have 3 objects in array.

my_array = [1,2,3]

Then, Kaminari.paginate_array(my_array).page(1).per(10).padding(-5)
returns count as 0, and 0 objects in the result.
It should return back count as 3 and results as [1,2,3]

I am using
kaminari-0.17.0
rails 4.2.7.1
ruby-2.3.1

[amatsuda]

@yo-gen Thank you, but I'm wondering what actually does a negative padding mean.
Was there any actual use case for a negative padding in your app, or would adding a guard for padding() not to accept a negative number satisfy you?

[yo-gen]

Yes. We use it in our app. Our requirement is to fetch 5 videos in the first call. And in subsequent page calls we want 10 videos to be fetched.
So the following call:
Kaminari.paginate_array(@channel_videos).page(params[:page]).per(10).padding(-5) works fine, except for when @channel_videos has less than 5 videos and params[:page] =1

[yuki24]

@yo-gen Well, we are asking because we want to understand why you have that requirement. From a user's perspective, it's extremely weird to see a smaller number of elements only on the first page.

Also, please format your comment properly. I personally consider unformatted comments to be a rude attitude.

[yo-gen]

@yuki24 I am sorry. I had no intentions of being rude. I am a Github starter and haven't ever used it actively. So, please tell me the problems with my formatting of the comments and I'll take care henceforth. Thanks.
About our requirement, we make the first call, when the user opens our app and also every time when the user comes back to the Home tab which is very frequent.
This call is very heavy as we send data of a list of videos and a lot of associated products. To reduce the response time for our first call, we thought of requesting 5 elements for first call and if user goes on to scroll below then we fetch 10 videos from there on.

[yuki24]

@yo-gen I've formatted your comments above. You can use GitHub Flavored Markdown almost anywhere on GitHub so others can easily read comments.

Speaking of your requirement, it sounds more like a performance issue, and you are just trying to work around it, and I'm not sure if we should change kaminari just for your edge case.

[yo-gen]

@yuki24 Alright. We had already implemented our case by calling per_page=5 & padding = 0 on first call and per_page = 10 & padding = -5 from the second call.

However I thought this was some small bug in Kaminari as it returned correct value with negative_padding if number_of_elements > padding and returned 0 objects for number_of_elements < padding which did not seem like intended behaviour.
So I had posted this issue, more from a consistency perspective.
You can close it if you find it irrelevant.
Thanks.

[amatsuda]

I'm not strongly against the proposed behavior, but there exists a problem regarding negative padding.

I investigated this issue a little bit, and found that negative padding with the Array extension is completely broken. It actually returns such a funny result:

Kaminari.paginate_array([*1..10]).page(1).per(5).padding(-2)
#=> [9, 10]

OTOH passing a negative number to padding() on an AR relation performs very differently. Actually it differs even between the adapters:

• SQLite3

User.page(1).per(5).padding(-2)
 User Load (0.2ms)  SELECT  "users".* FROM "users" LIMIT ? OFFSET ?  [["LIMIT", 5], ["OFFSET", -2]]
#=> [1, 2, 3, 4, 5]

• PostgreSQL

User.page(1).per(5).padding(-2)
  User Load (4.7ms)  SELECT  "users".* FROM "users" LIMIT $1 OFFSET $2  [["LIMIT", 5], ["OFFSET", -2]]
ActiveRecord::StatementInvalid: PG::InvalidRowCountInResultOffsetClause: ERROR:  OFFSET must not be negative

• MySQL

User.page(1).per(5).padding(-2)
  User Load (2.9ms)  SELECT  `users`.* FROM `users` LIMIT 5 OFFSET -2
ActiveRecord::StatementInvalid: Mysql2::Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '-2' at line 1: SELECT  `users`.* FROM `users` LIMIT 5 OFFSET -2

Based on this fact, I'm leaning towards rejecting negative padding, unless @yo-gen or anyone else submits a patch that nicely fixes the inconsistency between these adapters and Array.

[amatsuda]

@yo-gen I just pushed 21baf16 that rejects negative padding for now as I stated in the last comment, because I couldn't come up with a nice solution that implements your proposal for both Active Record and Array.

Of course I'm open to accept your patch if you can solve this :)
Thanks!

[cthorner]

Just upgraded kaminari and this breaks our functionality of giving a negative offset when we delete an item from the list.

[yuki24]

@cthorner could you file a new issue with a use case where a negative offset is helpful? According to the conversation above, negative offsets could cause more trouble than being helpful.

[Tectract]

Just submitted PR 1004

#1004

I'm not the only whose app was broken by "disallowing" negative pagination... @cthorner