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 -

        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.
        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
        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
        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.
        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.
        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 "".

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development