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
>>
>>
>>