Skip to content

Use adapter support flags instead of checking adapter type in INDEX INCLUDE tests #54532

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

Merged
merged 1 commit into from
Feb 15, 2025

Conversation

aidanharan
Copy link
Contributor

@aidanharan aidanharan commented Feb 14, 2025

Motivation / Background

This PR updates the INDEX INCLUDE tests in activerecord/test/cases/migration/index_test.rb to run based on support flags rather than the adapter type.

Detail

PostgreSQL is the only core adapter (PostgreSQL/MySQL/SQLite/Trilogy) that support INDEX INCLUDE. However, SQL Server also supports INDEX INCLUDE and I would like to reuse the Rails tests in ActiveRecord SQL Server Adapter instead of recreating them in the adapter.

The tests currently check if the adapter is PostgreSQL and that INDEX INCLUDE is supported. The check to see if the adapter is PostgreSQL is not needed.

Removing the PostgreSQL adapter check makes it necessary to check if PARTIAL INDEX is supported in a separate test.

Also, removed the following check as its a list of all the core adapters and so does nothing:

if current_adapter?(:SQLite3Adapter, :Mysql2Adapter, :TrilogyAdapter, :PostgreSQLAdapter)

The changes are small and better viewed in the diff with the Hide whitespace option enabled.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]

@aidanharan aidanharan changed the title Use adapter support flags instead of checking adapter type in index include tests Use adapter support flags instead of checking adapter type in INDEX INCLUDE tests Feb 14, 2025
@kamipo kamipo merged commit 3fd7d44 into rails:main Feb 15, 2025
3 checks passed
@aidanharan aidanharan deleted the index-test-index-include branch February 15, 2025 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants