persistence@glassfish.java.net

Re: code review for issue 189

From: Michael Bouschen <Michael.Bouschen_at_Sun.COM>
Date: Mon, 20 Mar 2006 20:36:29 +0100

Hi Tom,

AFAIK, the JPQL compiler currently uses a different strategy to map JOIN
clauses in case the joined variable is selected. It creates a new
ExpressionBuilder for the joined variable ('o' in the example below) and
adds an expression to the WHERE clause that combines the two
ExpressionBuilder instances as shown in my code snippet below. This
should return the same result, shouldn't it?

Regards Michael

> Here is how I would write the ReportQuery for "SELECT c FROM Order o
> LEFT OUTER JOIN o.customer c"
>
> ReportQuery query = new ReportQuery(Order.class, new
> ExpressionBuilder(Order.class));
> ExpressionBuilder builder = query.getExpressionBuilder();
> query.returnWithoutReportQueryResult();
> query.addAttribute("customer",
> builder.getAllowingNull("customer"));
>
> On my object model, it generates:
>
> SELECT t0.CUST_ID, t0.CITY, t0.NAME, t0.CUST_VERSION
> FROM CMP3_CUSTOMER t0, CMP3_ORDER t1
> WHERE (t0.CUST_ID (+) = t1.customer_CUST_ID)
>
> This SQL looks correct to me. I am not, however certain how easy it
> is to generate a similar ReportQuery in EJBQL. -Tom
>
>
> Michael Bouschen wrote:
>
>> Hi Gordon, hi Tom, hi Jielin,
>>
>> I did some experiment with using the ReportQuery class directly. I
>> hope the code below is the correct TopLink code for the JPQL query:
>> SELECT c FROM Order o LEFT OUTER JOIN o.customer c
>>
>> ExpressionBuilder cBuilder = new ExpressionBuilder(Customer.class);
>> ExpressionBuilder oBuilder = new ExpressionBuilder(Order.class);
>> Expression expr =
>> oBuilder.getAllowingNull("customer").equal(cBuilder);
>> ReportQuery reportQuery = new ReportQuery(Order.class, oBuilder);
>> reportQuery.dontMaintainCache();
>> reportQuery.setShouldReturnWithoutReportQueryResult(true);
>> reportQuery.setSelectionCriteria(expr);
>> reportQuery.addItem("c", cBuilder);
>>
>> Please note the third line calls getAllowingNull on the order
>> builder. However the generated SQL does not include an outer join
>> clause and is exactly the same when calling
>> oBuilder.get("customer").equal(cBuilder):
>> SELECT t0.ID, t0.NAME, t0.VERSION, t0.CITY, t0.COUNTRY_ID,
>> t0.COUNTRY_CODE, t0.COUNTRY_NAME
>> FROM CUSTOMER_TABLE t0, ORDER_TABLE t1 WHERE (t1.CUST_ID = t0.ID)
>>
>> Regards Michael
>>
>>
>>
>>
>>> Hello Michael, Jielin
>>> I haven't had a chance to look at the fix yet, but the reason for
>>> nonFetchJoin is the case when there is no join implied in the where
>>> clause. Adding the expression to the query as a nonFetchJoin and
>>> having it in the expression as well, will most likely cause a double
>>> join to occur.
>>> If needed by the QL parser we may be able to do some
>>> post-processing in the query but it probably easier for the parser
>>> to detect the special case.
>>> --Gordon
>>>
>>> -----Original Message-----
>>> From: Michael Bouschen [mailto:Michael.Bouschen_at_Sun.COM]
>>> Sent: Sunday, March 19, 2006 3:23 PM
>>> To: persistence_at_glassfish.dev.java.net
>>> Subject: Re: code review for issue 189
>>>
>>>
>>> Hi Jielin, hi Tom, hi Gordon,
>>>
>>> Tom, Gordon,
>>> could you please comment on issues (1) and (2) below, thanks!
>>>
>>> Jielin,
>>> great that you figured out calling addNonFetchJoinAttributes would add
>>> the OUTER JOIN clause to the generated SQL.
>>>
>>> However, I think there are some issues with the proposed fix:
>>> (1) When I added addNonFetchJoinAttributes to class ParseTree, I
>>> understood we need to call addNonFetchJoinedAttribute *only* in case
>>> the
>>> variable is declared in a JOIN clause, but not used in the query. An
>>> example is variable 'a' in the following query:
>>> SELECT c FROM Customer c JOIN c.orders o JOIN c.address a WHERE
>>> o.totalPrice > 2000.0
>>> I understood the compiler must not call addNonFetchJoinedAttribute for
>>> variable 'o' because the join is handled when processing
>>> 'o.totalPrice'.
>>> (2) Maybe I'm wrong and we should call addNonFetchJoinedAttribute for
>>> all variables declared in a JOIN or IN clause. Actually this would ease
>>> the code in ParseTree.addNonFetchJoinAttributes, because then we only
>>> need a single for loop.
>>> (3) Method ParseTreeContext.getUsedVariables returns a collection of
>>> all
>>> variables, not just the used variables. The map usedVariables in
>>> ParseTreeContext includes an entry for all variables, the value is
>>> Boolean.TRUE if the variable has been used and Boolean.FALSE otherwise.
>>> With your change we call addNonFetchJoinedAttribute twice for an used
>>> variable. If (2) is correct, I propose to rename method
>>> getUsedVariables to getVariables and just keep the for loop you added.
>>> This means removing the first for loop iterating the unused variables.
>>>
>>> Regards Michael
>>>
>>>
>>>
>>>
>>>> Hi, Tom, Gordon, Michael,
>>>>
>>>> Please find attached changes to fix issue 189 - outer join is missing
>>>>
>>>> from generated sql.
>>>
>>>
>>>
>>>> If the query is like:
>>>>
>>>> SELECT c from Order o LEFT OUTER JOIN o.customer c
>>>>
>>>> The generated sql "SELECT t0.id, ... from CUSTOMER_TABLE t0,
>>>> ORDER_TABLE t1 WHERE (t1.CUST_ID = t0.ID)" is missing LEFT OUTER JOIN.
>>>>
>>>> I made change in ParseTree:addNonFetchJoinAttributes() to add the node
>>>> expression to ObjectLevelReadQuery if the node is JoinDeclNode.
>>>>
>>>> Now the generated sql is like:
>>>>
>>>> SELECT t0.ID, ... FROM ORDER_TABLE t2 LEFT OUTER JOIN CUSTOMER_TABLE
>>>> t1 ON (t1.ID = t2.CUST_ID), CUSTOMER_TABLE t0 WHERE (t2.CUST_ID =
>>>> t0.ID)
>>>>
>>>> There is duplicate tables in the generated sql which is caused by
>>>> issue 197.
>>>>
>>>> Thanks.
>>>>
>>>> Jielin
>>>>
>>>>
>>>
>>>
>>
>