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

Initial work to support Active Record 7.1 (Rails 7.1.x) #1150

Merged
merged 18 commits into from
Jul 25, 2024

Conversation

JesseChavez
Copy link
Contributor

Hi @headius

Here is some initial work to support active record 7.1.

  • More than 90% of ARJDBC tests pass in my local
  • The work for sqlite was already there
  • The jndi connection pool callbacks is disabled temporarily,we still to work on this
  • No sure if we need to update/fix .github/workflows/ruby.yml
  • I have not run the rails tests yet. I would assume will run.

I can still look at the failing tests and rails tests later the following weeks.

- .jrubyrc, to enable some extra debugging
- .solargraph.yml and .rubocop.yml, for lsp in my editor
- pik.sh, to pick my version of jruby to run test
…is later.

- it was disabled to so we can focus on fixing the AR integration first
- cannot run tests, process crashes
- it was disabled to so we can focus on fixing the core part first
- cannot run tests, process crashes
we needed to add new_client class method similar to active record CRuby
adapters so we can easily mock the raw_connection.
<ActiveRecord::StatementInvalid> expected but was
<ActiveRecord::JDBCError(<java.sql.SQLSyntaxErrorException: Table 'arjdbc_test.animals' doesn't exist>)
arjdbc/jdbc/RubyJdbcConnection.java:775:in `execute'
Loading development environment (Rails 7.0.8.4)
[1] pry(main)> t = Time.current
=> Thu, 25 Jul 2024 16:56:41.434382638 AEST +10:00
[2] pry(main)>
[3] pry(main)> t.to_s(:db)
DEPRECATION WARNING: TimeWithZone#to_s(:db) is deprecated. Please use TimeWithZone#to_fs(:db) instead. (called from require at bin/rails:4)
=> "2024-07-25 06:56:41"
https://github.com/rails/rails/blob/7-1-stable/activerecord/lib/active_record/connection_adapters/mysql2/database_statements.rb#L21

https://github.com/rails/rails/blob/7-1-stable/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb#L63

there is diference between mysql/sqllite and postgres internal_exec_query method arity
however due default values is not a issue at least for AR 7.1

some error example:

Error: test_schema_dump(PostgresSchemaDumpTest): ArgumentError: unknown keywords: :allow_retry, :materialize_transactions
/home/jessec/bryk/vendor_gems/stable/activerecord-jdbc-adapter/lib/arjdbc/abstract/database_statements.rb:37:in `internal_exec_query'
/home/jessec/.gem/jruby/3.1.4/gems/activerecord-7.1.3.4/lib/active_record/connection_adapters/postgresql/schema_statements.rb:549:in `foreign_keys'
error:

<ActiveRecord::StatementInvalid> expected but was
<ActiveRecord::JDBCError(<org.postgresql.util.PSQLException: ERROR: relation "animals" does not exist
  Position: 454>)
…ore creating a savepoint

change relates to
  rails/rails@3f19431

materilaization is trigger by executing any query inside the transaction

Cherry-picked from my fork
@enebo enebo merged commit 42a23bf into jruby:master Jul 25, 2024
1 of 13 checks passed
@enebo
Copy link
Member

enebo commented Jul 25, 2024

Hello @JesseChavez. This looks great. At one point sqlite3 was running on Rails AR tests but some change in AR itself broke that (just noting I was confused when this PR came in because I know it was running to completion -- not your problem though). You had to commented out things which should be deleted but it is not worth holding this up for that.

@JesseChavez JesseChavez deleted the rails_71_support branch July 31, 2024 22:16
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