Skip to content
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

Issue 1931: Pagination not working properly for combined filters list. #1932

Merged
merged 4 commits into from
May 10, 2024

Conversation

kaladay
Copy link
Contributor

@kaladay kaladay commented May 7, 2024

resolves #1931

This is a regression from the aggressive performance optimizations such as commit b2c538c.

The case that is not correctly handled is where there are multiple separate field values being selected and filtered upon.

Instead of fully removing the optimization, this uses the optimization when there are zero or one field_values being filtered on. Then when there are two or more field_values then omit the optimization.

This is a regression from the aggressive performance optimizations such as commit b2c538c.

The case that is not correctly handled is where there are multiple separate field values being selected and filtered upon.

This reverts the problematic optimization to ensure that the generated queries are correct.
This has a cost of losing a significant performance optimization.
The performance optimization will have to be re-looked at and a more correct approach without any regressions will need to be figured out.
@kaladay kaladay requested review from smutniak and cstarcher May 7, 2024 18:31
smutniak
smutniak previously approved these changes May 8, 2024
kaladay added 2 commits May 8, 2024 15:07
…ers list."

This reverts commit 5ce9f38.

The plan is to go with a smarter design that conditionally uses the aggressive query optimization.
This is a regression from the aggressive performance optimizations such as commit b2c538c.

The case that is not correctly handled is where there are multiple separate field values being selected and filtered upon.

The solution here is different from the previous solution in commit 5ce9f38.

This approach preserves the aggressive optimization but when there are more than one filter values being filter it then disables the filter.
This allows for using the aggressive optimization whenever possible.
This uses the basic non-optimized select query (with a count) when there are two or more field values.
Copy link
Contributor

@smutniak smutniak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still working

@smutniak smutniak merged commit 9bc6886 into main May 10, 2024
3 of 5 checks passed
@kaladay kaladay deleted the 1931-pagination_embargo_graduation branch May 13, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pagination not working properly for combined filters list (embargo and graduation semester)
2 participants