[#CLJ-1242] = on sorted collections with different key types incorrectly throws - Clojure JIRA

Details

Type: Defect
Status: Reopened
Priority: Major
Resolution: Unresolved
Affects Version/s: Release 1.5, Release 1.6
Fix Version/s: Release 1.9
Component/s: None
Labels:
- collections

Patch:
Code and Test
Approval:
Incomplete

Description

Comparing a sorted-set with numbers to a set with keywords is not symmetric:

user=> (= #{:a} (sorted-set 1))
false
user=> (= (sorted-set 1) #{:a})
ClassCastException java.lang.Long cannot be cast to clojure.lang.Keyword  clojure.lang.Keyword.compareTo (Keyword.java:109)

The latter case should return false instead of throwing.

Cause: APersistentMap.equiv() and APersistentSet.equiv() do not expect this exception be thrown from the containsKey()/contains() check.

Proposed: It would probably be best for PersistentTreeMap and PersistentTreeMap to implement equiv() and handle that possibility appropriately. Should also consider similar changes in equals() if necessary.

See also: ~~CLJ-1983~~ (downstream example with clojure.data/diff)

More Options
- Sort By Name
- Sort By Date
- Ascending
- Descending
- Download All

Attachments

0001-fix-for-CLJ-1242-tests.patch

31/Jul/13 7:33 AM

3 kB

Nicola Mometto

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

OHTA Shogo added a comment - 31/Jul/13 8:02 PM

PersistentVector also has the same problem.

user=> (compare [1] [:a])
java.lang.ClassCastException: clojure.lang.Keyword cannot be cast to java.lang.Number

The cause of this problem is that Util.compare() casts the second argument
to Number without checking its type when the first argument is a Number.

Show

OHTA Shogo added a comment - 31/Jul/13 8:02 PM PersistentVector also has the same problem. user=> (compare [1] [:a]) java.lang.ClassCastException: clojure.lang.Keyword cannot be cast to java.lang.Number The cause of this problem is that Util.compare() casts the second argument to Number without checking its type when the first argument is a Number.

Hide

Permalink

OHTA Shogo added a comment - 31/Jul/13 8:26 PM

Umm, my brain was not working right.
Util.compare() should raise an Exception when the arguments' type are different.

Show

OHTA Shogo added a comment - 31/Jul/13 8:26 PM Umm, my brain was not working right. Util.compare() should raise an Exception when the arguments' type are different.

Hide

Permalink

François Rey added a comment - 02/May/15 4:44 PM

Upvoting.
Here's a instance of this bug in codox:
https://github.com/weavejester/codox/issues/91

Show

François Rey added a comment - 02/May/15 4:44 PM Upvoting. Here's a instance of this bug in codox: https://github.com/weavejester/codox/issues/91

Hide

Permalink

Stuart Halloway added a comment - 30/Jul/15 11:09 AM

The behavior of get is consistent with Java collections, so I think changing that expectation should be considered a feature request and not a bug.

The fix for equals should be informed by the approach taken in the JDK, where the implementation of equals (not get) has exception catchers.

Show

Stuart Halloway added a comment - 30/Jul/15 11:09 AM The behavior of get is consistent with Java collections, so I think changing that expectation should be considered a feature request and not a bug. The fix for equals should be informed by the approach taken in the JDK, where the implementation of equals (not get) has exception catchers.

Hide

Permalink

Alex Miller added a comment - 21/Jan/16 10:33 AM

I re-focused this ticket on just the equality aspect. The other request regarding `get` with a value of a different type is consistent with Java behavior and should be considered "as designed" - a separate enhancement ticket could be considered for that one.

user=> (def s (java.util.TreeSet.))
#'user/s
user=> (.add s 1)
true
user=> (.contains s "a")
ClassCastException java.lang.Long cannot be cast to java.lang.String  java.lang.String.compareTo (String.java:108)

Show

Alex Miller added a comment - 21/Jan/16 10:33 AM I re-focused this ticket on just the equality aspect. The other request regarding `get` with a value of a different type is consistent with Java behavior and should be considered "as designed" - a separate enhancement ticket could be considered for that one.

user=> (def s (java.util.TreeSet.))
#'user/s
user=> (.add s 1)
true
user=> (.contains s "a")
ClassCastException java.lang.Long cannot be cast to java.lang.String  java.lang.String.compareTo (String.java:108)

Hide

Permalink

Alex Miller added a comment - 19/Jul/16 2:00 PM

oops, sorry for the close/reopen.

Show

Alex Miller added a comment - 19/Jul/16 2:00 PM oops, sorry for the close/reopen.

Hide

Permalink

Rich Hickey added a comment - 19/Aug/16 10:47 AM

Stu - link for "The fix for equals should be informed by the approach taken in the JDK, where the implementation of equals (not get) has exception catchers." ?

Show

Rich Hickey added a comment - 19/Aug/16 10:47 AM Stu - link for "The fix for equals should be informed by the approach taken in the JDK, where the implementation of equals (not get) has exception catchers." ?

Hide

Permalink

Alex Miller added a comment - 22/Sep/16 1:45 PM

I don't think this change is good in its current location - should reconsider alternative impl based on the proposed suggestion.

Show

Alex Miller added a comment - 22/Sep/16 1:45 PM I don't think this change is good in its current location - should reconsider alternative impl based on the proposed suggestion.

People

Assignee:

Unassigned

Reporter:

Nicola Mometto

Vote (6)

Watch (4)

Dates

Created:

31/Jul/13 7:33 AM

Updated:

Thursday 1:45 PM