db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Matrigali <mikem_...@sbcglobal.net>
Subject Re: Suspectible swallowing of AssertFailure in BTreeScanInfo
Date Thu, 15 Mar 2007 23:14:20 GMT
I do agree it is ugly, but I can't come up with any history why
it is the way it is (it's been that way for more than 8 years).
It should have been documented.

There are a few options:
1) change it to throw an exception, this leads to a likelyhood that
    some application out there may get an exception where they currently
    have a running/working query (with a very a minor part of the 
statistics being wierd) -- as you found by causing a few of our
    tests to fail.
2) change the code to return something other than -1.  The -1 is just
    an internal implementation.  The real information going back out
    the interface is in the same file, getAllScanInfo where you could
    do something like: if (val == -1) string_to_return = "internal error 
getting tree height".  This could cause trouble for someone trying to
parse an expected number, but there are all sorts of caveats that we
are allowed to change our query plan format.
3) we could document externally what -1 means -- ie. tree height 
unavailable for this query plan.
4) add some comments, and file a JIRA describing the situation.  I am 
not sure if the bug is the caller or the store.
5) if it is just a problem with the scan being open we could check
    for open and set -1 if it is not.  But that actually will be more
    overhead for the normal case where the scan is open, and where we
    now just use the try/catch to catch the error case.

Kristian Waagan wrote:
> Mike Matrigali skrev:
> 
>> It looks to me like a hack to get around a bug in the caller, ie.
>> calling getScanInfo() on a closed conglomerate.
>> Usage documentation for GenericScanController.java!close in
>> java/engine/org/apache/derby/iapi/store/access/GenericScanController.java
>> says that controller should not be used after close().
>>
>> To my knowledge this interface is only used to print out interesting 
>> stuff to runtime statistics, so silently providing as much info as 
>> possible was probably considered better than failing a query.   Did
>> you run full set of tests and only saw one error - or was that just
>> one of many?
> 
> 
> Hi Mike,
> 
> I think a total of three tests failed when I ran the full test suite. I 
> did not print the stack trace for the Throwable then, but all three went 
> away when I went back to swallowing the exception.
> So I don't think this is major problem, and I will add a comment and 
> possibly tighten the catch. This will only affect sane builds anyway. 
> I'll also see how deep the code within the try-catch goes and check for 
> the "right" assertion error if there are lots of possible error situations.
> 
> 


Mime
View raw message