Closed jonasseglare closed 7 months ago
That makes sense, I also think it is generally better to return nil than to throw errors for parametrized queries even though it is a bit concerning if you use the wrong type in the entity position, it is likely a bug from the user. So maybe throwing an error here is actually better for the user. Not sure. Either way it should be consistent.
As discussed this only occurs when someone makes an inconsistent query and therefore does not need a fix. It is nicer when the query engine just skips incorrect inputs than throwing exceptions.
What version of Datahike are you using?
0.6.1551
What version of Java are you using?
openjdk version "20.0.1" 2023-04-1
What operating system are you using?
Ubuntu 22
What database EDN configuration are you using?
Describe the bug
I discovered an inconsistency in how queries are evaluated. In the file
lookup_refs_test.cljc
, there is the following test:which passes.
Based on that test, I wrote this test, which also passes:
What is potentially problematic here is that
?e
cannot be bound to the value"A"
because it is not a valid entity id reference. The query engine chooses to handle this by ignoring the invalid binding and the result of the query becomes#{}
. However, if I write a similar test,I get this stack trace:
What is the expected behaviour?
For the sake of consistency, I think this test should pass without throwing an exception when added to the test
test-lookup-refs-query
:The first step to make it pass would probably be to to remove the branch where
:consts
is accumulated in the functionresolve-in
.How can the behaviour be reproduced?
See this commit on the branch
jonasseglare/inconsistent-invalid-binding-demo
for a unit test that demonstrates the issue. If you like I can open a PR from that branch so that we have this issue demonstrated in the source code.Add this code to
test-lookup-refs-query
inlookup_refs_test.clj
: