JDO
  1. JDO
  2. JDO-448

Add an xml element to specify the fetch plan to use for a query

    Details

      Description

      In Chapter 18, add an xml element to specify the fetch plan to use for a query.

      1. jdo-448.patch
        6 kB
        Michelle Caisse

        Activity

        Michelle Caisse created issue -
        Craig L Russell made changes -
        Field Original Value New Value
        Fix Version/s JDO 2 maintenance release 1 [ 12310923 ]
        Craig L Russell made changes -
        Component/s specification [ 12311332 ]
        Hide
        Craig L Russell added a comment -

        There are a couple of ways to handle this issue.

        1. Add element fetch-group*, and attributes max-fetch-depth, fetch-size to element query. These are used to set the FetchPlan in the stored query.

        <query name="BySalary" max-fetch-depth="3" fetch-size="17">
        <fetch-group name="default"/>
        <fetch-group name="images"/>
        SELECT THIS from Employee WHERE salary == :salary
        </query>

        2. Add a new fetch-plan element that contains fetch-group* and attributes name, max-fetch-depth, fetch-size. Add attribute fetch-plan to element query.

        <fetch-plan name="EmployeeImages" max-fetch-depth="3" fetch-size="17">
        <fetch-group name="default"/>
        <fetch-group name="images"/>
        </fetch-plan>

        <query name="BySalary" fetch-plan="EmployeeImages">
        SELECT THIS from Employee WHERE salary == :salary
        </query>

        3. Add a new fetch-plan element that contains fetch-group* and attributes max-fetch-depth, fetch-size. Add element fetch-plan to element query.

        <query name="BySalary">
        <fetch-plan name="EmployeeImages" max-fetch-depth="3" fetch-size="17">
        <fetch-group name="default"/>
        <fetch-group name="images"/>
        </fetch-plan>
        SELECT THIS from Employee WHERE salary == :salary
        </query>

        I like the idea of named fetch plans because the same fetch plan could then be used in multiple queries. Also, I think there is value in being able to specify declaratively what a fetch plan is, without having to use an API to declare it. We could use this in other places in the spec.

        So I'm leaning toward option 2.

        Show
        Craig L Russell added a comment - There are a couple of ways to handle this issue. 1. Add element fetch-group*, and attributes max-fetch-depth, fetch-size to element query. These are used to set the FetchPlan in the stored query. <query name="BySalary" max-fetch-depth="3" fetch-size="17"> <fetch-group name="default"/> <fetch-group name="images"/> SELECT THIS from Employee WHERE salary == :salary </query> 2. Add a new fetch-plan element that contains fetch-group* and attributes name, max-fetch-depth, fetch-size. Add attribute fetch-plan to element query. <fetch-plan name="EmployeeImages" max-fetch-depth="3" fetch-size="17"> <fetch-group name="default"/> <fetch-group name="images"/> </fetch-plan> <query name="BySalary" fetch-plan="EmployeeImages"> SELECT THIS from Employee WHERE salary == :salary </query> 3. Add a new fetch-plan element that contains fetch-group* and attributes max-fetch-depth, fetch-size. Add element fetch-plan to element query. <query name="BySalary"> <fetch-plan name="EmployeeImages" max-fetch-depth="3" fetch-size="17"> <fetch-group name="default"/> <fetch-group name="images"/> </fetch-plan> SELECT THIS from Employee WHERE salary == :salary </query> I like the idea of named fetch plans because the same fetch plan could then be used in multiple queries. Also, I think there is value in being able to specify declaratively what a fetch plan is, without having to use an API to declare it. We could use this in other places in the spec. So I'm leaning toward option 2.
        Craig L Russell made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        Michelle Caisse added a comment -

        I also like being able to define a named fetch-plan. This works well with xml metadata where you can nest a fetch-plan element under <jdo>, but with annotations you would have to apply the @FetchPlan annotation to a class or interface, while the intent is for the scope to be global.

        Show
        Michelle Caisse added a comment - I also like being able to define a named fetch-plan. This works well with xml metadata where you can nest a fetch-plan element under <jdo>, but with annotations you would have to apply the @FetchPlan annotation to a class or interface, while the intent is for the scope to be global.
        Craig L Russell made changes -
        Assignee Craig Russell [ clr ] Michelle Caisse [ mcaisse ]
        Hide
        Michelle Caisse added a comment -

        The attached patch adds an annotation and xml schema for a named fetch plan. The annotation applies to a type and the xml metadata is nested within the jdo element. I did not add fetch-plan the jdoquery schema because that would have required adding fetch-group to the schema, which in turn would have required adding field, property, ...

        Show
        Michelle Caisse added a comment - The attached patch adds an annotation and xml schema for a named fetch plan. The annotation applies to a type and the xml metadata is nested within the jdo element. I did not add fetch-plan the jdoquery schema because that would have required adding fetch-group to the schema, which in turn would have required adding field, property, ...
        Michelle Caisse made changes -
        Attachment jdo-448.patch [ 12371718 ]
        Hide
        Andy Jefferson added a comment -

        Why not add @FetchPlans too, in the same style as @FetchGroups ? Would mean that a class could define multiple fetch plans, rather than being limited to a single one.

        Show
        Andy Jefferson added a comment - Why not add @FetchPlans too, in the same style as @FetchGroups ? Would mean that a class could define multiple fetch plans, rather than being limited to a single one.
        Hide
        Andy Jefferson added a comment -

        @FetchPlan : The default for "maxFetchDepth" should be 1, and the default for fetchSize should be 0 (FETCH_SIZE_OPTIMAL) for consistency with javax.jdo.FetchPlan

        Show
        Andy Jefferson added a comment - @FetchPlan : The default for "maxFetchDepth" should be 1, and the default for fetchSize should be 0 (FETCH_SIZE_OPTIMAL) for consistency with javax.jdo.FetchPlan
        Hide
        Michelle Caisse added a comment -

        Thanks for the comments Andy. I've attached a patch with the changes.

        Show
        Michelle Caisse added a comment - Thanks for the comments Andy. I've attached a patch with the changes.
        Michelle Caisse made changes -
        Attachment jdo-448.patch [ 12371838 ]
        Michelle Caisse made changes -
        Attachment jdo-448.patch [ 12371718 ]
        Hide
        Andy Jefferson added a comment -

        Michelle, looks good, except the @FetchPlan annotation has the defaults reversed from my comment above.

        Show
        Andy Jefferson added a comment - Michelle, looks good, except the @FetchPlan annotation has the defaults reversed from my comment above.
        Hide
        Andy Jefferson added a comment -

        Also @Query will need an attribute "fetchPlan" to allow the same as <query fetch-plan="...">

        Show
        Andy Jefferson added a comment - Also @Query will need an attribute "fetchPlan" to allow the same as <query fetch-plan="...">
        Hide
        Craig L Russell added a comment -

        The xsd patch will apply to both api2 and api2-legacy.

        Show
        Craig L Russell added a comment - The xsd patch will apply to both api2 and api2-legacy.
        Hide
        Michelle Caisse added a comment -

        New patch with latest fixes attached.

        Show
        Michelle Caisse added a comment - New patch with latest fixes attached.
        Michelle Caisse made changes -
        Attachment jdo-448.patch [ 12371884 ]
        Michelle Caisse made changes -
        Attachment jdo-448.patch [ 12371838 ]
        Hide
        Craig L Russell added a comment -

        Looks good.

        The comment
        +/**
        + * Annotation for a group of fetch-group objects
        + *
        + * @version 2.1
        + * @since 2.1

        Should refer to a group of FetchPlan not fetch-group.

        Show
        Craig L Russell added a comment - Looks good. The comment +/** + * Annotation for a group of fetch-group objects + * + * @version 2.1 + * @since 2.1 Should refer to a group of FetchPlan not fetch-group.
        Hide
        Craig L Russell added a comment -

        + /**
        + * The fetch groups in this fetch plan.
        + * @return the fetch groups
        + */
        + String[] fetchGroups() default "";

        The default should be [ ] not "".

        Show
        Craig L Russell added a comment - + /** + * The fetch groups in this fetch plan. + * @return the fetch groups + */ + String[] fetchGroups() default ""; The default should be [ ] not "".
        Michelle Caisse made changes -
        Status In Progress [ 3 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Andy Jefferson made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        315d 3m 1 Craig L Russell 20/Oct/07 00:24
        In Progress In Progress Resolved Resolved
        82d 12m 1 Michelle Caisse 09/Jan/08 23:37
        Resolved Resolved Closed Closed
        886d 11h 32m 1 Andy Jefferson 14/Jun/10 12:09

          People

          • Assignee:
            Michelle Caisse
            Reporter:
            Michelle Caisse
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development