Open xificurC opened 4 years ago
I wanted to compare to datomic but it behaves even differently:
user=> (db/q '[:find ?id :where [[:id :b] :id ?id]] (db/db c))
IllegalArgumentExceptionInfo :db.error/not-a-binding-form Invalid binding form: :id datomic.error/arg (error.clj:57)
Thanks for reporting! What do you thinkg the correct behaviour should be?
I actually haven't thought of that, I guess I was expecting you to have a preference, wrt backward compatibility.
Thinking about lookup refs in a recursive rule I'd expect the unification to stop, not throw. Therefore allowing a non-matching lookup ref seems reasonable.
Reading datomic's docs they allow lookup refs in a parameterized query. I don't have a laptop right now, if you want to check to be compatible, feel free.
I checked with datomic-free and it actually throws. The model is a bit different but the point is the same:
user=> (d/q '[:find ?foo :in $ ?id :where [?id :foo ?foo]] (d/db c) [:id :c])
Execution error (IllegalArgumentException) at datomic.datalog/resolve-id (datalog.clj:272).
Cannot resolve key: [:id :c]
I find this inconsistent because I thought of lookup refs as shortcuts, i.e. [[:id :c] :foo ?foo]
to be equivalent to [[?e :id :c] [?e :foo ?foo]]
. I guess they went the other way - the datomic blog post seems to suggest they were thinking of this when using the non-query APIs. There it might make more sense to throw, although I'm not exactly a fan of that either, but I digress.
What about a rule like [(x ?a) [?a :attr ?id] (y [:id ?id])]
? In theory the :attr
value should be a ref but for various reasons it might be modeled differently where a rule like this could make sense. Would one want to throw there? I guess not.
It seems it's hard to model this in a way that keeps backward compatibility and datomic compatibility and consistency at the same time. If you care more about:
I'd understand if you didn't want to break userspace or if some clients expected datomic compatibility, but my vote would go for consistency, then backward compatibility, then datomic.
What about a rule like [(x ?a) [?a :attr ?id] (y [:id ?id])]
So the idea of lookup refs is that they are resolved only when they appear at a statically known entity id positions. In this case you have no idea what y
is going to do with its argument so it won’t get resolved.
Lookup refs are a bit of a hack added at later stages, that’s why they might behave inconsistently. They are resolved in some places Datomic authors thought about, but would simply not work in some others.
I see, that should make reasoning about them a bit simpler.
Since you asked for my idea of correct behavior I would suggest to not throw either way. This would be a difference compared to datomic. Either way, I'd settle with a solution that behaves the same way when sending the lookup ref as an argument and when inlining it in the query. I'm open to creating a PR if you tell me your final decision and give some pointers what to poke at.
There's a bunch of similarly looking issues from the past but none seem to be this particular case. A bad lookup ref inside the query throws an exception but the same query returns an empty set when the lookup ref is passed in as a parameter. Since the resulting query is identical I'd expect the semantics to be identical too.