castor
  1. castor
  2. CASTOR-3115 Make new ParseTreeWalker to translate queries correctly
  3. CASTOR-3118

Refactor CastorQLParseTreeWalker to understand COUNT(*) and COUNT(<path>) aggregations

    Details

    • Type: Sub-task Sub-task
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: JDO queries
    • Labels:
      None
    • Number of attachments :
      4

      Description

      The new ParseTreeWalker has problemswith projects with a COUNT aggregate, yet. ("SELECT COUNT(...) ..." as in test97.testOQLQueryWithParameter()).
      This is going to be fixed.

      1. patch-CastorQLParseTreeWalker-COUNT-110628.txt
        6 kB
        Johannes Venzke
      2. patch-CastorQLParseTreeWalker-COUNT-110701.txt
        7 kB
        Johannes Venzke
      3. patch-COUNT-110625.txt
        29 kB
        Johannes Venzke
      4. patch-COUNT-110627.txt
        31 kB
        Johannes Venzke

        Activity

        Hide
        Johannes Venzke added a comment -

        projects=projections

        Show
        Johannes Venzke added a comment - projects=projections
        Hide
        Johannes Venzke added a comment -

        Introduced Count function and adapted Parsers.

        Show
        Johannes Venzke added a comment - Introduced Count function and adapted Parsers.
        Hide
        Johannes Venzke added a comment -

        Recreated patches due to SVN changes.
        Note that support for COUNT STAR queries is not implemented, yet.

        Show
        Johannes Venzke added a comment - Recreated patches due to SVN changes. Note that support for COUNT STAR queries is not implemented, yet.
        Hide
        Ralf Joachim added a comment -

        The patch provided contains changes to QL interfaces that break the original design idea. In addition CastorQL and EJBQL parsers seam to have some other major bugs that need to be resolved first. To find out all of them we have to add tests for all required features and as the syntax of both parsers is different we should review them independent of the other. Therefore the attached patch have to be splitted up into 3 pieces. The new patches should be attached to the different subtask to CASTOR-3115 I created.

        THe most critical point here is the change in class design. The QO interfaces are intended to be used by users of Castor. All supported syntax elements have to be supported through these interfaces and users should not use or be aware of the implementations behind these interfaces. Therefore the following change in the patch is a no go:

        -        select.addProjection(schema.field("bar"));
        +        select.addProjection(new ProjectionImpl(schema.field("bar")));
        

        On the other hand it is a good idea to introduce a new aggregate package, AbstractAggregate and Count class. I also appreciate the tests added.

        Before investing more time in the implementation of QO I like to see suggestions how a user should work with aggrgation functions with QO interfaces only.

        Show
        Ralf Joachim added a comment - The patch provided contains changes to QL interfaces that break the original design idea. In addition CastorQL and EJBQL parsers seam to have some other major bugs that need to be resolved first. To find out all of them we have to add tests for all required features and as the syntax of both parsers is different we should review them independent of the other. Therefore the attached patch have to be splitted up into 3 pieces. The new patches should be attached to the different subtask to CASTOR-3115 I created. THe most critical point here is the change in class design. The QO interfaces are intended to be used by users of Castor. All supported syntax elements have to be supported through these interfaces and users should not use or be aware of the implementations behind these interfaces. Therefore the following change in the patch is a no go: - select.addProjection(schema.field( "bar" )); + select.addProjection( new ProjectionImpl(schema.field( "bar" ))); On the other hand it is a good idea to introduce a new aggregate package, AbstractAggregate and Count class. I also appreciate the tests added. Before investing more time in the implementation of QO I like to see suggestions how a user should work with aggrgation functions with QO interfaces only.
        Hide
        Johannes Venzke added a comment -

        Current state of development.

        Show
        Johannes Venzke added a comment - Current state of development.
        Hide
        Johannes Venzke added a comment -

        I did it.

        Show
        Johannes Venzke added a comment - I did it.

          People

          • Assignee:
            Ralf Joachim
            Reporter:
            Johannes Venzke
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated: