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

chore: invoke deprecated methods in default supports method for backwards compatibility #5897

Merged
merged 1 commit into from
May 10, 2024

Conversation

filipelautert
Copy link
Collaborator

@filipelautert filipelautert commented May 10, 2024

TLDR:

Invoke deprecated methods in default supports method to ensure that extensions are not broken, but one day when those are removed it will return only true

Description

The new support method was introduced in PR #5619 so specially NoSQL databases can check for other supports as Collections without changing Database class and adding a supportsCollections() method.
In core all the locations were supportsSequences() existed were changed to supports(Sequence) , and all *Database classes had this new method implemented. But extensions don't have it, so they could return the wrong value. For instance, BigQuery doesn't support sequences but would start to return true to core.
With this fix we continue to return what is expected.

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

Copy link
Contributor

@StevenMassaro StevenMassaro left a comment

Choose a reason for hiding this comment

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

Code here looks fine but I don't know any of the context around this change.

@filipelautert
Copy link
Collaborator Author

Code here looks fine but I don't know any of the context around this change.

@StevenMassaro the new support method was introduced so specially NoSQL databases can check for other supports as Collections without changing Database class and adding a supportsCollections() method.
In core all the locations were supportsSequences() existed were changed to supports(Sequence) , and all *Database classes had this new method implemented. But extensions don't have it, so they could return the wrong value. For instance, BigQuery doesn't support sequences but would start to return true to core.
With this fix we continue to return what is expected.

I'm adding this info to the PR description, thanks!

Copy link
Contributor

@MalloD12 MalloD12 left a comment

Choose a reason for hiding this comment

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

LGTM

@filipelautert filipelautert merged commit 1bf155a into master May 10, 2024
38 of 40 checks passed
@filipelautert filipelautert deleted the fix-supports-method-for-extensions branch May 10, 2024 17:51
@filipelautert filipelautert added this to the 1NEXT milestone May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants