Thank you for looking at the patch :) Knut Anders Hatlen <[EMAIL PROTECTED]> writes:
> Hi Dyre, > > I had a quick look at the patch, and there was one thing I didn't > quite understand in IndexToBaseRowNode.init(). When heapReferencedCols > is null, allReferencedCols is also set to null. Intuitively, I would > say allReferencedCols should be equal to indexReferencedCols in that > case (since A U NULL == A). Or maybe indexReferencedCols is always > null when heapReferencedCols is null? Well, intuitively I would agree with you. The thing is that I have tried to duplicate the logic previously found in java/engine/org/apache/derby/impl/sql/execute/IndexRowToBaseRowResultSet.java It seems like there is no handling of the case when heapReferencedCols==null but indexReferencedCols!=null. But I don't know if this is because it is known not to happen, or if it is an oversight. I just looked at the old code again, and it seems like this situation will cause the FormatableBitSet copy constructor to be invoked with a null argument. Which should cause an assert in sane mode and an NPE in insane mode... > Also, in the same method, is the "return" necessary? If it isn't, I > think it would be better to remove it since it could easily be > overlooked by someone who adds more code below the if-block later. That return statement should definitely not be there. Thanks. -- dt
