Thanks for looking at this so quickly, Rick!
> Thanks for the patch, Dag. Here's my understanding of the general
> problem you have found: When a ResultSet encounters an error at open()
> time, it fails to close its ResultSet children.
That is correct.
> I wonder if there is another version of this bug hanging around in
> the ResultSet.close() methods themselves: If an error occurs during
> the close of one child of a ResultSet, will the other children be
> closed or will they dangle open? It may be that you will bag more
> instances of this bug if you do the following:
> 1) Make the ResultSet.close() methods more defensive so that they
> continue closing the rest of their children even if one child raises
> an error during close().
This could be done, sure. However, can you see a scenario where an
error at close time would be retryable (like time-out at open time) ?
As the code in the patch currently stands, it does not check if the
exception is a a retryable one, admitted, it just goes ahead and
closes the child result set(s). In addition to time-out, I imagine
dead-lock would be a possible candidate candidate, as well as missing
permissions (which is why the new defensive code does not check for
> 2) Have the ResultSet call its own close() method if an error occurs
> during open().
What I did for now, was to make sure the open method does not mark the
result set as open before all other actions have been successfully
performed, cf. the change to UnionResultSet. Would calling close() in
addition add value to my approach?
(I was hesitant to apply to much "power", here without understanding if it's needed
Perhaps, for those result sets which also make calls to getNextRowCore before they complete openCore (e.g. GroupedAggregateResultSet) need some extra logic
like that typically contained in close, like e.g. clearCurrentRow, i.e. there is more state to roll back than just closing the underlying result sets. I'll have a look..