[rules-dev] BUG: [5.3.0.Final] CollectSetAccumulateFunction should probably use IdentityHashMap internally

[SirMungus]

Edson Tirelli-4 wrote
> 
> a) Please note that clashes objects was taken into account in the design
> of the solution, but there is a bug in there.
> 
I eventually surmised that from seeing the use of MutableInt within it.

Edson Tirelli-4 wrote
> 
> b) If you look at [1] you will see that the value that is being collected
> is already cached by the engine by fact handle ID and used on reverse. So
> that takes care of expressions, for instance, where a new object is
> created every time. For instance:
> 
> collectSet( $firstName + $lastName )
> 
> In that case, every time the expression is evaluated, it will create a new
> immutable String object that we tract by fact ID, cache it, and properly
> manage with equality semantics.
>     *    *    *
> d) The problem is that we are collecting *sets* of objects, and because of
> that, we have to use equality semantics to check for duplicates. I was
> using a java Map to track that, and that works find for the use case (b)
> above, but obviously breaks on use case (c). Moving to identity does not
> seem correct to me as it would work for use case (c), but no longer work
> for (b).
> 
> I don't have a solution yet, as I am involved in another large complex
> task, but I will re-evaluate this as soon as possible. Meanwhile, if
> someone has a suggestion that supports both cases above, please let us
> know.
> 
Your scenario (b) is a very interesting point.  I'm getting a bit beyond my
depth here, but when the existing node in the RETE tree changes such that it
realizes it will no longer contribute to the rule, does it re-evaluate the
"$firstname + $lastname" and then try to remove it, or does it use a cached
FactHandle from its prior evaluation.  If it re-evaluates, I agree that it
is a problem.  If it uses the FactHandle from before, then the
IdentityHashMap implementation should be fine, because the equals()
semantics will be applied in the getResults().  Your text above reads to me
as though the latter is the case.  So, aren't we fine here?

--
View this message in context: http://drools.46999.n3.nabble.com/BUG-5-3-0-Final-CollectSetAccumulateFunction-should-probably-use-IdentityHashMap-internally-tp3774079p3784627.html
Sent from the Drools: Developer (committer) mailing list mailing list archive at Nabble.com.