Add subquery support to JPOX

• Key: JDO-568
• Type: Task
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Andy Jefferson
• Reporter: Michael Bouschen
• Component/s: tck2, tck2-legacy
• Affects Version/s: JDO 2 maintenance release 1
• Fix Version/s: JDO 2 maintenance release 1

Description

The subquery test cases currently fail, because JPOX does not yet support subqueries:
    [java] RUN CorrelatedSubqueries.testPositive ERROR
    [java] RUN CorrelatedSubqueriesWithParameters.testPositive ERROR
    [java] RUN MultipleCallsReplaceSubquery.testPositive ERROR
    [java] RUN NonCorrelatedSubqueries.testPositive ERROR
    [java] RUN NullCandidateCollectionExpression.testPositive ERROR
    [java] RUN NullSubqueryParameter.testPositive ERROR
    [java] RUN UnmodifiedSubqueryInstance.testPositive ERROR

Here is the corresponding JPOX JIRA: http://www.jpox.org/servlet/jira/browse/CORE-3207.

Comments

[Andy Jefferson]

Now looks more like this :-

    [java] RUN CorrelatedSubqueries.testPositive FAILURE
    [java] RUN CorrelatedSubqueriesWithParameters.testPositive ERROR
    [java] RUN NonCorrelatedSubqueries.testPositive ERROR
    [java] RUN NullSubqueryParameter.testPositive FAILURE

JPOX JIRA issue updated to http://www.jpox.org/servlet/jira/browse/RDBMS-91

[Andy Jefferson]

NonCorrelatedSubqueries test has an error. It tries to call
List allEmployees = (List)pm.newQuery(Employee.class).execute();
yet there is no transaction, and non-tx-read is not set. Should it call executeJDOQuery and use the result ?

This likely applies also to CorrelatedSubqueriesXXX

[Michael Bouschen]

Good catch!

The purpose of this query is to generate a memory collection including all the employees and pass this as the candidate collection to the query test. I think we cannot use method executeJDOQuery for this purpose, because the method executes the query and compares the result against the expected result passed as a parameter. I propose to enclose the query execution in a tx begin/commit.

Attached you find a patch for review (JDO-568.patch). It adds a helper method to the superclass that returns the list of all employees.

[Andy Jefferson]

Thx Michael. Patch looks fine, but before committing it just one more thing ;-)
In the single-string variants of the the subqueries you have

"SELECT FROM Employee WHERE this.hiredate > :hired && this.weeklyhours > (SELECT AVG(e.weeklyhours) FROM Employee e)"

The subquery seems to have an "alias" specified for the candidate ("e"). I don't see this in the spec for single-string queries (14.6.13), so maybe its a JPQLism that should be omitted ?

[Andy Jefferson]

When looking at the "correlated" tests with this "candidateExpression", what is the expected result of calling execute() on a subquery of that form, for example

Query q = pm.newQuery("SELECT AVG(e.weeklyhours) FROM this.department.employees e");
q.execute();

I'd expect JDOUserException since the query can only be used as a subquery (the candidate expression is by no means a "candidate-class-name" in 14.6.13), but this would need to be added to the spec in 14.6.1

[Craig Russell]

Hi Andy,

> The subquery seems to have an "alias" specified for the candidate ("e"). I don't see this in the spec for single-string queries (14.6.13), so maybe its a JPQLism that should be omitted ?

For a subquery, an alias is needed to disambiguate a field reference in the subquery from the outer query, since for the api version any references to the outer query need to be passed as parameters.

> Query q = pm.newQuery("SELECT AVG(e.weeklyhours) FROM this.department.employees e");
> q.execute();

> I'd expect JDOUserException...

I agree that the subquery cannot be executed directly as a top level query. This means that the FROM clause for subqueries is different from outer queries.

Once we agree on these two items, I'll update the specification.

[Andy Jefferson]

Hi Craig,
no problem with including aliases for that purpose since "this" is already in use. Spec clarification would be great.

[Michael Bouschen]

I checked in the patch (see revision 614649).

The FROM clause of a subquery allows two syntax elements in addition to what the FROM clause of an outer query supports:
- Specify an alias other than "this". This allows accessing fields from the inner and outer candidate class.
- Use an expression when specifying the candidates in the FROM clause.

I agree the query "SELECT AVG(e.weeklyhours) FROM this.department.employees e" is only valid when used as subquery.

[Andy Jefferson]

2 further comments on the "NonCorrelatedSubqueries"
1. The single-string query has Employee hardcoded yet should be a fully-qualified name. This likely applies to other subquery test single-string forms.
2. "runTestSubqueries02" has a query with a parameter yet none is supplied. Not checked the Correlated tests for whether they have this too

[Andy Jefferson]

Comment on "NullSubqueryParameter", the JDOQL that JPOX runs is
SELECT FROM org.apache.jdo.tck.pc.company.Employee
WHERE this.weeklyhours > emp.weeklyhours && emp.firstname == 'emp1First'
VARIABLES Employee emp

which seems correct to me, based on the comments of the test about what a null subquery parameter means. This is then converted into the following SQL

SELECT {...}
FROM datastoreidentity0.PERSONS THIS , datastoreidentity0.PERSONS UNBOUND_EMP
WHERE (THIS.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.Employee'>
    OR THIS.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.PartTimeEmployee'>
    OR THIS.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.FullTimeEmployee'>)
AND THIS.WEEKLYHOURS > UNBOUND_EMP.WEEKLYHOURS
AND UNBOUND_EMP.FIRSTNAME = <'emp1First'>

Maybe the UNBOUND_EMP should have a discriminator restriction, but apart from that ?

[Andy Jefferson]

In fact looking at the testdata for subqueries tests in
src/testdata/org/apache/jdo/tck/pc/company/companyForSubqueriesTests.xml

... there are NO Employees with more "weeklyhours" than the Employee with firstName of "emp1First" (40) so JPOX returns the correct result, and the test seems wrong to me. Perhaps if that ">" in the JDOQL was "<" ?

[Andy Jefferson]

Issues about single-string query use of "Employee", and lack of parameter input are fixed in SVN.

[Andy Jefferson]

And the "NullSubqueryParameter" test had an API query using ">" yet the single-string had "==" (which matched the test expectations), so SVN trunk is now changed to have consistent queries.

Test results are now :-
    [java] RUN CorrelatedSubqueries.testPositive ERROR
    [java] RUN CorrelatedSubqueriesWithParameters.testPositive ERROR
    [java] RUN MultipleCallsReplaceSubquery.testPositive
    [java] RUN NonCorrelatedSubqueries.testPositive
    [java] RUN NullCandidateCollectionExpression.testPositive
    [java] RUN NullSubqueryParameter.testPositive
    [java] RUN NullVariableDeclaration.testNegative
    [java] RUN UnmodifiedSubqueryInstance.testPositive
    [java] Error summary:
    [java] 002 errors: org.jpox.exceptions.JPOXException: JPOX doesnt currently support the use of candidate expressions in subqueries

[Michael Bouschen]

Hi Andy,

Good catch! I agree with all your changes.

Regards Michael

[Andy Jefferson]

"CorrelatedSubqueriesWithParameters" has a query of
Query sub = pm.newQuery(Employee.class);
sub.setResult("avg(this.weeklyhours)");
sub.setFilter("this.manager == :manager");
Query apiQuery = pm.newQuery(Employee.class);
apiQuery.setFilter("this.weeklyhours> averageWeeklyhours");
apiQuery.addSubquery(sub, "double averageWeeklyhours", "this.department.employees", "this.manager");

which in single-string JDOQL is
SELECT FROM Employee WHERE this.weeklyhours >
    (SELECT AVG(e.weeklyhours) FROM this.department.employees e
     WHERE e.manager == this.manager)

JPOX is translating this into

SELECT THIS.DISCRIMINATOR,THIS.DATASTORE_IDENTITY, ...
FROM datastoreidentity0.PERSONS THIS
WHERE (THIS.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.Employee'>
    OR THIS.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.PartTimeEmployee'>
    OR THIS.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.FullTimeEmployee'>)
AND THIS.WEEKLYHOURS > (
    SELECT AVG(SUB.WEEKLYHOURS) FROM datastoreidentity0.PERSONS SUB
    INNER JOIN datastoreidentity0.DEPARTMENTS T1 ON T1.DATASTORE_IDENTITY = SUB.DEPARTMENT
    WHERE (SUB.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.Employee'>
        OR SUB.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.PartTimeEmployee'>
        OR SUB.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.FullTimeEmployee'>)
    AND T1.DATASTORE_IDENTITY = THIS.DEPARTMENT
    AND THIS.MANAGER = SUB.MANAGER)

which is correct in my understanding of the intention of correlated subqueries and their parameters.
The JPOX query returns
[PartTimeEmployee(9), FullTimeEmployee(6), FullTimeEmployee(2)]
whereas the query is expecting
[FullTimeEmployee(2), FullTimeEmployee(6)]

Anyone see the error in the query ? or the expectations?


The previous test "CorrelatedSubqueries" has the same query without the manager parameter and is generating the SQL
SELECT THIS.DISCRIMINATOR,THIS.DATASTORE_IDENTITY, ...
FROM datastoreidentity0.PERSONS THIS
WHERE (THIS.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.Employee'>
    OR THIS.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.PartTimeEmployee'>
    OR THIS.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.FullTimeEmployee'>)
AND THIS.WEEKLYHOURS > (
    SELECT AVG(SUB.WEEKLYHOURS) FROM datastoreidentity0.PERSONS SUB
    INNER JOIN datastoreidentity0.DEPARTMENTS T1 ON T1.DATASTORE_IDENTITY = SUB.DEPARTMENT
    WHERE (SUB.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.Employee'>
        OR SUB.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.PartTimeEmployee'>
        OR SUB.DISCRIMINATOR = <'org.apache.jdo.tck.pc.company.FullTimeEmployee'>)
    AND T1.DATASTORE_IDENTITY = THIS.DEPARTMENT)

and gives the expected result (with JPOX SVN)

[Andy Jefferson]

I'd suggest, after looking at the data as I interpret it, that the expectations are wrong.
In "dept2" we have emp7, emp8, emp9, emp10. Only emp8 and emp9 have a manager set so are the only ones considered in the average ... hence why emp9 is coming through. Or is this a complete misinterpretation of that JDOQL subquery?

[Michael Bouschen]

Hi Andy,

I agree the expected result of CorrelatedSubqueriesWithParameters.runTestSubqueries01 should include emp9. I overlooked that only emp8 and emp9 are considered when calculating the average for dept2.

I checked in a fix (see revision 615642).

Regards Michael

[Andy Jefferson]

Thx Michael.
I also fixed a couple of errors -missing param, and typo in param name.
The final thing remaining is in "runTestSubqueries03" where I am also expecting "emp9" to appear - probably the same thing as in "runTestSubqueries01". Can you check that one please.

[Michael Bouschen]

You are right! I updated the expected result in method runTestSubqueries03 (see revision 615658).

[Andy Jefferson]

All tests pass. Consigned to the great issue repository in the sky