db-ojb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Armin Waibel <arm...@apache.org>
Subject Re: Possible Code Fix to RsIterator problem (WAS: Issue with RsIterator.hasNext() and custom RowReader)
Date Sat, 03 Apr 2004 17:19:05 GMT
Hi Andrew,

Clute, Andrew wrote:
> I have coded a change that I think solves the problem, but I am not really sure if it
fits 
in with the philosophy of OJB.
> 

I agree with you that it's a point of principle to allow/not allow 
RowReader implementation to filter out rows (objects) from the 
ResultSet. If the OJB guys agree, I don't see a problem in apply your 
patch (as far as I can remember some weeks ago I checked in a patch for 
PB#getObjectByQuery(...) to support your special RowReader 
implementation - right?).

The problem with this patch is, that it could/will corrupt all size(), 
fullSize() methods of the OJBIterator if we allow to use RowReader 
implementations which filter out rows from the ResultSet, because the 
size(), fullSize() methods based on the used ResultSet. And this 
could/will lead to additional side-effects, e.g. in PagingIterator.

The problem is that RsIterator is the OJB-object view of the 
DB-ResultSet and RsIterator use methods of ResultSet (or if not 
supported the DB count(*)) to detect the size of the ResultSet.

In my opinion it's a moot point to allow users to implement RowReader 
which filter out rows from the ResultSet. I think the intension of 
RowReader is to allow user to manipulate the retrieved rows (objects). 
So I dither if it is a good idea to allow such implementations or not.

Convince me ;-)

regards,
Armin

> The premise of the change is that the hasNext() method on RsIterator will actively walk
the
  ResultSet until it finds a non-null object that is returned from the 
RowReader, or until
the end of the ResultSet (as opposed to using the .next() value from the 
ResultSet).
And since hasNext() has done the work of reading the object in, it keeps 
it around and
next() returns that result.
> 
> It seems like a complicated solution, but if custom RowReaders are allowed to filter
out 
objects and return null, either RsIterator has to be aware of that and 
skip them, or other
places have to account for a null object being in the result.
> 
> So, this is part philisophical, and part pragmatic -- should custom RowReaders be able
to 
return null? And if so, who is responsible for checking that.
> 
> Either way, here is my code changes to RsIterator.java
> 
> -Andrew
> 
> 
> -- add a local variable:
> 
>     /**
>      * Andrew Clute: next object to be returned from the next() call -- is 
>      * populated in the hasNext() method
>      */
>     private Object nextObject = null;
> 
> .....
> 
> -- hasNext() and next() methods:
>  /**
>      * returns true if there are still more rows in the underlying ResultSet.
>      * Returns false if ResultSet is exhausted.
>      */
>     public synchronized boolean hasNext()
>     {
>         try
>         {
>             if (!isHasCalledCheck())
>             {
>                 setHasCalledCheck(true);
>                 if (!getRsAndStmt().m_rs.next()){
>                     setHasNext(false);
>                 }
>                 /*
>                     Andrew Clute: Because a custom RowReader could in fact filter
>                     out a result from the RecordSet, we need to give that RowReader a
chance
>                     to do it's work before we answer this method. The idea being that
if 
>                     a filtered-out object where to exist in the non-last position, the
>                     Iterator would skip to the next valid Object -- in essecence ignoring
>                     the filtered-out object.
>                     
>                     Since we have done the work to read the row, we keep it around and

>                     return it as the value for next()
>                 */
>                 else
>                 {
>                     nextObject = getObjectFromResultSet();
>                     while (nextObject == null && getRsAndStmt().m_rs.next())
>                     {
>                          nextObject = getObjectFromResultSet();
>                          m_current_row++;
>                     }
>                     setHasNext(nextObject != null);
>                 }            
>                             
>                 if (!getHasNext())
>                 {
>                     releaseDbResources();
>                 }
>             }
>         }
>         catch (Exception ex)
>         {
>             setHasNext(false);
>             releaseDbResources();
>             if(ex instanceof ResourceClosedException)
>             {
>                 throw (ResourceClosedException)ex;
>             }
>         }
>         if (logger.isDebugEnabled())
>             logger.debug("hasNext() -> " + getHasNext());
> 
>         return getHasNext();
>     }
> 
>     /**
>      * moves to the next row of the underlying ResultSet and returns the
>      * corresponding Object materialized from this row.
>      */
>     public synchronized Object next() throws NoSuchElementException
>     {
>         try
>         {
>             if (!isHasCalledCheck())
>             {
>                 hasNext();
>             }
>             setHasCalledCheck(false);
>             if (getHasNext())
>             {
>                 /*
>                     Andrew Clute: Because hasNext() has been modified to check to ensure
>                     a valid object will be returned from the RowReader, we are now just

>                     returning the result given to us from the RowReader in hasNext()
>                 */
>                 Object obj = nextObject;
>                 nextObject = null;
>                 m_current_row++;
> 
>                 // Invoke events on PersistenceBrokerAware instances and listeners
>                 // set target object
>                 getAfterLookupEvent().setTarget(obj);
>                 getBroker().fireBrokerEvent(getAfterLookupEvent());
> 
>                 return obj;
>             }
>             else
>             {
>                 throw new NoSuchElementException("inner hasNext was false");
>             }
>         }
>         catch (Exception ex)
>         {
>             releaseDbResources();
>             // ex.printStackTrace();
>             if(ex instanceof ResourceClosedException)
>             {
>                 throw (ResourceClosedException) ex;
>             }
>             else
>             {
>                 logger.error("Error while iterate ResultSet for query " + m_queryObject,
ex);
>                 throw new NoSuchElementException("Could not obtain next object: " + ex.getMessage());
>             }
>         }
>     }
> 
> 
> 
> 
> 
> 
> 
> 
> -----Original Message-----
> From: Clute, Andrew [mailto:Andrew.Clute@osn.state.oh.us]
> Sent: Thu 4/1/2004 4:11 PM
> To: OJB Developers List; OJB Users List
> Subject: Issue with Iterator.hasNext() and custom RowReader
>  
> As a little background, I have created a custom RowReader that allows me
> to filter out objects based upon some criteria on that object. More
> specifically, if a deleted date exist on the object, I filter it out and
> return null.
> 
> The idea behind this is that I want to maintain some history in my
> database, but not even have the filtered object available to be used
> outside of OJB. The custom rowreader works pretty well in doing that,
> except I have run into an issue with Iterators returned from PB, and
> also internal functions inside of OJB that use these iterators.
> 
> In my application, I know that the Iterator might have a spot that
> contains a null object, but the it.hasNext() will return true, because
> the null object was placed into there.
> 
> However, there are places inside of OJB where the assumption has been
> made that an object coming out of an Iterator will always be non-null.
> This is probably not a bad assumption as you would assume this would
> never be the case. But, as you can probably tell, what is happening is
> the Iterator thinks it has a next value, but doesn't really know that it
> doesn't until the next() method is called, and the RowReader filters it
> out.
> 
> A good example of where OJB makes this assumption is in
> ReferencePrefetcher.associateBatched(). But this is just one of many.
> 
> 
> Now, I see a couple options:
> 1)Stop using custom RowReaders, and make it known via documentation that
> a RowReader must still return a non-null objects, otherwise other code
> will break
> 2) Fix all the points in other OJB methods where the assumption is made
> that a value that comes out the Iterator is not null, and make it do a
> null-check
> 3) Change RsIterator.next() to be smater and give the RowReader a chance
> to filter out the objects. However, the problem with this is if the
> 'filtered' object exist in the middle of the chain, the rs.next() would
> return false, and the rest of the non-filtered objects would be lost.
> 
> I would love to get some thoughts on this. My guess is that number 1 or
> 2 is the best answers, although 1 seems to be pretty harsh.
> 
> Thanks
> 
> -Andrew
> 
> 
> 
> 
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
> For additional commands, e-mail: ojb-dev-help@db.apache.org
> 
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: ojb-dev-unsubscribe@db.apache.org
For additional commands, e-mail: ojb-dev-help@db.apache.org


Mime
View raw message