Skip to content

Loading…

DDC-335: Refactor DQL EBNF to use JOIN FETCH as syntax for fetch joins only #4148

Closed
doctrinebot opened this Issue · 7 comments

1 participant

@doctrinebot

Jira issue originally created by user @beberlei:

There are several problems with the current approach on fetch joins:

  1. There is no way to determine in the parser already if a join is fetch or not, this makes it much harder to implement things like @OrderBy or @OrderColumn
  2. A DQL like "SELECT u, g.name FROM User u JOIN u.group g" currently tries to partially load the group object instead of just returning g.name as scalar. This is very unintuiative.

Solution, change the EBNF too:

Join                                       ::= ["LEFT" ["OUTER"] ]( "INNER") "JOIN" [FETCH] JoinAssociationPathExpression 
                                               ["AS"] AliasIdentificationVariable [("ON" ]( "WITH") ConditionalExpression)

Question would be how to specify partial object selects.

Romans idea was something like:

select u.{name, other, field, stuff}

However my take is this would again put information into the select clause that might be interesting elsewhere, so

SELECT u FROM User u JOIN FETCH u.group PARTIAL id, name

That way we could add both $isFetchJoin and $isPartial flags to the AST/Join Node plus an additional $partialFields array.

@doctrinebot

Comment created by @beberlei:

Ah just to remember, the idea on the partial fetch syntax was something like:

select u FROM User u.{name, other, field, stuff} JOIN FETCH u.group g.{id, name, baz}
@doctrinebot

Comment created by @beberlei:

I think this will also make the HINTPARTIALLOAD constant obsolet, since the DQL is already implicitly requesting a partial load.

@doctrinebot

Comment created by romanb:

Regarding HINTFORCE_PARTIALLOAD: Not necessarily, but it might need to be renamed since its still used to decide whether to stub associations with empty collections/proxies.

Anyways, I've played around a bit with different implementations in the last days and I changed my mind regarding changing the fetch join syntax. It has many complications, like more difficult sql construction in the walker, among other things, and of course the fact that it would be a huge bc break.

However, some things will change. "select u.name" will select a scalar value, as you expect. The new syntax for partial object selection will currently be:

select partial u.{id,name}, partial a.{id,city} from User u join u.address a

There will be a new AST node PartialObjectExpression that represents such a construct.

Thats the current state of my progress. Regarding the isFetchJoin/isPartial decisions, we need to find another way and I'm confident there are some other ways.

So far...

@doctrinebot

Comment created by @beberlei:

If we change u.name to retrieving a scalar value always its easy to get all the fetch joins identification variables inside the SELECT already:

If (identificiationVariable) => fetchJoin

Then when the join is found, you can mark the Join as Fetch already.

@doctrinebot

Comment created by romanb:

Implemented.

@doctrinebot

Issue was closed with resolution "Fixed"

@doctrinebot doctrinebot added this to the 2.0-BETA1 milestone
@doctrinebot doctrinebot closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.