Fix async queries to work with query cache and other cached async queries

[fatkodima]

Fixes #50776.

[navidemad]

I tried your fix still same issue.
I found with that change it worked.

Suggested change

	if result.is_a?(FutureResult)
	if result.is_a?(FutureResult::Complete)

result.class
ActiveRecord::FutureResult::Complete

result
#<ActiveRecord::FutureResult::Complete:0x0000000142b451c0
 @result=
  #<ActiveRecord::Result:0x0000000141c96778
   @column_types=
    {0=>#<ActiveModel::Type::Integer:0x0000000142271508 @limit=8, @precision=nil, @range=-9223372036854775808...9223372036854775808, @scale=nil>,
     "count"=>#<ActiveModel::Type::Integer:0x0000000142271508 @limit=8, @precision=nil, @range=-9223372036854775808...9223372036854775808, @scale=nil>},
   @columns=["count"],
   @hash_rows=nil,
   @rows=[[70]]>>

result.is_a?(FutureResult)
false

result.is_a?(FutureResult::Complete)
true

[fatkodima]

Can you provide an example of what queries do you use that cause the issue?

[navidemad]

You can find a reproducible case there:
https://gist.github.com/navidemad/e4af727d335faa3148d4675e698cba4e

[fatkodima]

Can you test now?

[navidemad]

sorry for the delay, it works with your latest change

[franzliedke]

For consistency?

Suggested change

	assert_equal count.value, ShipPart.async_count.value
	assert_async_equal count.value, ShipPart.async_count

[fatkodima]

Will not work, because nil? method is not defined on the ActiveRecord::Promise.

[franzliedke]

Huh? If count.value returns a promise, something is wrong anyway, right? (Which is the bug we're talking about here.)

[fatkodima]

Yeah, thanks. I missed that the count.value stayed in your example, so it works.

[fatkodima]

We need to test for both classes, because when async is configured in the app - we have FutureResult::SelectAll class and when not, FutureResult::Complete.

See

rails/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb

Lines 609 to 636 in 25b7f64

	def select(sql, name = nil, binds = [], prepare: false, async: false)
	if async && async_enabled?
	if current_transaction.joinable?
	raise AsynchronousQueryInsideTransactionError, "Asynchronous queries are not allowed inside transactions"
	end
	future_result = async.new(
	pool,
	sql,
	name,
	binds,
	prepare: prepare,
	)
	if supports_concurrent_connections? && current_transaction.closed?
	future_result.schedule!(ActiveRecord::Base.asynchronous_queries_session)
	else
	future_result.execute!(self)
	end
	return future_result
	end
	result = internal_exec_query(sql, name, binds, prepare: prepare)
	if async
	FutureResult::Complete.new(result)
	else
	result
	end
	end

[franzliedke]

Good spot!

I don't suppose we can find a solution involving duck typing or something different more polymorphy-friendly? 🤔

At least we could move this if / else in a method within the FutureResult module, to make it more likely for this case to be added if another class is added there?

FutureResult.wrap(result)

[byroot]

Yes. Would be good to introduce a predicate that is common to all Result classes. e.g. Result#future?

[byroot]

But FutureResult.wrap is nice too.

[fatkodima]

Thank you for suggestions! ❤️ I implemented a FutureResult.wrap and also updated a few places in the codebase to use it.

[byroot]

Backported to 7-1 stable as 89e9753