Bug: Keyset pagination on date columns behave incorrectly #749

maikdijkstra · 2024-11-15T15:52:55Z

👀 Before submitting...

I upgraded to pagy version 9.2.2
I searched through the Documentation
I searched through the Issues
I searched through the Q&A

🧐 REQUIREMENTS

I am providing a VALID code file that confirms the bug
I am NOT posting any of the USELESS THINGS listed above
I am aware that this issue will be automatically closed if the code file is missing or INVALID

💬 Description

When using the new keyset pagination feature, I noticed an issue with paginating a query which first orders on a possibly non-unique timestamp like created_at and then a unique identifier like id. Together, this ordering is unique. However, due to the truncation of the timestamp when creating @pagy.next the pagination is not correct and some records appear on multiple pages.

keyset_ar.ru:

# frozen_string_literal: true

# DESCRIPTION
#    Showcase the keyset ActiveRecord pagination
#
# DOC
#    https://ddnexus.github.io/pagy/playground/#5-keyset-apps
#
# BIN HELP
#    bundle exec pagy -h
#
# DEV USAGE
#    bundle exec pagy clone keyset_ar
#    bundle exec pagy ./keyset_ar.ru
#
# URL
#    http://0.0.0.0:8000

VERSION = '9.2.2'

# Bundle
require 'bundler/inline'
require 'bundler'
Bundler.configure
gemfile(ENV['PAGY_INSTALL_BUNDLE'] == 'true') do
  source 'https://rubygems.org'
  gem 'activerecord'
  gem 'puma'
  gem 'sinatra'
  gem 'sqlite3'
end

# Pagy initializer
require 'pagy/extras/limit'
require 'pagy/extras/keyset'
require 'pagy/extras/pagy'
Pagy::DEFAULT[:limit] = 10
Pagy::DEFAULT.freeze

# Sinatra setup
require 'sinatra/base'
# Sinatra application
class PagyKeyset < Sinatra::Base
  include Pagy::Backend
  # Root route/action
  get '/' do
    Time.zone = 'UTC'

    @order = { created_at: :asc, id: :asc }
    @pagy, @lists = pagy_keyset(List.order(@order))
    erb :main
  end

  helpers do
    include Pagy::Frontend

    def order_symbol(dir)
      { asc: '&#x2197;', desc: '&#x2198;' }[dir]
    end
  end

  # Views
  template :layout do
    <<~ERB
      <!DOCTYPE html>
      <html lang="en">
      <html>
      <head>
         <title>Pagy Keyset App</title>
        <meta name="viewport" content="width=device-width, initial-scale=1.0">
        <style type="text/css">
          @media screen { html, body {
            font-size: 1rem;
            line-height: 1.2s;
            padding: 0;
            margin: 0;
          } }
          body {
            background: white !important;
            margin: 0 !important;
            font-family: sans-serif !important;
          }
          .content {
            padding: 1rem 1.5rem 2rem !important;
          }

          <%= Pagy.root.join('stylesheets', 'pagy.css').read %>
        </style>
      </head>
      <body>
        <%= yield %>
      </body>
      </html>
    ERB
  end

  template :main do
    <<~ERB
      <div class="content">
        <h1>Pagy Keyset App</h1>
        <p>Self-contained, standalone app usable to easily reproduce any keyset related pagy issue with ActiveRecord sets.</p>
        <p>Please, report the following versions in any new issue.</p>
        <h2>Versions</h2>
        <ul>
          <li>Ruby:    <%= RUBY_VERSION %></li>
          <li>Rack:    <%= Rack::RELEASE %></li>
          <li>Sinatra: <%= Sinatra::VERSION %></li>
          <li>Pagy:    <%= Pagy::VERSION %></li>
        </ul>
        <%= Base64.urlsafe_decode64(@pagy.next) %>
        <h3>Collection</h3>
        <div id="records" class="collection">
        <table border="1" cellspacing="0" cellpadding="3">
          <tr>
            <th>name</th>
            <th>created_at</th>
          </tr>
          <% @lists.each do |list| %>
          <tr>
            <td><%= list.name %></td>
            <td><%= list.created_at.inspect %></td>
          </tr>
          <% end %>
        </table>
        </div>
        <p>
        <nav class="pagy" id="next" aria-label="Pagy next">
          <%= pagy_next_a(@pagy, text: 'Next page &gt;') %>
        </nav>
      </div>
    ERB
  end
end

# ActiveRecord setup
require 'active_record'
# Log
output = ENV['APP_ENV'].equal?('showcase') ? IO::NULL : $stdout
ActiveRecord::Base.logger = Logger.new(output)
# SQLite DB files
dir = ENV['APP_ENV'].equal?('development') ? '.' : Dir.pwd  # app dir in dev or pwd otherwise
abort "ERROR: Cannot create DB files: the directory #{dir.inspect} is not writable." \
  unless File.writable?(dir)
# Connection
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: "#{dir}/tmp/pagy-keyset-ar.sqlite3")
# Schema
ActiveRecord::Schema.define do
  create_table :lists, force: true do |t|
    t.string :name
    t.timestamps
  end

  create_table :items, force: true do |t|
    t.string :name
    t.references :list
    t.timestamps
  end
end

# Models
class List < ActiveRecord::Base; end

# Seed
for i in 1..100
  list = List.create!(name: "List #{i}")
end


run PagyKeyset

The text was updated successfully, but these errors were encountered:

ddnexus · 2024-11-16T01:10:37Z

@maikdijkstra Thanks for you report.

That is a documented quirk of SQLite that should not happen with other DBs with proper time types.

The serialialization of the page param uses to_json, that misses the microseconds, and the comparison of time columns in SQLite (that includes microseconds by default) is alphanumeric, hence the ordering mismatch.

Not sure we can consider it a pagy bug: it looks more like a specific inconsistency of ActiveRecord to me ATM.

ddnexus · 2024-11-16T04:56:55Z

Not a bug indeed, you can encode the right json time values for your SQLite specific storage, by setting the precision to microseconds right after the require activerecord:

ActiveSupport::JSON::Encoding.time_precision = 6

I will add that to the keyset_a.ru, in order to avoid similar cases.

ddnexus · 2024-11-16T05:00:31Z

Thank you for you report.

…749)

ddnexus · 2024-11-17T02:59:51Z

I didn't find any simple way to configure the to_json to fix the precision if you don't use AR, so I added the broader option to customize the json used to encode the next, also available to Sequel::Dataset sets.

Released in 9.3

maikdijkstra added the bug label Nov 15, 2024

ddnexus added invalid and removed bug labels Nov 16, 2024

ddnexus closed this as completed Nov 16, 2024

ddnexus added a commit that referenced this issue Nov 16, 2024

Suggest precision fix for keyset_ar.ru time columns in SQLite DBs (#749)

0dd749f

ddnexus added a commit that referenced this issue Nov 17, 2024

Add the :jsonify_keyset_attributes variable to override the encoding (#…

8a9ae6f

…749)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug: Keyset pagination on date columns behave incorrectly #749

Bug: Keyset pagination on date columns behave incorrectly #749

maikdijkstra commented Nov 15, 2024

ddnexus commented Nov 16, 2024 •

edited

Loading

ddnexus commented Nov 16, 2024 •

edited

Loading

ddnexus commented Nov 16, 2024

ddnexus commented Nov 17, 2024 •

edited

Loading

Bug: Keyset pagination on date columns behave incorrectly #749

Bug: Keyset pagination on date columns behave incorrectly #749

Comments

maikdijkstra commented Nov 15, 2024

👀 Before submitting...

🧐 REQUIREMENTS

💬 Description

ddnexus commented Nov 16, 2024 • edited Loading

ddnexus commented Nov 16, 2024 • edited Loading

ddnexus commented Nov 16, 2024

ddnexus commented Nov 17, 2024 • edited Loading

ddnexus commented Nov 16, 2024 •

edited

Loading

ddnexus commented Nov 16, 2024 •

edited

Loading

ddnexus commented Nov 17, 2024 •

edited

Loading