Fix pluck to correctly type cast same column names and association columns
#39264
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…columns That issues are caused by using only the model's cast types on the relation. To fix that issues, use the attribute's type caster takes precedence over the model's cast types on the relation. Fixes rails#35232. Fixes rails#36042. Fixes rails#37484.
eileencodes
added a commit
to eileencodes/rails
that referenced
this pull request
May 14, 2020
The PR rails#39264 fixed `pluck` so that when plucking on a join the string columns correctly returned casted values rather than uncasted values. This led me to dig into other patterns to ensure the changes were correct. I discovered that 6.0 has been not-casting values for string pluck, plucks with Arel.sql and symbol plucks. While string and arel.sql plucks are fixed, symbol plucks still return uncasted values when called on the joined table. Note the columns are correctly casted for the Author table, but not for the books table. I'm not entirely sure how to fix this since Rails quotes the column name if it's a symbol so it has no type caster to cast on. This is a problem for all non-string values like boolean, enum, etc.
kamipo
added a commit
to kamipo/rails
that referenced
this pull request
May 14, 2020
…bles Follow up to rails#39264, and fixes demonstrated case in rails#39290. If the column has no type caster and the model don't know the attribute, let's will attempt to lookup cast type from join dependency tree.
cribbles
added a commit
to Shopify/identity_cache
that referenced
this pull request
Apr 5, 2023
Fixes a bug where `load_multi_rows` would break with the min-supported gemspec due to earlier versions of ActiveSupport's `#pluck` failing to accomodate redundant column names. Fixed in rails/rails#39264. This is principally an issue when running `fetch_multi` with a composite index including `id`. In these cases we simply exclude the redundant `id` from the `pluck` call and reintroduce it in the result at the same index.
cribbles
added a commit
to Shopify/identity_cache
that referenced
this pull request
Apr 5, 2023
Fixes a bug where `load_multi_rows` would break with the min-supported gemspec due to earlier versions of ActiveSupport's `#pluck` failing to accomodate redundant column names. Fixed in rails/rails#39264. This is principally an issue when running `fetch_multi` with a composite index including `id`. In these cases we simply exclude the redundant `id` from the `pluck` call and reintroduce it in the result at the same index.
cribbles
added a commit
to Shopify/identity_cache
that referenced
this pull request
Apr 5, 2023
Fixes a bug where `load_multi_rows` would break with the min-supported gemspec due to earlier versions of ActiveSupport's `#pluck` failing to accomodate redundant column names. Fixed in rails/rails#39264. This is principally an issue when running `fetch_multi` with a composite index including `id`. In these cases we simply exclude the redundant `id` from the `pluck` call and reintroduce it in the result at the same index.
cribbles
added a commit
to Shopify/identity_cache
that referenced
this pull request
Apr 5, 2023
Fixes a bug where `load_multi_rows` would break with the min-supported gemspec due to earlier versions of ActiveSupport's `#pluck` failing to accomodate redundant column names. Fixed in rails/rails#39264. This is principally an issue when running `fetch_multi` with a composite index including `id`. In these cases we simply exclude the redundant `id` from the `pluck` call and reintroduce it in the result at the same index.
cribbles
added a commit
to Shopify/identity_cache
that referenced
this pull request
Apr 5, 2023
Fixes a bug where `load_multi_rows` would break with the min-supported gemspec due to earlier versions of ActiveSupport's `#pluck` failing to accomodate redundant column names. Fixed in rails/rails#39264. This is principally an issue when running `fetch_multi` with a composite index including `id`. In these cases we simply exclude the redundant `id` from the `pluck` call and reintroduce it in the result at the same index.
cribbles
added a commit
to Shopify/identity_cache
that referenced
this pull request
Apr 6, 2023
Fixes a bug where `load_multi_rows` would break with the min-supported gemspec due to earlier versions of ActiveSupport's `#pluck` failing to accomodate redundant column names. Fixed in rails/rails#39264. This is principally an issue when running `fetch_multi` with a composite index including `id`. In these cases we simply exclude the redundant `id` from the `pluck` call and reintroduce it in the result at the same index.
cribbles
added a commit
to Shopify/identity_cache
that referenced
this pull request
Apr 6, 2023
Fixes a bug where `load_multi_rows` would break with the min-supported gemspec due to earlier versions of ActiveSupport's `#pluck` failing to accomodate redundant column names. Fixed in rails/rails#39264. This is principally an issue when running `fetch_multi` with a composite index including `id`. In these cases we simply exclude the redundant `id` from the `pluck` call and reintroduce it in the result at the same index.
cribbles
added a commit
to Shopify/identity_cache
that referenced
this pull request
Apr 6, 2023
Fixes a bug where `load_multi_rows` would break with the min-supported gemspec due to earlier versions of ActiveSupport's `#pluck` failing to accomodate redundant column names. Fixed in rails/rails#39264. This is principally an issue when running `fetch_multi` with a composite index including `id`. In these cases we simply exclude the redundant `id` from the `pluck` call and reintroduce it in the result at the same index.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
That issues are caused by using only the model's cast types on the
relation.
To fix that issues, use the attribute's type caster takes precedence
over the model's cast types on the relation.
Fixes #35232.
Fixes #36042.
Fixes #37484.