Looks good Mitesh,
Please check it in.
--GOrdon
-----Original Message-----
From: Mitesh Meswani [mailto:mitesh.meswani_at_Sun.COM]
Sent: Wednesday, March 15, 2006 10:07 PM
To: persistence_at_glassfish.dev.java.net
Cc: Tom Ware
Subject: Re: Fix for issue 248
Hi Gordon, Tom,
Please find attached the changes. Following is a brief summary of change
entity-persistence
Added ExpressionOperator.currentDate that corresponds to JPQL
CURRENT_DATE semantics
Added mapping to this operator to relevant platforms.
Added simpleFunctionNoParentheses to ExpresionOperator Refactored code
in various platforms to use this function
Added currentDateDate to Expression that uses ExpressionOperator.currentDate
Modified DateFunctionNode to refer to Expression.currentDateDate instead
of Expression.currentDate
entity-persistence-test
Added a test that exercise CURRENT_DATE to JUnitEJBQLDateTimeTestSuite
tests run
JUnitEJBQLDateTimeTestSuite on derby, oracle, db2, sybase and msssql
(Note that the test fails on sybase and mssql)
Thanks,
Mitesh
Gordon Yorke wrote:
> ExpressionOperator.Today is internal so it has no documented behaviour, however it is what is created when currentDate() is called. I should have looked at the exact names of our methods before I responded. You should create a Expression.currentDATE() or currentDateDate() or jpqlCurrentDate() or some other better name.
>
> The problem with changing the PlatForm is that Expression.currentDate() (which uses the ExpressionOperator.Today) has existing functionality. The javadoc for Expression.currentDate() is ambiguous and because there is no pre-existing Expression.currentTimestamp() there is no suggestion of the actual return type of Expression.currentDate(). As such we can not alter the behaviour of Expression.currentDate() which your changes to the OraclePlatform would have done.
>
> --Gordon
>
> -----Original Message-----
> From: Mitesh Meswani [mailto:mitesh.meswani_at_Sun.COM]
> Sent: Wednesday, March 15, 2006 5:30 PM
> To: persistence_at_glassfish.dev.java.net
> Cc: Tom Ware
> Subject: Re: Fix for issue 248
>
>
> Ok. I will go ahead and change DateFunctionNode to use newly added
> ExpressionOperator.currentDate instead of ExpressionOperator.Today.
>
> Looking at the code and method names used. (For example
> Expression.currentDate() which currently maps to
> ExpressionOperator.Today() ), I am curious what is the documented
> behavior for ExpressionOperator.Today in toplink. Does it correspond to
> current date or current date-time (as it is coded now)?
>
> Thanks,
> Mitesh
>
> I am curious whether Expression
> Gordon Yorke wrote:
>
>> Well I am suggesting that the EJBQL parser use ExpressionOperator.currentDate instead of ExpressionOperator.today, I think that is better than altering the definition of currently used APIs.
>> --Gordon
>>
>> -----Original Message-----
>> From: Mitesh Meswani [mailto:mitesh.meswani_at_Sun.COM]
>> Sent: Wednesday, March 15, 2006 5:02 PM
>> To: persistence_at_glassfish.dev.java.net
>> Cc: Tom Ware
>> Subject: Re: Fix for issue 248
>>
>>
>>
>> Gordon Yorke wrote:
>>
>>
>>> Hi Mitesh,
>>> I think we should define a new ExpressionOperator for use by EJBQL. ExpressionOperator.Today is a pre-existing expression that has defined behaviour. If there are current users using it we should not break their code.
>>>
>>> Perhaps ExpressionOperator.currentDate, ExpressionOperator.currentTimestamp.
>>>
>>>
>>>
>> This implies that we will need to change the ejbql compiler to output
>> ExpressionOperator.currentDate instead of ExpressionOperator.currentDate
>> for ejbql expression "CURRENT_DATE". Is that correct?
>>
>> Thanks,
>> Mitesh
>>
>>
>>
>>> I think your proposal for applying the TO_DATE on Oracle for ExpressionOperator.currentDate would be correct.
>>>
>>> What do you think?
>>> --Gordon
>>>
>>> -----Original Message-----
>>> From: Mitesh Meswani [mailto:mitesh.meswani_at_Sun.COM]
>>> Sent: Tuesday, March 14, 2006 9:15 PM
>>> To: Tom Ware
>>> Cc: persistence_at_glassfish.dev.java.net
>>> Subject: Fix for issue 248
>>>
>>>
>>> Hi Tom,
>>> Could you please review the attached fix for issue
>>> 248(https://glassfish.dev.java.net/issues/show_bug.cgi?id=248)
>>> It changes the code to use "CURRENT DATE" instead of "CURRENT TIMESTAMP"
>>> for ExpressionOperator.Today. I have tested the change on both db2 as
>>> well as derby
>>>
>>> Following is the diff
>>> $ cvs diff
>>> src/java/oracle/toplink/essentials/platform/database/DB2Platform.java
>>> diff -r1.6 DB2Platform.java
>>> 467c467
>>> < v.addElement("CURRENT TIMESTAMP");
>>> ---
>>> > v.addElement("CURRENT DATE");
>>>
>>> --------------------------------------------------
>>> I tried to add a test case to cover EJBQL expression CURRENT_DATE to
>>> JUnitEJBQLDateTimeTestSuite as follows.
>>>
>>> public void testCurrentDate() {
>>> List result = createEntityManager().createQuery("SELECT
>>> OBJECT(o) FROM DateTime o WHERE o.date = CURRENT_DATE").
>>> getResultList();
>>> assertTrue("There should be one result", result.size() == 1);
>>> }
>>>
>>> The test passes on derby and db2 but fails on oracle. This is because on
>>> Oracle ExpressionOperator.Today is mapped to SYSDATE which returns both
>>> date and time and hence the generated sql (select ... from
>>> CMP3_DATE_TIME where SQL_DATE = SYSDATE) for above ejbql does not return
>>> any rows . I think we should switch mapping of ExpressionOperator.Today
>>> to TO_DATE(SYSDATE) on OraclePlatform. What do you think?
>>>
>>> Thanks,
>>> Mitesh
>>>
>>>
>>>
>>>
>>>
>>
>
>