Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-335

Refactor DQL EBNF to use JOIN FETCH as syntax for fetch joins only

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0-ALPHA4
    • Fix Version/s: 2.0-BETA1
    • Component/s: DQL
    • Security Level: All
    • Labels:
      None

      Description

      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.

        Issue Links

          Activity

          Benjamin Eberlei created issue -
          Benjamin Eberlei made changes -
          Field Original Value New Value
          Link This issue is required for DDC-195 [ DDC-195 ]
          Roman S. Borschel made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Benjamin Eberlei added a comment -

          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}
          
          Show
          Benjamin Eberlei added a comment - 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}
          Hide
          Benjamin Eberlei added a comment -

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

          Show
          Benjamin Eberlei added a comment - I think this will also make the HINT_PARTIAL_LOAD constant obsolet, since the DQL is already implicitly requesting a partial load.
          Hide
          Roman S. Borschel added a comment -

          Regarding HINT_FORCE_PARTIAL_LOAD: 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...

          Show
          Roman S. Borschel added a comment - Regarding HINT_FORCE_PARTIAL_LOAD: 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...
          Hide
          Benjamin Eberlei added a comment -

          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.

          Show
          Benjamin Eberlei added a comment - 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.
          Hide
          Roman S. Borschel added a comment -

          Implemented.

          Show
          Roman S. Borschel added a comment - Implemented.
          Roman S. Borschel made changes -
          Status In Progress [ 3 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Benjamin Eberlei made changes -
          Workflow jira [ 10891 ] jira-feedback [ 15548 ]
          Benjamin Eberlei made changes -
          Workflow jira-feedback [ 15548 ] jira-feedback2 [ 17412 ]
          Benjamin Eberlei made changes -
          Workflow jira-feedback2 [ 17412 ] jira-feedback3 [ 19669 ]

          This list may be incomplete, as errors occurred whilst retrieving source from linked applications:

          • Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={query=DDC-335, expand=changesets[0:20].revisions[0:29],reviews}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

            People

            • Assignee:
              Roman S. Borschel
              Reporter:
              Benjamin Eberlei
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: