Skip to content

Fix PostgreSQL compatibility issue with DISTINCT and ORDER BY #1582

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

EduardMcfly
Copy link

Problem

PostgreSQL requires that when using DISTINCT, all columns in the ORDER BY clause must also be present in the SELECT clause. This causes issues when using Ransack with DISTINCT queries that include sorting.

Solution

This PR adds a new DistinctSortsProcessor class that:

  • Automatically detects when a query uses DISTINCT with ORDER BY
  • Adds necessary SELECT clauses for sort columns with unique aliases
  • Updates the ORDER BY clause to use the aliased column names
  • Maintains backward compatibility with existing queries

Changes

  • New file: lib/ransack/distinct_sorts_processor.rb - Core processor logic
  • Modified: lib/ransack/search.rb - Integrates the processor into the result method
  • Modified: lib/ransack.rb - Adds require for the new processor
  • New file: spec/ransack/distinct_sorts_processor_spec.rb - Comprehensive test coverage

Testing

The implementation includes extensive test coverage for:

  • Different types of sort expressions (Arel nodes, strings)
  • Complex sort strings with NULLS clauses
  • Integration with existing Ransack functionality
  • Edge cases and error conditions

Compatibility

This change is fully backward compatible and only activates when both DISTINCT and ORDER BY are present in a query.

Closes #429

- Add DistinctSortsProcessor to handle DISTINCT queries with ORDER BY clauses
- Fixes issue where PostgreSQL requires all ORDER BY columns to be in SELECT clause
- Automatically adds necessary SELECT clauses and aliases for sort columns
- Includes comprehensive test coverage for the new functionality
- Maintains backward compatibility with existing queries

Closes activerecord-hackery#429
- Detect existing SELECT expressions and reuse their aliases
- Refactor regex patterns and add helper methods
- Optimize performance by avoiding unnecessary query modifications
- Introduced a helper method to create distinct queries, improving readability.
- Updated test cases to use the new helper method for consistency.
- Refactored assertions to streamline the testing process and enhance clarity.
- Added new tests for processing sorts and extracting sort expressions.
- Improved handling of empty sorts and existing select aliases.

This refactor aims to enhance the maintainability and readability of the test suite for the DistinctSortsProcessor.
- Introduced `SqlExpressionParser` for splitting complex SQL expressions while respecting quotes and brackets.
- Refactored `process_sorts` method in `DistinctSortsProcessor` to handle string sorts and utilize the new parser.
- Improved handling of select values and sort expressions to enhance clarity and maintainability.
- Added comprehensive tests for the new SQL expression parsing functionality.

This update enhances the flexibility and robustness of sort processing in Ransack.
- Updated methods to enhance clarity and maintainability, including renaming `extract_sort_expression` to `extract_sort` and introducing a new `extract_sort_expression` method.
- Improved the `build_select_value` method to streamline the process of building select values from sorts.
- Added tests for new and existing methods to ensure robust handling of various sort expressions, including strings and Arel nodes.
- Enhanced the `matches_select_expression?` method to better match select strings with sort expressions.

This refactor aims to improve the overall functionality and maintainability of the DistinctSortsProcessor.
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.

bug when sorting by association when using postresql with distinct(:true)
1 participant