(.contains (transient #{}) 17) ==> true!?!

[Tassilo Horn]

Hi all,

I've implemented IEditableCollection support for ninjudd's
PersistentOrderedSet. But when using that, my application delivered
wrong results. See <87hbb6c...@member.fsf.org> and follow-ups.

I was able to track it down to the strangeness in the subject:

(.contains (transient (hash-set)) foo)

returns true for any object foo, that is, the empty transient hash-set
contains everything.

The above clojure code is equivalent to this java code, which I used in
the PersistentOrderedSet extension:

((ITransientSet) PersistentHashSet.EMPTY.asTransient()).contains(o)

For now, I worked around this strange behavior by checking the count()
of the TransientHashSet backing my TransientOrderedSet implementation in
addition to contains().

But anyway: Is that behavior intended and facilitated internally
somehow? If so, is there some documentation about the rationale?

Bye,
Tassilo

[Ken Wesson]

At first blush, the answer appears to be "no".

Here is APersistentSet.contains:

public boolean contains(Object key){
return impl.containsKey(key);
}

Here is ATransientSet.contains:

public boolean contains(Object key) {
return this != impl.valAt(key, this);
}

This should probably use a sentinel; it's possible to put a transient
map into itself, at least in principle. But the real problem:

user=> (.valAt (.asTransient clojure.lang.PersistentHashMap/EMPTY) 1 42)
nil

Oops! This should have returned 42, not nil!

The actual error is on line 278 of PersistentHashMap.java:

Object doValAt(Object key, Object notFound) {
if (key == null)
if (hasNull)
return nullValue;
else
return notFound;
if (root == null)
return null;
return root.find(0, Util.hash(key), key, notFound);
}

This obviously should say

if (root == null)
return notFound;

!!!

[Alan]

Thanks Ken, I found this too when I saw Tassilo's problem on irc
(didn't notice it was on ML as well). I was going submit a patch for
this, but if you've already done so let me know and I won't.

On Mar 14, 12:37 pm, Ken Wesson <kwess...@gmail.com> wrote:

> On Mon, Mar 14, 2011 at 4:13 PM, Tassilo Horn <tass...@member.fsf.org> wrote:
> > Hi all,
>
> > I've implemented IEditableCollection support for ninjudd's
> > PersistentOrderedSet.  But when using that, my application delivered

> > wrong results.  See <87hbb6c4qf....@member.fsf.org> and follow-ups.

[Ken Wesson]

On Mon, Mar 14, 2011 at 4:41 PM, Alan <al...@malloys.org> wrote:
> Thanks Ken, I found this too when I saw Tassilo's problem on irc
> (didn't notice it was on ML as well). I was going submit a patch for
> this, but if you've already done so let me know and I won't.

I don't have a CA so no, I haven't. Go ahead.

[Alan]

On Mar 14, 2:10 pm, Ken Wesson <kwess...@gmail.com> wrote:

> On Mon, Mar 14, 2011 at 4:41 PM, Alan <a...@malloys.org> wrote:
> > Thanks Ken, I found this too when I saw Tassilo's problem on irc
> > (didn't notice it was on ML as well). I was going submit a patch for
> > this, but if you've already done so let me know and I won't.
>
> I don't have a CA so no, I haven't. Go ahead.

It's at http://dev.clojure.org/jira/browse/CLJ-757 if anyone wants to
follow along.

[Tassilo Horn]

Hi all,

for testing my FunQL clojure graph query language against another query
language (GReQL), I want to write tests like that:

(defcomparisontest class-count-restricted (gbjg)
;; GReQL version
"count(V{ClassDefinition})"
;; FunQL variant 1
(vcount (gbjg) :cls 'ClassDefinition)
;; FunQL variant 2
(count (vseq (gbjg) :cls 'ClassDefinition)))

So you provide the name of the test, the graph the query is evaluated
on, a GReQL reference query, and arbitrary many FunQL forms that have to
calculate the exact same result.

I had such a macro that expects exactly one funql form, but while trying
to extend that to arbitrary many funqls, I've reached the limit of my
macro foo.

That's what I have come up with:

(defmacro defcomparisontest
"Define a GReQL/FunQL comparison test with name n on graph g that asserts the
equality of the results evaluating greql and all funqls."
[n g greql & funqls]
`(deftest ~n
~g ;; ensure the graph is loaded, if it is given as memoized function.
(println "####################################################")
(println "Comparison test:" ~(name n))
(let [gr# (do
(print "GReQL evaluation time:")
(jvalue-unpack (time (greql-eval ~g ~greql))))]
(doseq [funql# '~funqls]
(print "FunQL evaluation time:")
(is (= gr#
(time (let [r# funql#]
(if (instance? clojure.lang.LazySeq r#)
(doall r#)
r#)))))))))

Here, the problem is that the comparison compares the evaluation result
of the greql query against gr# the unevaluated funql form.

So I replaced the "(let [r# funql#]" with "(let [r# (eval funql#)]", but then
I seem to have namespace issues. I get errors like

java.lang.Exception: Unable to resolve symbol: vcount in this context
java.lang.Exception: Unable to resolve symbol: vseq in this context

which both are functions defined in funql.core, which my funql.test.core
namespace uses; (:use [funql.core] :reload) in the ns definition.

Could anyone with some advanced macro foo enlighten me how to do it
right?

By the way: in general, I would prefer if the expansion would contain a
sequence of (do ...) blocks with one assertion, one for each funql in
funqls, instead of the doseq.

Thanks a bunch in advance!
Tassilo

[Tassilo Horn]

Hi all,

raek helped me on IRC, and that's what finally works:

(defmacro defcomparisontest
"Define a GReQL/FunQL comparison test with name n on graph g that asserts the
equality of the results evaluating greql and all funqls."
[n g greql & funqls]
`(deftest ~n
~g ;; ensure the graph is loaded, if it is given as memoized function.
(println "####################################################")
(println "Comparison test:" ~(name n))
(let [gr# (do
(print "GReQL evaluation time:")
(jvalue-unpack (time (greql-eval ~g ~greql))))]

(doseq [funql-fn# ~(vec (for [funql funqls] `(fn [] ~funql)))]

(print "FunQL evaluation time:")
(is (= gr#

(time (let [r# (funql-fn#)]

(if (instance? clojure.lang.LazySeq r#)
(doall r#)
r#)))))))))

Bye,
Tassilo