[ 
https://issues.apache.org/jira/browse/CALCITE-3386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16946247#comment-16946247
 ] 

Haisheng Yuan commented on CALCITE-3386:
----------------------------------------

"CyclicMetadataException is intended for signaling; it is not an error, most of 
the time." - this statement still holds after the fix. There's no change to the 
way how exception is thrown and handled. We just make sure the exception object 
contions the right call stack info so the origin of this signal can be properly 
determined. 

This is a very subtle error. A developer without knowledge of this bug could 
spend hours debugging at completely wrong place before they realize this is a 
known issue and apply the patch. That's a lot to ask.

On the other hand, I am still not sure I understand your concern. Are you worry 
about performance degradation? Do we have any numbers to back it up?

> CyclicMetadataException gives misleading stack trace
> ----------------------------------------------------
>
>                 Key: CALCITE-3386
>                 URL: https://issues.apache.org/jira/browse/CALCITE-3386
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>            Reporter: Zuozhi Wang
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.22.0
>
>          Time Spent: 1.5h
>  Remaining Estimate: 0h
>
> Calcite currently reuses the same instance when throwing 
> CyclicMetadataException as shown below. 
> {code:java}
> /** Singleton instance. Since this exception is thrown for signaling 
> purposes, * rather than on an actual error, re-using a singleton instance 
> saves the * effort of constructing an exception instance. */ 
> @SuppressWarnings("ThrowableInstanceNeverThrown") 
> public static final CyclicMetadataException INSTANCE = new 
> CyclicMetadataException();
> {code}
>  
> Reusing the same exception instance gives the wrong stack trace, see: 
> [https://stackoverflow.com/questions/15090664/is-it-safe-to-store-an-instance-of-an-exception-and-reuse-it]
> The misleading stack trace causes many confusions when debugging. The 
> potential performance impact because of object construction is minor compared 
> to the time spent on debugging. Therefore it should be changed to not reuse 
> the singleton instance.
> A pull request have been opened to fix this issue: 
> [https://github.com/apache/calcite/pull/1484]
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to