Hi Jielin,
yes, please file a new issue for the validation.
Regards Michael
> Hi, Michael,
>
> Please see my comments below.
>
> Michael Bouschen wrote:
>
>> Hi Jielin,
>>
>> this is an interesting issue, because I think we have a more general
>> validation problem here. Today the compiler does not check whether
>> the operand types of a comparison expression (=, <>, <, ...) are
>> compatible. This is caught by the underlying query runtime. I think
>> we should add this validation to the query compiler. But this is not
>> part of issue 269, it should be a separate issue.
>
> Do we have a separate issue for this? If not, I am going to file a new
> issue for validation to the query compiler.
>
>>
>> I see two problems with the proposed fix:
>>
>> (1) The fix only catches the case where the enum value is on the left
>> and the non-enum is on the right, e.g.
>> SELECT e FROM Employee e WHERE e.status = 1
>> We also need to catch the swapped case:
>> SELECT e FROM Employee e WHERE 1 = e.status
>>
>> I propose to change the code in EqualsNode.validate to something like:
>> if (typeHelper.isEnumType(leftType) &&
>> !typeHelper.isEnumType(rightType)) {
>> throw
>> EJBQLException.invalidEnumEqualExpression(typeHelper.getTypeName(leftType));
>>
>> } else if (typeHelper.isEnumType(rightType) &&
>> !typeHelper.isEnumType(leftType)) {
>> throw
>> EJBQLException.invalidEnumEqualExpression(typeHelper.getTypeName(rightType));
>>
>> }
>>
> I will made the change.
>
>> (2) The new error message says: "Invalid enum equal expression,
>> expected {0} constants by using parameter binding.". This sounds like
>> I have to compare an enum type field with an input parameter. But the
>> spec allows several other enum comparisons:
>> - An enum literal, e.g.
>> SELECT e FROM Employee e WHERE e.status =
>> mypackage.EmployeeStatus.FULL_TIME
>> - Another enum type field.
>> - A subquery returning a single enum value.
>> Maybe you can change the error message to something like:
>> Invalid enum equal expression, cannot compare enum value of type {0}
>> with a non enum value of type {1}.
>
> I will made the change.
>
> Thanks.
>
> Jielin
>
>>
>> What do you think?
>>
>> Regards Michael
>>
>>> Hi, Michael,
>>>
>>> Here is the modified code for
>>> https://glassfish.dev.java.net/issues/show_bug.cgi?id=269. It does
>>> not throw correct error message when compare enum with wrong data
>>> type in the query.
>>>
>>> Please review the change.
>>>
>>> Thanks.
>>>
>>> Jielin
>>>
>>>
>>>