Hi Michael,
The changes look good. (including Function Expression)
Go ahead and check in,
Tom
Michael Bouschen wrote:
>Hi Tom,
>
>cc'ing ejb3-toplink-ext because I still have problems with the
>persistence mailing list.
>
>Thanks for the info, I was able to get the attribute type from the
>mapping and calculate the return type for the SUM aggregate. Attached
>you find my changes for code review. Please have a look. If you agree, I
>check in the fix tomorrow and set the corresponding issue to resolved:
> https://glassfish.dev.java.net/issues/show_bug.cgi?id=57
>
>Two remarks:
>(1) I added two addSum methods to ReportQuery taking a result type argument.
>(2) When adding DISTINCT to the aggregate argument (e.g. SELECT
>SUM(DISTINCT o.totalPrice) FROM ...) the query returned BigDecimal
>instated of DOUBLE. I fixed this in class FunctionExpression, please
>have a look at line 480/481.
>
>Regards Michael
>
>
>
>>Hi Michael,
>>
>> Comment inline.
>>
>>Michael Bouschen wrote:
>>
>>
>>
>>>Hi Tom, hi Gordon,
>>>
>>>attached you find changes that make use of addCount and addAverage
>>>taking a result type argument. It also fixes a bug that the
>>>generateExpression method in AggregateNode and CountNode does not add
>>>the aggregate function to the returned expression.
>>>
>>>With these changes the query "SELECT COUNT(o.orderId) FROM Order o"
>>>returns the expected Long instance. It still returns a BigDecimal for
>>>the following queries:
>>> SELECT COUNT(o) FROM Order o
>>> SELECT COUNT(o.customer) FROM Order o
>>>The SQL for these queries uses a COUNT(*):
>>> SELECT COUNT(*) FROM CMP3_ORDER
>>> SELECT COUNT(*) FROM CMP3_CUSTOMER t0, CMP3_ORDER t1
>>> WHERE (t0.CUST_ID = t1.CUST_ID)
>>>But the SQL for the EJBQL query accessing a state field in the COUNT
>>>expression is: "SELECT COUNT(ORDER_ID) FROM ...". Maybe this makes a
>>>difference for the return type.
>>>
>>>
>>>
>>>
>>It looks like we have a bug in the report query code here. I'll add it
>>to our bug list.
>>
>>
>>
>>>I tried another experiment using the following EJBQL:
>>> SELECT COUNT(o), COUNT(c) FROM Customer c JOIN c.orders o
>>>The generated SQL uses two COUNT(*) in the SELECT clause:
>>> SELECT COUNT(*), COUNT(*) FROM CMP3_ORDER t0, CMP3_CUSTOMER t1
>>> WHERE (t0.CUST_ID = t1.CUST_ID)
>>>I'm really not sure what the correct SQL should look like. I thought
>>>we could do a COUNT on the primary key columns instead of the
>>>COUNT(*), but this would not work in case of multiple primary key
>>>columns.
>>>
>>>
>>>
>>>
>>That's a really good test case. I think counting the primary key columns
>>should be fine. For multiple primary key columns we can just pick one
>>of them (since they are primary keys, none of them will be null).
>>
>>
>>
>>>I checked the spec about the return type of a SUM aggregate:
>>>"SUM returns Long when applied to state-fields of integral types
>>>(other than BigInteger); Double when applied to state-fields of
>>>floating point types; BigInteger when applied to state-fields of type
>>>BigInteger; and BigDecimal when applied to state-fields of type
>>>BigDecimal."
>>>We could add a similar API addSum taking a return type to the
>>>ReportQuery, but then the AggregateNode need to checks the type of the
>>>aggregate argument being an integral type, etc. Does an Expression
>>>instance provide any type information I can check?
>>>
>>>
>>>
>>>
>>We can easily add the method that includes type for SUM in report
>>query. To get the type, you will have to look at the session stored in
>>the GenerationContext. That session has a project with a table of
>>descriptors. Those descriptors have the mappings. You can look up the
>>mapping for an attribute. The 'attribute' classification on each
>>mapping holds the type. There is likely more information you need to
>>get this going, so let me know what you need me to do some leg work on
>>and I will do it.
>>
>>-Tom
>>
>>
>>
>>>Regards Michael
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>
>
>
>
>
--