Defect #29581

Issues in paginated views may be lost because sorting criteria are not unique

Added by Mizuki ISHIKAWA about 1 year ago. Updated 5 months ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:Issues list
Target version:4.1.0
Resolution:Fixed Affected version:

Description

This is a problem that the issue that should be present is not displayed on the issues/index.
The same problem may exist even at index other than issues/index.

This problem occurs when using PostgreSQL.
If you use only non-unique fields such as trackers or categories as the sorting criteria, there is a possibility that some issues will not be displayed on pagination views.

This phenomenon is described in the document of PostgreSQL. (As a specification rather than a bug)
https://www.postgresql.org/docs/8.3/static/queries-limit.html

Questions about the same problem:
https://stackoverflow.com/questions/13580826/postgresql-repeating-rows-from-limit-offset

I attached file is the test I wrote to reproduce this problem.
If you are using PostgreSQL, that test will fail. ( Non-paginated issue ids and paginated issue ids should be the same. )

Failure:
--- expected
+++ actual
@@ -1 +1 @@
-[2, 12, 11, 8, 7, 5, 3, 1, 13]    # Non-paginated issue ids
+[11, 12, 12, 7, 7, 5, 3, 1, 13]  # Paginated issue ids

add-test-that-fails-when-using-postgresql.patch Magnifier (877 Bytes) Mizuki ISHIKAWA, 2018-09-10 07:35

fix-29581.patch Magnifier (3.36 KB) Mizuki ISHIKAWA, 2018-09-20 10:21

fix-29581-v2.patch Magnifier (3.55 KB) Mizuki ISHIKAWA, 2019-06-18 07:25

fix-29581-v3.patch Magnifier (3.67 KB) Mizuki ISHIKAWA, 2019-06-19 02:17


Related issues

Duplicated by Redmine - Defect #31924: Paging misses some entries Closed

Associated revisions

Revision 18263
Added by Jean-Philippe Lang 5 months ago

Issues in paginated views may be lost because sorting criteria are not unique (#29581).

Patch by Mizuki ISHIKAWA.

History

#1 Updated by Mizuki ISHIKAWA about 1 year ago

I wrote a patch to solve this problem.

I fixed to add unique fields (ex: issues.id, time_entries.id) as sort criteria.

#2 Updated by Go MAEDA about 1 year ago

  • Target version set to 4.1.0

Setting target version to 4.1.0.

#3 Updated by vzvu 3k6k 5 months ago

order_option += ['issues.id ASC'] unless order_option.include?("issues.id DESC") || order_option.include?("issues.id ASC")

Is it ok to use `issues.id ASC` as a default implicit order?

IssueQuery#default_sort_criteria uses issues.id DESC.

(Pair-reviewed with maimai77)

#4 Updated by Mizuki ISHIKAWA 5 months ago

vzvu 3k6k wrote:

order_option += ['issues.id ASC'] unless order_option.include?("issues.id DESC") || order_option.include?("issues.id ASC")

Is it ok to use `issues.id ASC` as a default implicit order?

IssueQuery#default_sort_criteria uses issues.id DESC.

(Pair-reviewed with maimai77)

Thank you for reviewing fix-29581.patch.

As you point out, it seems natural to use the same sort criteria as IssueQuery#default_sort_criteria.
I have attached the file to fixed patch.

#5 Updated by vzvu 3k6k 5 months ago

Thank you for your response! Your v2 patch looks good to me.

#6 Updated by Seiei Miyagi 5 months ago

'issues.id DESC'

Is it OK to write table name of the Issue model directly?
In app/models/issue_query.rb, It seems code like following is more preferable.

"#{Issue.table_name}.id DESC" 

#7 Updated by Mizuki ISHIKAWA 5 months ago

Seiei Miyagi wrote:

'issues.id DESC'

Is it OK to write table name of the Issue model directly?
In app/models/issue_query.rb, It seems code like following is more preferable.

[...]

Thank you for pointing it out.
I changed the way of writing table names.

#8 Updated by Jean-Philippe Lang 5 months ago

  • Status changed from New to Closed
  • Assignee set to Jean-Philippe Lang
  • Resolution set to Fixed

Patch committed, thanks.

#9 Updated by Go MAEDA 3 months ago

Also available in: Atom PDF