You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@clerezza.apache.org by Reto Bachmann-Gmuer <re...@trialox.org> on 2010/06/03 15:05:35 UTC

-1 Re: svn commit: r945091 - /incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java

Sorry for opposing so lately, but:

-  I don't see what bug this patch is supposed to fix
-  It seems that with this would also remove trples from members other
than the one at position 0. The class is very pporly documented, but
its quite clear that there is only one graph that can be modified in
the union, the one at position 0
-  having lastRetirned as instance field of the iterator is pointless
after this patch

reto

On Mon, May 17, 2010 at 1:29 PM,  <mi...@apache.org> wrote:
> Author: mir
> Date: Mon May 17 11:29:21 2010
> New Revision: 945091
>
> URL: http://svn.apache.org/viewvc?rev=945091&view=rev
> Log:
> bug fixed (by andre)  in UnionGraph.remove()
>
> Modified:
>    incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java
>
> Modified: incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java
> URL: http://svn.apache.org/viewvc/incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java?rev=945091&r1=945090&r2=945091&view=diff
> ==============================================================================
> --- incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java (original)
> +++ incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java Mon May 17 11:29:21 2010
> @@ -100,7 +100,7 @@ public class UnionMGraph extends Abstrac
>                                if (lastReturned == null) {
>                                        throw new IllegalStateException();
>                                }
> -                               UnionMGraph.this.remove(lastReturned);
> +                               currentBaseIter.remove();
>                                lastReturned = null;
>                        }
>                };
>
>
>

Re: -1 Re: svn commit: r945091 - /incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java

Posted by Reto Bachmann-Gmuer <re...@trialox.org>.
Sounds plausible. The we should do it this way but check that the
iterator is from the the basegraph at position 0.

Cheers,
reto

On Thu, Jun 3, 2010 at 5:09 PM, Manuel Innerhofer
<ma...@trialox.org> wrote:
> As far as I remember the bug arises when using jena tdb. If removing and
> the currentBaseIter is the one at position 0, then a
> ConcurrentModificationException is thrown, because the remove()-method
> of the TripleCollection is used instead of the remove()-method of the
> Iterator.
>
> On Thu, 2010-06-03 at 15:05 +0200, Reto Bachmann-Gmuer wrote:
>> Sorry for opposing so lately, but:
>>
>> -  I don't see what bug this patch is supposed to fix
>> -  It seems that with this would also remove trples from members other
>> than the one at position 0. The class is very pporly documented, but
>> its quite clear that there is only one graph that can be modified in
>> the union, the one at position 0
>> -  having lastRetirned as instance field of the iterator is pointless
>> after this patch
>>
>> reto
>>
>> On Mon, May 17, 2010 at 1:29 PM,  <mi...@apache.org> wrote:
>> > Author: mir
>> > Date: Mon May 17 11:29:21 2010
>> > New Revision: 945091
>> >
>> > URL: http://svn.apache.org/viewvc?rev=945091&view=rev
>> > Log:
>> > bug fixed (by andre)  in UnionGraph.remove()
>> >
>> > Modified:
>> >    incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java
>> >
>> > Modified: incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java
>> > URL: http://svn.apache.org/viewvc/incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java?rev=945091&r1=945090&r2=945091&view=diff
>> > ==============================================================================
>> > --- incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java (original)
>> > +++ incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java Mon May 17 11:29:21 2010
>> > @@ -100,7 +100,7 @@ public class UnionMGraph extends Abstrac
>> >                                if (lastReturned == null) {
>> >                                        throw new IllegalStateException();
>> >                                }
>> > -                               UnionMGraph.this.remove(lastReturned);
>> > +                               currentBaseIter.remove();
>> >                                lastReturned = null;
>> >                        }
>> >                };
>> >
>> >
>> >
>
>
>

Re: -1 Re: svn commit: r945091 - /incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java

Posted by Manuel Innerhofer <ma...@trialox.org>.
As far as I remember the bug arises when using jena tdb. If removing and
the currentBaseIter is the one at position 0, then a
ConcurrentModificationException is thrown, because the remove()-method
of the TripleCollection is used instead of the remove()-method of the
Iterator.

On Thu, 2010-06-03 at 15:05 +0200, Reto Bachmann-Gmuer wrote:
> Sorry for opposing so lately, but:
> 
> -  I don't see what bug this patch is supposed to fix
> -  It seems that with this would also remove trples from members other
> than the one at position 0. The class is very pporly documented, but
> its quite clear that there is only one graph that can be modified in
> the union, the one at position 0
> -  having lastRetirned as instance field of the iterator is pointless
> after this patch
> 
> reto
> 
> On Mon, May 17, 2010 at 1:29 PM,  <mi...@apache.org> wrote:
> > Author: mir
> > Date: Mon May 17 11:29:21 2010
> > New Revision: 945091
> >
> > URL: http://svn.apache.org/viewvc?rev=945091&view=rev
> > Log:
> > bug fixed (by andre)  in UnionGraph.remove()
> >
> > Modified:
> >    incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java
> >
> > Modified: incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java
> > URL: http://svn.apache.org/viewvc/incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java?rev=945091&r1=945090&r2=945091&view=diff
> > ==============================================================================
> > --- incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java (original)
> > +++ incubator/clerezza/trunk/org.apache.clerezza.parent/org.apache.clerezza.rdf.utils/src/main/java/org/apache/clerezza/rdf/utils/UnionMGraph.java Mon May 17 11:29:21 2010
> > @@ -100,7 +100,7 @@ public class UnionMGraph extends Abstrac
> >                                if (lastReturned == null) {
> >                                        throw new IllegalStateException();
> >                                }
> > -                               UnionMGraph.this.remove(lastReturned);
> > +                               currentBaseIter.remove();
> >                                lastReturned = null;
> >                        }
> >                };
> >
> >
> >