persistence@glassfish.java.net

RE: code review for issue 189

From: Gordon Yorke <gordon.yorke_at_oracle.com>
Date: Sun, 19 Mar 2006 16:39:00 -0500

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
>