Skip to content

Fix: scope to_arrow / each_record_batch to ADBC connections - #21

Closed
suaron wants to merge 1 commit into
red-data-tools:mainfrom
suaron:fix_to_arrow_each_record_batch
Closed

Fix: scope to_arrow / each_record_batch to ADBC connections#21
suaron wants to merge 1 commit into
red-data-tools:mainfrom
suaron:fix_to_arrow_each_record_batch

Conversation

@suaron

@suaron suaron commented May 8, 2026

Copy link
Copy Markdown
Contributor

Summary

to_arrow and each_record_batch were leaking onto every
ActiveRecord::Relation. Any model not backed by ADBC crashed with
NoMethodError: undefined method 'attach_model' for an instance of ActiveRecord::Result. That made this gem unsafe to drop into a Rails

Both methods now check the model's connection. ADBC keeps the fast
path. Anything else calls super, so red-arrow-activerecord can
take over if it's loaded; otherwise you get a NoMethodError that
actually names the problem instead of an attach_model traceback.

The module is prepended instead of included, so the guard wins
method lookup whichever order the gems get required in.

`ActiveRecord::Relation` and called
`ActiveRecordADBCAdapter::Result#attach_model` on whatever the query
returned. For a model backed by Postgres, MySQL, or SQLite3 that
result is a vanilla `ActiveRecord::Result`, which has no
`attach_model`, so `to_arrow` blew up:

NoMethodError: undefined method `attach_model' for an instance of
ActiveRecord::Result

Both methods now check the model's connection. ADBC keeps the fast
path. Anything else calls `super`, so `red-arrow-activerecord` can
take over if it's loaded; otherwise you get a `NoMethodError` that
actually names the problem.

Module is `prepend`ed instead of `include`d so the guard wins method
lookup whichever order the gems get required in.

Signed-off-by: Dmytro Piliugin <dmytro.piliugin@gmail.com>
@kou

kou commented May 8, 2026

Copy link
Copy Markdown
Member

I can understand your concern but when/why do you call to_arrow/each_record_batch against non ADBC backed models?

@suaron

suaron commented May 9, 2026

Copy link
Copy Markdown
Contributor Author

activerecord-adbc-adapter is meant to be used inside Rails apps, so a mixed-adapter setup is the expected case, not the exception - Postgres/SQLite as primary DB + ADBC for analytics/warehouse models living side-by-side.

From there, Querying.delegate(:to_arrow, :each_record_batch, to: :all) puts these methods on every ActiveRecord::Relation, not only ADBC-backed ones.

So any code that checks relation.respond_to?(:to_arrow) before calling it gets true from every model, then crashes inside attach_model. Also red-arrow-activerecord coexistence - both gems define to_arrow; prepend + super keeps the ADBC fast path and delegates to red-arrow when it's loaded.

Once delegated globally, the gem owns the behavior on models it doesn't.

@kou

kou commented May 22, 2026

Copy link
Copy Markdown
Member

How about #22?
#22 requires red-arrow-activerecord and integrates with red-arrow-activerecord well.

@suaron

suaron commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

#22 works for me — it's a better fix than mine. Comparing adapter_class instead of checking out a connection is cleaner, and depending on red-arrow-activerecord directly makes the super path reliable.

Closing this in favor of #22.

@suaron suaron closed this Jun 9, 2026
@suaron
suaron deleted the fix_to_arrow_each_record_batch branch June 9, 2026 11:14
kou added a commit that referenced this pull request Jun 10, 2026
Fix GH-21

Reported by Dmytro Piliugin. Thanks!!!
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