Tom Ware schrieb:
> Hi Markus,
>
> I took a look at your code and in general, it appears to work better
> than the existing code. I suggest adding one additional operator
> overload:
>
> The line:
> addOperator(ExpressionOperator.simpleFunction(ExpressionOperator.Length,
> "CHAR_LENGTH"));
>
> in the previous version of the platform appears to allow Length to
> work properly. It might be a good idea to add that function.
>
> -Tom
While implementation actually is easy, I do not see the benefit of doing
so, since Expression.Operator.Length is defaulted to "LENGTH" which
works well in SQL Anywhere (SQL Anyhwere supports both, LENGTH and
CHAR_LENGTH). So instead I have added the following line:
this.addOperator(ExpressionOperator.charLength());
which adds support of the "CHAR_LENGTH" operator (the "LENGTH" operator
is supported already by DatabasePlatform, while "CHAR_LENGTH" seems to
be optional -- so now I added it).
In fact I do not understand that the "old" platform should have had a
problem with the default implementation "LENGTH" and why "CHAR_LENGTH"
should be better: "LENGTH" was supported by SQL Anywhere since its early
days, while "CHAR_LENGTH" was added in some newer release to provide
standard SQL compliance (LENGTH is a vendor specific keyword of sybase,
while CHAR_LENGTH is part of the SQL standard). So I would understand
that it was necessary to do it the reverse way, implementing
ExpressionOperator.CharLength as "LENGTH", while I do not understand
what the original author intended with the line you found.
But actually I do not see any benefit in this, since the
entity-persistence-tests passed 601 tests before, and still is passing
the same 601 tests now. So what benefit do the GlassFish users have? Is
"CHAR_LENGTH" part of a future version of the EJB specification? Or is
GlassFish internally running in some kind of "optimized mode" when it
detects both, support for the LENGTH operator AND additional support for
the CHAR_LENGTH operator? So are there more such tricks, then I will add
support for more operators? :-)
As you know I do not want to add any line just for fun. Each of my code
lines shall be of a real use and shall provide the best possible
experience to the user. So if there is no real need or benefit of this
new code line, I'd like to remove it.
Thanks
Markus
>
>
> Tom Ware wrote:
>
>> Hi Markus,
>>
>> Do you also have changes to the
>> VendorNameToPlatformMapping.properties you'd like to include?
>>
>> -Tom
>>
>> Markus KARG wrote:
>>
>>
>>
>>> Tom,
>>>
>>>
>>>
>>>> I am copying the persistence list on this reply since there may be
>>>> some people on the list that are interested.
>>>>
>>>> I believe there are two things we have to do.
>>>>
>>>> 1. You had some concerns about the licensing. We need to alleviate
>>>> those concerns. Hopefully some of the Sun folks have made some
>>>> progress and can share it with us.
>>>> 2. We should get your code reviewed. When we all agree that it is
>>>> in shape to check-in we will check it in. To get the code
>>>> reviewed, I beleive the best strategy is to send it to the
>>>> persistence list and request a review. Let me know if that makes
>>>> sense to you.
>>>>
>>>>
>>> there still is the issue with the jConnect bug that Sybase denies to
>>> fix and the failed test "Query.setFirstResult". Is this a problem
>>> for somebody? Maybe Tom knows how we can turn the sequence from "? =
>>> a" to "a = ?" ?
>>>
>>> If nobody has a problem with that, I do not see a reason why not to
>>> replace SQLAnyWherePlatform by my SQLAnywherPlatform.
>>>
>>> Attached is the code. Please give it a review. :-)
>>>
>>> Thanks
>>> Markus
>>>
>>>
>>>
>>
>>
>>
>>
>