replikativ / datahike

A fast, immutable, distributed & compositional Datalog engine for everyone.
https://datahike.io
Eclipse Public License 1.0
1.63k stars 97 forks source link

[Bug] `d/entity` throws Exception #356

Open MrEbbinghaus opened 3 years ago

MrEbbinghaus commented 3 years ago

From the docs of d/entity :

If entity does not exist, `nil` is returned:

          (entity db -1) ; => nil

But this is not the behavior. (At least with the file backend)

(d/entity @conn 1234567890) ; not in my db
=> #:db{:id 1234567890}
(d/entity @conn -1)
2021-07-05T14:50:30.244Z Jeb ERROR [datahike.db:1064] - Expected number or lookup ref for entity id, got  -1 {:error :entity-id/syntax, :entity-id -1}
Execution error (ExceptionInfo) at datahike.db/entid (db.cljc:1064).
Expected number or lookup ref for entity id, got -1
kordano commented 3 years ago

Thanks, we will adjust the documentation accordingly.

TimoKramer commented 1 year ago

Is this expected behaviour? I would expect to get a nil back? At least for a positive number I would expect that.

jsmassa commented 1 year ago

Yes, it sounds like the behaviour for positive numbers is definitely incorrect and for invalid input like negative numbers we either should adjust the documentation or the behaviour.

MrEbbinghaus commented 1 year ago

I just tried it with Datomic:

(d/entity db -1)
=> #:db{:id -1}
(def nonexistent-entity (d/entity db 2234234))
=> #:db{:id 2234234}
(:foo nonexistent-entity)
=> nil

It makes sense that you can create entities with nonexistent IDs, since attributes are looked up on demand. The Datomic docs even state:

Entities are not suitable for existence tests, which should typically be performed via lookup of a unique identity.


That said: datahike returns nil for nonexistent entities.

TimoKramer commented 1 year ago

Thanks @MrEbbinghaus for clarification. I was struck by something like this:

(d/entity db [:nonexistent "Foo"])

2023-06-29T06:32:13.243Z Zockolette ERROR [datahike.db.utils:110] - Lookup ref attribute should be marked as :db/unique:  [:nonexistent "Foo"] {:error :lookup-ref/unique, :entity-id [:nonexistent "Foo"}
Execution error (ExceptionInfo) at datahike.db.utils/entid (utils.cljc:110).
Lookup ref attribute should be marked as :db/unique: [:nonexistent "Foo"]

And I think this should not throw an Exception, like it should not throw with -1. So this is a bug as I see it. Right?

TimoKramer commented 1 year ago

To match Datomic behaviour it should return an entityID. But to fix this it should at least not throw.