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

Some important fixes #1166

Merged
merged 4 commits into from
Jan 24, 2025
Merged

Some important fixes #1166

merged 4 commits into from
Jan 24, 2025

Conversation

JesseChavez
Copy link
Contributor

Hi @enebo

The last fix is quite important since common queries would fail for example

find_by!(some_field: 1.4)

or other queries that use the statement cache.

No sure why the CI does not complete in github workflows, perhaps not enough resources or file descriptor limit, the CI completes fine in my local.

Perhaps we can branch off version 71 and release a RC version and start working in AR 7.2 in master branch.

@enebo
Copy link
Member

enebo commented Jan 23, 2025

@JesseChavez When I browse the diff github seems to inline test run errors into and I am seeing quite a bit of this:

NoMethodError: private method `hash' called for #<ActiveRecord::Relation::QueryAttribute:0x49da976>

I am guessing some external types are manually calling hash and it being marked private is generating those errors in your monkey patch?

@enebo
Copy link
Member

enebo commented Jan 23, 2025

@JesseChavez I guess branching would not hurt since we are so far behind and these are working a lot more than they were.

…tter compare objects

This a important since arel and AR commonly use array - operator
@JesseChavez
Copy link
Contributor Author

@enebo

I made the hash method public

@JesseChavez
Copy link
Contributor Author

Hi @enebo

Do you need help with branching off ?

I am thinking if we can extract the jdbc drivers as separate gems, just an idea

like this:

https://github.com/JesseChavez/jdbc-pgsql

or

https://github.com/JesseChavez/jdbc-mssql

I don't mean now, I mean for AR 8.0 and beyond

@enebo enebo merged commit a457e26 into jruby:master Jan 24, 2025
7 of 13 checks passed
@enebo
Copy link
Member

enebo commented Jan 24, 2025

@JesseChavez How would the jdbc driver differ from https://rubygems.org/gems/jdbc-postgres?

I will be making 71-stable today but I want to try and figure out what is wrong with Rails jobs on CI. I even notices sqlite3 on one job is grabbing Rails 8+ for some reason but none of them make it to the point where we get a summary.

@enebo
Copy link
Member

enebo commented Jan 24, 2025

ok sqlite3 figured out (not going to running commentary to a PR but since I immediately figured it out I figured I would share....it added main to the matrix and this no doubt worked until 8 where 9.4 stopped being a possibility.

@JesseChavez
Copy link
Contributor Author

@enebo Thanks for branching off now we can push fixes to support rails 7.2

https://github.com/JesseChavez/jdbc-pgsql is same as https://rubygems.org/gems/jdbc-postgres however I have control of it.

I don't use the gems

activerecord-jdbcmysql-adapter
activerecord-jdbcpostgresql-adapter
activerecord-jdbcsqlite3-adapter

My setup is something like:

platforms :jruby do
  gem 'activerecord-jdbc-alt-adapter', '~> 71.0'
  gem "jdbc-mssql", "~> 12.6.0"
  gem "jdbc-pgsql", "~> 42.7.0"
  gem 'jruby-openssl'
...

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