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

Check subtype limit before using the default limit #19447

Merged

Conversation

jacob-daniel-waller
Copy link
Contributor

As described here #19420. When using the Postgres BigInt[] field type the big int value was not being translated into schema.rb. This caused the field to become just a regular integer field when building off of schema.rb. This fix will addrees this by checking the subtype's limit first and if not found then defaulting to the limit. As in this case the subtype limit is correct.

#19420

@@ -560,7 +560,7 @@ def fetch_type_metadata(column_name, sql_type, oid, fmod)
simple_type = SqlTypeMetadata.new(
sql_type: sql_type,
type: cast_type.type,
limit: cast_type.limit,
limit: cast_type.try(:subtype).try(:limit) || cast_type.limit,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think (cast_type.subtype ? cast_type.subtype.limit : cast_type_limit) is preferable over try here (unless cast_type does not always respond to .subtype)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, please avoid using try in the Rails codebase. Please also avoid ternaries.

@sgrif
Copy link
Contributor

sgrif commented Mar 22, 2015

Let's just delegate limit in the Array type rather than reaching into its internals

As described here rails#19420. When
using the Postgres BigInt[] field type the big int value was not being
translated into schema.rb. This caused the field to become just a
regular integer field when building off of schema.rb. This fix will
address this by delegating the limit from the subtype to the Array type.

rails#19420
@jacob-daniel-waller
Copy link
Contributor Author

@sgrif - Thanks for the tip! Much simpler this way also. Let me know if what I did was the proper fix.

sgrif added a commit that referenced this pull request Mar 22, 2015
…hema_rb

Check subtype limit before using the default limit
@sgrif sgrif merged commit 2271f7d into rails:master Mar 22, 2015
@sgrif
Copy link
Contributor

sgrif commented Mar 22, 2015

Thanks! <3

@rafaelfranca
Copy link
Member

Backported in 3a71142

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.

None yet

4 participants