fix: Erroneous where clause on subquery w/ un-selected 'deleted_at' #185

[grossvogel]

Connects to #185

• Test case illustrating the example
• Actually fix it 😄

Notes

• I tried several iterations to arrive at this repro, and the key seems to be that the subquery is the "main" table / schema of the outer query, causing the get_schema_module call to recurse down and add the where clause to the outer query. It seems that what we really want instead is for the clause to be added to the subquery itself not to the outer query.

[LeakyBucket]

This gets us to green tests although I'm not super enthused about "modifying" the Query struct directly.

[grossvogel]

@LeakyBucket I think you've put your finger on (and helped me to see) the crux. In order to support subqueries, this change got us into the business of recursing down into the query to decide whether to alter it, and I'm now thinking we may not be able to honor that promise unless we're also willing to modify the deeper parts of the query that are triggering that decision logic. 🤔

[LeakyBucket]

@grossvogel Yeah, while I don’t believe the modification to the query in this case is too risky I do not know it won’t result in a nasty surprise for someone. I don’t know enough about the internals of Ecto to say for sure it wouldn’t create a different structure based on the full context of the query which the direct update would not do.

I feel like the change as presented here would need some extensive expansion to the test suite to try and validate a much broader class of queries.