Hi, Michael,
My change is not affected. I will check them in today.
Thanks.
Jielin
Michael Bouschen wrote:
> Hi Jielin,
>
> the changes look good!
>
> Just a heads up: Wonseok plans to change some Date values in the test
> data of entity-persistence-tests (see mail thread with subject "Some
> tests in JUnitEJBQLDateTimeTestSuite failed on Oracle"). Is your
> change affected by this?
>
> Regards Michael
>
>
>> Hi, Michael,
>>
>> I made the change.
>> I added two unit tests in the past to test update embedded field and
>> test unknown enumerated class constant. Would you please also review
>> the change?
>>
>> Thanks.
>>
>> Jielin
>>
>> Michael Bouschen wrote:
>>
>>> Hi Jielin,
>>>
>>> looks good! Just one comment about class DotNode.java, method
>>> checkNavigation:
>>> For the invalidNavigation exception I propose to take line and
>>> column information from the specified node parameter (so
>>> node.getLine(), node.getColumn()). The other exception
>>> invalidCollectionNavigation in method checkNavigation is fine.
>>>
>>> Regards Michael
>>>
>>>> Hi, Micahel,
>>>>
>>>> Attached is the change based on your comments. Would you please
>>>> review them?
>>>>
>>>> Thanks.
>>>>
>>>> Jielin
>>>>
>>>> Michael Bouschen wrote:
>>>>
>>>>> Hi Jielin,
>>>>>
>>>>> please find my remarks below. Most of the remarks are about taking
>>>>> line and column information from a different node than the current
>>>>> one.
>>>>>
>>>>> EJBQLException.java/EJBQLExceptionResource.java:
>>>>> - I propose to add line and column information for error message
>>>>> 8018 invalidMultipleUseOfSameParameter. Method defineParameterType
>>>>> of ParseTreeContext could take line and column as additional
>>>>> parameters and pass it to the error message. The method is called
>>>>> from ParameterNode.
>>>>> - The same appies for error message 8019
>>>>> multipleVariableDeclaration. Methods registerSchema and
>>>>> registerJoinVariable of ParseTreeContext could get additional line
>>>>> and column parameters.
>>>>>
>>>>> EJBQLExceptionResource.java:
>>>>> - The error message of 8032 does not use the attribute name. I
>>>>> propose to change the text to: Invalid access of attribute [{0}]
>>>>> in SET clause target [{1}], only state fields ...
>>>>> - The error message of entry 8033 is broken.
>>>>> - In the definition of field contents there is a typo in the
>>>>> second entry from the bottom: it should have the number 8037
>>>>> instead of 8034.
>>>>> - The error message of 8038 should start with an 'a': "line {1},
>>>>> column {2}: a problem ...
>>>>>
>>>>> DotNode.java:
>>>>> Using getLine() and getColumn in the class DotNode returns the
>>>>> position of the dot itself. In some cases we can give a better
>>>>> error message when using the position of the erroneous syntax
>>>>> element mentioned in the error message. E.g. if the error message
>>>>> says unknown attribute we should use the position of the attribute
>>>>> instead of the position of the dot:
>>>>> - Method validate, exception unknownAttribute: right.getLine(),
>>>>> right.getColumn()
>>>>> - Method validate, exception resolveEnumConstant: right.getLine(),
>>>>> right.getColumn()
>>>>> - Method validate, exception aliasResolutionException:
>>>>> leftMost.getLine(), leftMost.getColumn()
>>>>> - Method checkNavigation, exception invalidCollectionNavigation
>>>>> right.getLine(), right.getColumn()
>>>>>
>>>>> EqualsAssignmentNode.java:
>>>>> - Method validateTarget
>>>>> I propose to use the line and column information of the attribute
>>>>> node for the invalidSetClauseTarget exception. You could intoduce
>>>>> a new local variable attributeNode and then use
>>>>> attributeNode.getLine() and attributeNode.getColumn() for the
>>>>> error message:
>>>>> Node attributeNode = (AttributeNode)node.getRight();
>>>>> String attribute = attributeNode.getAttributeName();
>>>>> ...
>>>>> - Method validateNavigation
>>>>> Same as above: use attributeNode.getLine() and
>>>>> attributeNode.getColumn()
>>>>> Node attributeNode = (AttributeNode)qualifier.getRight();
>>>>> String attribute = attributeNode.getAttributeName();
>>>>> ...
>>>>>
>>>>> GroupByNode.java:
>>>>> - Method validate: use line and column information of the select
>>>>> expression (selectExpr.getLine() and selectExpr.getColumn()).
>>>>>
>>>>> HavingNode.java:
>>>>> - Method validate: use line and column information of the having
>>>>> expression (having.getLine() and having.getColumn()).
>>>>>
>>>>> InNode.java:
>>>>> - Method validate: use line and column information of the node in
>>>>> the IN expression list (node.getLine() and node.getColumn()).
>>>>>
>>>>> ModNode.java:
>>>>> - Method validate: use line and column information of the mode
>>>>> argument nodes (left.getLine() and left.getColumn(),
>>>>> denominator.getLine() and denominator.getColumn()).
>>>>>
>>>>> OrderByItemNode.java:
>>>>> - Method validate: use line and column information of the order by
>>>>> item node (orderByItem.getLine() and orderByItem.getColumn()).
>>>>>
>>>>> Regards Michael
>>>>>
>>>>>> Hi, Michael,
>>>>>>
>>>>>> Attched is the changes for issue 1355. The issue is about adding
>>>>>> line and column information when there is any error in a Java
>>>>>> Persistence query results in a EJBQLException.
>>>>>>
>>>>>> Basically, there are two type of error handling for jpql in the
>>>>>> current code: one is thrown from
>>>>>> EJBQLParser.handleANTLRException(); the other is all validation
>>>>>> methods in the Node subclasses. Line and column information are
>>>>>> added in these two category. If the same error method is used in
>>>>>> other place and don't have line and column information, a new
>>>>>> method is added with same same having suffix 2.
>>>>>>
>>>>>> Would you please review the changes?
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>> Jielin
>>>>>
>>>>>
>>>>>
>>>>>