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

        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 "".
        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
        Michelle Caisse added a comment -

        New patch with latest fixes attached.

        Show
        Michelle Caisse added a comment - New patch with latest fixes attached.
        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
        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
        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
        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.
        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
        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
        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, ...
        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.
        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.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development