Rails 6: Coerce eagerload too many IDs test

[aidanharan]

Original Rails test fails with SQL Server error message "The query processor ran out of internal resources and could not produce a query plan". This error goes away if you change database compatibility level to 110 (SQL 2012). However, you cannot change the compatibility level during a test. The purpose of the test is to ensure that the bind params are not used if they exceed the bind_params_length of the adapter. The coerced test still does this as there will be 32,768 remaining citation records in the database (65,536 - 32,768) and the bind_params_length of the SQL Server adapter is 2,098.

Rails PR: rails/rails#33844

[wpolicarpo]

I have some concerns/suggestions I would like discuss and investigate:

1. Why was this test passing before? What changed in 6.0 that causes it to fail now?
2. If we really need to coerce this test, shouldn't we raise a more meaningful exception for when that happens in real life (outside that test case)?
3. Can we somehow use a strategy similar to Fix EagerLoadingTooManyIdsTest#test_preloading_too_many_ids #786 (probably not, but who knows)?

[aidanharan]

The query that is causing the issue has the IDs broken up into groups of 10,000:

SELECT COUNT(*) FROM (SELECT DISTINCT [citations].[id] FROM [citations] LEFT OUTER JOIN [citations] [citations_citations] ON [citations_citations].[citation_id] = [citations].[id] WHERE ([citations].[id] IN (0, 1, ..., 9999) OR [citations].[id] IN (10000, 10001, ..., 19999) OR [citations].[id] IN (20000, 20001, ..., 29999) OR [citations].[id] IN (30000, 30001, ..., 32767))) subquery_for_count

1. Why was this test passing before? What changed in 6.0 that causes it to fail now?
The test is already coerced in master. The coercion was removed recently from 6-0-dev in PR #793
as part of a cleanup. The purpose of the test is to check that an unprepared statement is being used when the number of values exceed the adapter's bind_params_length. I've updated the coerced test to check that an unprepared statement is being used.

2. If we really need to coerce this test, shouldn't we raise a more meaningful exception for when that happens in real life (outside that test case)?
I'm not sure what a more meaningful exception would be in this case. I could create a new error called InternalDatabaseError but not sure what benefit that would really be.

3. Can we somehow use a strategy similar to #786 (probably not, but who knows)?
This error can affect complex queries, see https://www.mssqltips.com/sqlservertip/5279/sql-server-error-query-processor-ran-out-of-internal-resources-and-could-not-produce-a-query-plan/. I'm not sure about making a change like #786 as the issue is based on the query and the specific database setup. Related to point 2 if someone runs the original test on a different database the test might pass.

[wpolicarpo]

Ok, now I see that test was coerced as per #34945.

So I guess Rails wants us to keep responsibility over the query rather than proactively trying to preview complex execution plans. Given that, I'm OK with this as all attempts were stressed.