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

fix: Rails 7.2 can't cast error. #404

Conversation

vicocamacho
Copy link

@vicocamacho vicocamacho commented Sep 2, 2024

As part of the updates made to the PostgreSQL adapter for Rails 7.2. Rails changed the argument order for instantiating column objects here https://github.com/rails/rails/blob/a11f0a63673d274c59c69c2688c63ba303b86193/activerecord/lib/active_record/connection_adapters/postgresql/column.rb#L9. Because the PostGIS adapter inherits from the PosgreSQL adapter it requires just a small update to the order of the arguments when instantiating new columns.

@vicocamacho vicocamacho changed the title fix: Rails 7.2 compatiblity update fix: Rails 7.2 can't cast error. Sep 2, 2024
@mjy
Copy link

mjy commented Sep 2, 2024

Order doesn't matter in this context, does it?

3.3.4 :009 > def foo(a,b, c: nil, d: 9,  **)
3.3.4 :010 >   puts a,b,c,d
3.3.4 :011 > end
 => :foo 
3.3.4 :012 > foo(1,2,d:22, c: 1)
1
2
1
22
 => nil 

@vicocamacho
Copy link
Author

Order doesn't matter in this context, does it?

No, it shouldn’t, you are correct. Then the underlying issue should be something else. The weird thing is that I have an internal CI that was having the errors reported on the issue referenced here and they stopped happening once I applied this patch.

I’ll keep investigating

@vicocamacho
Copy link
Author

vicocamacho commented Sep 2, 2024

nvm, this is not fixing the issue, closing for now. I will keep investigating. Sorry for the false alarm.

@vicocamacho vicocamacho closed this Sep 2, 2024
@vicocamacho vicocamacho deleted the fix/update-column-method-signature-for-rails-7-2 branch September 2, 2024 21:46
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.

2 participants