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.
