core.logic

fd comparison relations behave strange when used with literals

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Environment:

Description

The fd comparison relations always succeed when used with strings or a string and an integer:

user> (run* [q]
 (<=fd "foo" "bar"))
(_.0)
user> (run* [q]
 (<=fd "foo" 1))
(_.0)
user> (run* [q]
 (<=fd 1 "bar"))
(_.0)

IMHO, they should always fail if one argument is not a integer.

Furthermore, you get an exception when comparing integers and the larger integer is given first.

user> (run* [q]
 (<=fd 1 2))
(_.0)  ;; That's correct, but...
user> (run* [q]
 (<=fd 2 1))
IllegalArgumentException No implementation of method: :member? of protocol: #'clojure.core.logic/ISet found for class: nil  clojure.core/-cache-protocol-fn (core_deftype.clj:533)

I'd expect that comparisons of two integers should always work. You can circumvent this issue by unifying the numbers with some logic variables first, but is that really needed?

user> (run* [q]
 (fresh [a b]
  (== a 1)
  (== b 2)
  (<=fd a b)))
(_.0)
user> (run* [q]
 (fresh [a b]
  (== a 1)
  (== b 2)
  (<=fd b a)))
()

The first issue cannot be circumvented using this approach, though.

user> (run* [q]
 (fresh [a b]
  (== a "foo")
  (== b 2)
  (<=fd b a)))
(_.0)
user> (run* [q]
 (fresh [a b]
  (== a "foo")
  (== b 2)
  (<=fd a b)))
(_.0)
user> (run* [q]
 (fresh [a b]
  (== a "foo")
  (== b "bar")
  (<=fd a b)))
(_.0)
user> (run* [q]
 (fresh [a b]
  (== a "foo")
  (== b "bar")
  (<=fd b a)))
(_.0)

Activity

Hide
David Nolen added a comment -

Thanks for the update!

Show
David Nolen added a comment - Thanks for the update!
Hide
Tassilo Horn added a comment -

> I see no particular need to validate inputs.

Well, the reported behavior ("every non-integer is both smaller and greater than anything") is likely to hide bugs in queries. But hey, even if you don't see a particular need, something you did fixed that problem anyhow.

This is with 0.8.0-beta1:

user> (run* [q] (<=fd 23 "1"))
IllegalArgumentException No implementation of method: :ub of protocol: #'clojure.core.logic/IInterval found for class: java.lang.String  clojure.core/-cache-protocol-fn (core_deftype.clj:562)
user> (run* [q] (<=fd "1" 23))
IllegalArgumentException No implementation of method: :lb of protocol: #'clojure.core.logic/IInterval found for class: java.lang.String  clojure.core/-cache-protocol-fn (core_deftype.clj:562)

IMO, both an exception and simply failing would be ok in those situations. Exception like now is probably even better.

> The other issues should be resolved in one of the latest betas.

Yes, it is. Thanks a lot, David! Feel free to close this issue.

Show
Tassilo Horn added a comment - > I see no particular need to validate inputs. Well, the reported behavior ("every non-integer is both smaller and greater than anything") is likely to hide bugs in queries. But hey, even if you don't see a particular need, something you did fixed that problem anyhow. This is with 0.8.0-beta1:
user> (run* [q] (<=fd 23 "1"))
IllegalArgumentException No implementation of method: :ub of protocol: #'clojure.core.logic/IInterval found for class: java.lang.String  clojure.core/-cache-protocol-fn (core_deftype.clj:562)
user> (run* [q] (<=fd "1" 23))
IllegalArgumentException No implementation of method: :lb of protocol: #'clojure.core.logic/IInterval found for class: java.lang.String  clojure.core/-cache-protocol-fn (core_deftype.clj:562)
IMO, both an exception and simply failing would be ok in those situations. Exception like now is probably even better. > The other issues should be resolved in one of the latest betas. Yes, it is. Thanks a lot, David! Feel free to close this issue.
Hide
David Nolen added a comment -

I see no particular need to validate inputs. The other issues should be resolved in one of the latest betas.

Show
David Nolen added a comment - I see no particular need to validate inputs. The other issues should be resolved in one of the latest betas.
Hide
David Nolen added a comment -

Thanks for the report will look into it.

Show
David Nolen added a comment - Thanks for the report will look into it.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: