couchbase / couchbase-lite-android-ce

The community edition of couchbase lite for android
Apache License 2.0
9 stars 1 forks source link

Result.toMap() contains Long value in place of Boolean when explicit query projection used #34

Closed JamesGay473 closed 4 years ago

JamesGay473 commented 4 years ago

Hey!

I've encountered a small issue with CBL 2.6.2 CE whereby if an explicit projection is specified for a Query and toMap() is called on the Result then values in the map which are expected to be Boolean are actually returned as Long (where 'true' = 1L and 'false' = 0L).

I thought originally that this might be linked to CBL-49 but the issue doesn't seem to occur when SelectResult.all() is used as demonstrated by this existing unit test: https://github.com/couchbase/couchbase-lite-java/blob/c3f003dea33037f6172dbf9a337b65d5519768e2/lib/src/shared/test/java/com/couchbase/lite/QueryTest.java#L1627

I've modified that test to highlight the issue but if you would like any more information then let me know.

@Test
public void testResultToMapWithBoolean() throws Exception {
    MutableDocument exam1 = new MutableDocument("exam1");
    exam1.setString("exam type", "final");
    exam1.setString("question", "There are 45 states in the US.");
    exam1.setBoolean("answer", false);

    db.save(exam1);

    Query query = QueryBuilder
            .select(
                    SelectResult.property("exam type"),
                    SelectResult.property("question"),
                    SelectResult.property("answer")
            )
            .from(DataSource.database(db))
            .where(Meta.id.equalTo(Expression.string("exam1")));

    verifyQuery(query, (n, result) -> {
        Map<String, Object> map = result.toMap();
        // !!! ERROR !!!
        // Throws 'java.lang.ClassCastException: java.lang.Long cannot be cast to java.lang.Boolean'
        // !!! ERROR !!!
        assertFalse((Boolean) map.get("answer"));
    });
}
bmeike commented 4 years ago

Nice work. Thanks.

bmeike commented 4 years ago

This issue is being tracked here: https://issues.couchbase.com/browse/CBL-609

bmeike commented 4 years ago

For reasons that I do not entirely understand, this is expected behavior. The information that the value at a particular key is a boolean is lost during the conversion to a Map. The underlying type used to represent a boolean is Long: in the map, the value is type Long. This behavior is buried deep in our data representation and is not likely to change. Closing, with some embarrassment, as "works as designed"

snej commented 4 years ago

I wouldn't say expected behavior, but SQLite itself has no boolean type, only integers, and we have some tweaks in our query code to preserve the distinction. It looks like they're not working in this case, for some reason.

bmeike commented 4 years ago

Reopening on advice from @snej

snej commented 4 years ago

This is a bug in LiteCore. Fixed in https://github.com/couchbase/couchbase-lite-core/commit/da4791a9f0d0de8e79b2161f70c7011ba146c6e6 ... currently on the dev branch, which is being merged to master "soon".

JamesGay473 commented 4 years ago

Hi both - thanks for looking into this and for the information about the issue. Sounds like it was a pain to fix so really appreciate your efforts!

bmeike commented 4 years ago

This has been fixed on master. Unfortunately, there are still other problems on that branch, so I don't recommend trying it just yet. Will ping when it gets stable again

snej commented 4 years ago

Even after the fix is out, it's better to follow Postel's Law (be lenient in what you accept) and not assume a specific boxed type. There are other circumstances where LiteCore cannot tell that a value is a boolean and will treat it as an int: basically any time the value comes from a SQLite operator/function. So a == b has integer type, as does s LIKE 'foo'.

bmeike commented 4 years ago

Does this mean that, in Java, the correct way to handle things that might be Booleans is to get them as Objects and test their type?

snej commented 4 years ago

Honestly it's been so long since I used Java, I don't remember what the boxed classes look like. But it sounds like yes, you would have to test whether it's a Boolean or not.

bmeike commented 4 years ago

@JamesGay473 HEAD on master is in pretty good shape, now, if you are interested in trying the fix.

I've filed an additional bug, https://issues.couchbase.com/browse/CBL-658, to track fixing the cases that @snej mentions above