ensure select statements in joined queries have access to parent bindings

[zachdaniel]

Ultimately, I don't fully understand how parent binding sources are managed well enough to really say if this is the most correct fix. Since parent_as works in other, non-select contexts using this formulation, it feels like there may be a more targeted/specific fix available?

Regardless, the newly added test will fail without the fix, and the fix does not cause any additional test failures. Open to adjusting it if as needed.

[josevalim]

Thanks for the PR! I would like to hear @greg-rychlewski's opinion but my gut feeling is that the test should not pass indeed. In this case:

    query = from thing in Post, 
      as: :outer, 
      inner_lateral_join: sub in ^(from row in subquery(child)),
      on: true

The parent query of subquery(child) is from row in subquery(child). Therefore, the parent_as in the subquery is trying to access aliases from from row in subquery.

Forget subqueries for a second. Imagine you have child. When you do this:

another_query = from ...
query = from row in another_query

In this case, query and another_query are doing regular composition. They share the same as. It is not a parent/child relationship. query is not the parent of another_query.

Therefore, when you do this:

another_query = from ...
query = from Post, join: row in another_query

Once again, another_query is not subquery, query is not its parent. This PR changes them to be a child/parent relationship, which is wrong, given by the fact there are no subqueries involved.

[josevalim]

Let me give another example:

    query =
      from thing in Post,
        as: :outer,
        inner_lateral_join: sub in ^from(row in subquery(child), join: "outers", as: :outer),
        on: true

which one is now the actual parent? The query above doesn't work, because we only allow a subset of queries in joins, but we could allow the construct above in the future (it is not a technical limitation, it just wasn't implemented yet), and now there would be ambiguity, because you want the subquery(child) to bypass its actual parent and go all the way to the top.

[zachdaniel]

I could be wrong here, but don't parent_as bindings "walk upwards" until it finds a named binding? I'm pretty sure things in AshPostgres would break if this wasn't the case.

i.e given your example:

query =
  from thing in Post,
    as: :outer,
    inner_lateral_join: sub in ^from(row in subquery(child), join: "outers", as: :outer),
    on: true

parent_as(:outer) in child would find the closest. If you named them differently, though, then you could reference either.

[josevalim]

The walking upwards does not matter, because, as per my first comment, there is no parent/child relationship between the from and the query given in join. The parent/child relationship only happens when you wrap them in subqueries.

You have three queries:

QUERY 1 = from thing in Post, ...
QUERY 2 = from(row in subquery(child), join: "outers", as: :outer)
QUERY 3 = child

QUERY 1 and 2 is regular query composition. There is no parent/child relationship in there. The same way another_query query = from row in another_query is not a child of query, it is composition.

[josevalim]

In summary, you only have child/parent relationships if there is an explicit subquery call. That's exactly so people don't have to guess what is composition and what is parent/child. That's why I said it could only work if you did double nesting:

query =
  from thing in Post,
    as: :outer,
    inner_lateral_join: sub in subquery(from(row in subquery(child))
    on: true

(which in this case is obviously unnecessary)

[greg-rychlewski]

Yeah after looking at this I have the same opinion as @josevalim. I think the crux is these two things are not the same

inner_lateral_join: s in subquery(q)

inner_lateral_join: s in ^(from s in subquery(q))

The subquery in the latter example doesn't know anything outside of from s in subquery(q)).

I can understand the confusion though because if I was just writing the SQL by hand and joining on a query I would be forced to wrap the whole thing in a subquery or it would be invalid SQL. But with Ecto you have to think in terms of composition.

[zachdaniel]

Roger. It makes sense, but I think is a bit of a footgun. In a case like that when you're slotting an Ecto query into a place that can only ever be a subquery (lateral joins require a subquery) it feels like it should just treat the containing query as a "parent" for the purposes of parent_as. Which, FWIW, I think it does as long as I don't do from row in ^subquery which I'm doing somewhere currently 😆

Probably as simple as me not doing that or unwrapping it when I do the inner lateral join 🤷

[greg-rychlewski]

I think the issue is that in raw SQL the joined query must be in a subquery whether it is lateral or not. Lateral just changes how the join is done and allows access to the outside.

So when Ecto is allowing a join on a query under any circumstance, it is performing composition rather than an implicit subquery.

Which, for the record, I agree is confusing at first. The first time I saw it I had to take a look at the actual query that was being generated to fully understand what was going on. But it is providing useful functionality to users.