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

Adds Arel::FactoryMethods#cast(node, type) #48873

Merged
merged 2 commits into from Aug 4, 2023

Conversation

drnic
Copy link
Contributor

@drnic drnic commented Aug 2, 2023

For example:

  product_table = Product.arel_table
  product_table.cast(product_table[:position], "integer")

Produces SQL:

  CAST("products"."position" as integer)

Motivation / Background

CAST(field as type) is a widely supported SQL function. This PR adds native arel support for this named function with a cast(field, type) helper.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated 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]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

activerecord/test/cases/arel/factory_methods_test.rb Outdated Show resolved Hide resolved
activerecord/CHANGELOG.md Outdated Show resolved Hide resolved
For example:
  product_table = Product.arel_table
  product_table.cast(product_table[:position], "integer")

Produces SQL:
  CAST(position as integer)
@drnic
Copy link
Contributor Author

drnic commented Aug 3, 2023

I'll investigate the new build failures and make a fix

The result of the SQL is not the same on all databases, so writing a
test that would pass in all cases would mean conditionals here.

Since we already testing if the SQL results of a As node is correct in
the visitor tests, we only need to check if the As node attributes are
correct here.
@rafaelfranca
Copy link
Member

This error is just a quote discrepancy between the adapters. I pushed a commit to this PR with the fix. Since we are already testing the SQL result of visiting a AS node in the visitor tests, we can just assert if the attributes of the As node is correct in this test avoiding to have to add conditionals depending on the database adapter.

@drnic
Copy link
Contributor Author

drnic commented Aug 3, 2023

Gotcha. It seemed "right" to confirm the final SQL; but I was naive to assume all the adapters printed the same thing. Thanks for making time to fix and the explanations @rafaelfranca

@rafaelfranca rafaelfranca merged commit 2e3df04 into rails:main Aug 4, 2023
9 checks passed
@rafaelfranca
Copy link
Member

Thank you for implementing the feature

@drnic drnic deleted the arel-cast-function branch August 9, 2023 00:01
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.

None yet

2 participants