Repository navigation
Fix Arel visitor compatibility with Rails 8.1 - #284
Open
Krzysztof Adamski (rience) wants to merge 5 commits into
Open
Krzysztof Adamski (rience) wants to merge 5 commits into
Krzysztof Adamski (rience) wants to merge 5 commits into
Conversation
…h annotation gem) (#2)
Fix update_all / delete_all for rails 7.2
Rails 8.1 removed the `alias` attribute from `Arel::Nodes::Function` (and its subclasses like NamedFunction, Count, etc.). Guard `visit obj.alias` calls with `respond_to?` checks to maintain compatibility with both Rails 8.0 and 8.1. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Krzysztof Adamski (@rience) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
This was referenced Jul 3, 2026
Jackson Miller (jaxn)
added a commit
to ResaleAI/activerecord-multi-tenant
that referenced
this pull request
Sep 5, 2026
The workflow exported APPRAISAL but nothing consumed it, so every matrix job ran the default Gemfile: Ruby 3.0/3.1 resolved Rails 7.2 and passed, Ruby 3.2/3.3 resolved Rails 8.1 and failed on the unfixed upstream citusdata#284 Arel visitor issue. Each job now runs `bundle exec appraisal <name> rake spec` after generating and installing that appraisal's gemfile, and the job name uses matrix.appraisal (matrix.gemfile never existed). Matrix trimmed to what this fork ships for: rails/active-record 7.1, 7.2 and 8.0 on Ruby 3.1-3.3 x Citus 10-12, excluding Ruby 3.1 x 8.0 (Rails 8.0 needs Ruby >= 3.2). Ruby 3.0 and the 6.x/7.0 appraisals are dropped. static-checks used `gem install rubocop` (latest) and failed on 45 upstream offenses; rubocop is now pinned to ~> 1.71.0 in the Gemfile, the newest line that passes on upstream code, and run via bundle exec. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Jackson Miller (jaxn)
added a commit
to ResaleAI/activerecord-multi-tenant
that referenced
this pull request
Sep 5, 2026
Upstream citusdata#284 is not sufficient for Rails 8.1. Beyond the Arel `alias` removal it fixes, ActiveRecord::Relation#build_arel changed signature again: < 7.2 build_arel(aliases = nil) >= 7.2 build_arel(connection, aliases = nil) >= 8.1 build_arel(aliases) (activerecord-8.0.5.1/lib/active_record/relation/query_methods.rb:1750 vs activerecord-8.1.3.1/.../query_methods.rb:1750; 8.1's only caller is :1596, `@arel ||= build_arel(aliases)`, where 8.0 had `with_connection { |c| build_arel(c, aliases) }`.) generate_in_condition_subquery still took the 7.2 branch on 8.1, passing the adapter where `aliases` is expected. build_joins then treats it as a hash and raises: NoMethodError: undefined method `[]=' for an instance of ActiveRecord::ConnectionAdapters::PostgreSQLAdapter That is 4 failures in `rake spec` on the rails-8.1 appraisal (query_rewriter_spec :18, :26, :46, :133). Adding the 8.1 branch takes it to 1. MultiTenant's own `QueryMethodsExtensions#build_arel(*)` (query_rewriter.rb:289) already uses a splat and needs no change. Verified locally on Ruby 3.3.11 against Citus 12: rails-8.0 appraisal: 105 examples, 0 failures (before and after) default Gemfile (Rails 8.1.3.1): 105 examples, 4 failures -> 1 failure The remaining failure is a separate, non-gem issue: Rails 8.1 emits `UPDATE .. FROM .. WHERE` with an `__active_record_update_alias` self-join for joined update_all (arel/visitors/postgresql.rb:37-52), which Citus cannot plan ("complex joins are only supported when all distributed tables are co-located"). It is only reachable on the no-current-tenant path, where this extension delegates to super. Tracked separately; deliberately not papered over here. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
aliasattribute fromArel::Nodes::Functionand its subclasses (NamedFunction,Count,Avg,Max,Min,Sum,Exists)ArelVisitorsDepthFirstvisitor callsvisit obj.aliason these nodes, causingNoMethodError: undefined method 'alias'visit obj.aliascalls withrespond_to?(:alias)to support both Rails 8.0 and 8.1Test plan
🤖 Generated with Claude Code