Skip to content

Rails 8.0 Support, Postgres fixes - #1222

Open
JesseChavez wants to merge 36 commits into
jruby:masterfrom
JesseChavez:rails_80_fixes
Open

Rails 8.0 Support, Postgres fixes #1222
JesseChavez wants to merge 36 commits into
jruby:masterfrom
JesseChavez:rails_80_fixes

Conversation

@JesseChavez

@JesseChavez JesseChavez commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Hi @headius

Here is the work we have done, we are looking the failures of Postgres ARJDBC own tests failures.

We will also port the below changes that are in a old MR from us, we have that changes in production and they mean to be postgres fixes only but it seems they are sqlite and MySql

(bash)> git log -4
commit 8fca8db0990cd2f3fbf7a161b609aafdc2ed8f80 (HEAD -> 72-stable-prod-fixes, origin/72-stable-prod-fixes)
Author: Jesse Chavez <jesse.chavez.r@gmail.com>
Date:   Sat Dec 13 11:39:18 2025 +0900

    Fix queries like where(field: [""]) to match CRuby behaviour

commit aad30c3e4be23b0a63f533f56ae536d4e5f55ee2
Author: Jesse Chavez <jesse.chavez.r@gmail.com>
Date:   Fri Oct 17 22:40:46 2025 +0900

    Comment out code setting uuid type for plain varchar field

commit cf12d83fa383daa837f7e51dc5750dc840bfa1d3
Author: Jesse Chavez <jesse.chavez.r@gmail.com>
Date:   Sun Sep 28 12:50:37 2025 +0900

    Postgres, pass type casted binds for select queries

This reverts commit 2281241.

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
…to deadlocks

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
… PG gem

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
…orks

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
…for index-based lookup for BSON access

Mirrors upstream PG commit 100c86c00e

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
…ing_to_time

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
… numeric types

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
…handling

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
…nc_load_has_many

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
…ck_if_write_query and :mark_transaction_written_if_write

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Now mirrors built-in exec_insert, so it returns more than just the PK

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
…t_sql from Postgres adapter

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
…ostgres adapter

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
…rsion_to_default_timezone_utc

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
…to Arel bindings differing

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
… 10.0.5.0

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
…e for a lack of a GIL

Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
Signed-off-by: Jesse Chavez <jesse.chavez.r@gmail.com>
@headius

headius commented Aug 5, 2026

Copy link
Copy Markdown
Member

Ok finally getting back to this. There's a lot here to review.

@headius headius left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thankfully most of this is very small commits and they are largely reasonable. I have submitted some change request comments that should be addressed, especially any "hack" commits that admit they are poor solutions (let's iterate on those separately if they require more work).

As a general note, I'd like to avoid having to copy code from Rails every version, so any places where we are duplicating their code we should try to get them to move into a module. There's several such cases in this PR.

We could probably merge this and it would work great, but I'd like @JesseChavez to take a pass at addressing my comments and pull any especially hacky or inefficient commits out to separate PR(s).


private final CachingCallSite type_cast_site = new FunctionalCachingCallSite("type_cast");

private Object[] typeCastArrayValues(final ThreadContext context, final IRubyObject adapter, final RubyArray<?> values) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change concerns me; I'd like to see the equivalent code in the PG gem to be sure we're not doing excessive work here.

import org.jruby.RubyModule;
import org.jruby.RubyNumeric;
import org.jruby.RubyString;
import org.jruby.*;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Style concern: prefer explicit imports until there's at least ten of them.

processResultSet(context, resultSet);
}

// HACK: Needed for postgres to be able to return a sane result type instead of just a RubyFixnum

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Need to figure out something better than a HACK for this. Note also concerns in the same commit about this not being the best impl. Let's figure out the best impl.

require "active_model/type/helpers/time_value"

# The caller only handles ArgumentError as a failure,
# but JRuby raises TypeError for invalid formats in Time.new() (non-standard),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd like to have a JRuby bug reference here so we can revert this patch when JRuby gets fixed.

@@ -0,0 +1 @@
exclude :test_url_invalid_adapter, "sqlserver is not a built-in adapter, so rails complains when validating the error"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Very annoying that the Rails tests implicitly require there be no other adapters. This warrants a patch to Rails imho.

@@ -1,2 +1,3 @@
exclude :test_some_time, 'intermittent failures, leaks thread, fires at high frequency'
exclude :test_connection_pool_starts_reaper, 'intermittent failures, leaks thread, fires at high frequency'
exclude :test_reaper_works_after_pool_discard, 'deadlocks under JDBC due to high frequency discard'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we know why this deadlocks? Of course we have lots of excludes but this seems like one we should try to fix rather than excluding.

Comment thread Gemfile
gem 'minitest-rg', require: nil
gem 'minitest-rails', require: nil
gem 'benchmark-ips', require: nil
gem 'rdoc', '~> 7', require: false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be temporary; we need to work with rbs folks to get a release out with JRuby support.

@headius

headius commented Aug 5, 2026

Copy link
Copy Markdown
Member

Also a general note: it's hard to evaluate this with all the 8.0 jobs failing.

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.

3 participants