Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent filtering of numeric values #1065

Open
enitram opened this issue Oct 23, 2024 · 3 comments
Open

Inconsistent filtering of numeric values #1065

enitram opened this issue Oct 23, 2024 · 3 comments

Comments

@enitram
Copy link

enitram commented Oct 23, 2024

When comparing numbers of different types, nitrite returns different results depending on the filter (eq, lte, gte) and if the value was indexed.

Here is a small example comparing int and long:

        Nitrite db = Nitrite.builder().openOrCreate();
        NitriteCollection collection = db.getCollection("myCollection");

        Document doc = Document.createDocument("value", 42);
        collection.insert(doc);

//        assert collection.find(FluentFilter.where("value").eq(42L)).size() == 1;  // FAIL
        assert collection.find(FluentFilter.where("value").lte(42L)).size() == 1; // SUCCESS
        assert collection.find(FluentFilter.where("value").gte(42L)).size() == 1; // SUCCESS

        collection.createIndex(IndexOptions.indexOptions(IndexType.NON_UNIQUE), "value");

//        assert collection.find(FluentFilter.where("value").eq(42L)).size() == 1;  // FAIL
//        assert collection.find(FluentFilter.where("value").lte(42L)).size() == 1;  // FAIL
//        assert collection.find(FluentFilter.where("value").gte(42L)).size() == 1; // FAIL

        db.close();

This was tested with nitrite 4.3.0 and also shows the same results when comparing int and double.

@anidotnet
Copy link
Contributor

I'll take a look at it.

@leoger
Copy link

leoger commented Nov 16, 2024

@anidotnet I compared the implementations of apply in EqualsFilter.java and LesserEqualFilter.java and once I discovered this section, it seemed clear that this is part of the problem.

if (o1 instanceof Number && o2 instanceof Number) {
if (o1.getClass() != o2.getClass()) {
return false;
}

After all, the check for instanceof Number is specifically a recognition that different subclasses are still directly comparable. I wrote a little unit test locally based on @enitram's repro snippet above and I confirmed that commenting out lines 130-132 causes the first assert to no longer fail. 🎉 (Of course, it does make a few other tests elsewhere in the project fail, and now I'm out of my depth on the internal logic of Nitrite's filter abstractions.)

I hope this is at least somewhat helpful! 🙇

@anidotnet
Copy link
Contributor

Thanks a lot @leoger for your analysis. I am still quite busy with other urgent engagement. Once I get some times, I'll take a look at this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants