Bool element always false

[Brandon225]

all bool elements are being stored as false.

[Speedy37]

Could you provide a test case ? There is tests of this in the tuple code, I don’t see how this can happen.

[Aioxas]

Noticed here: https://github.com/bluejekyll/foundationdb-rs/blob/0.3/foundationdb/src/tuple/element.rs#L571

There are tests for a straight bool which works as intended, but not for an Element::Bool, i'm willing to put in a pr that addresses that

Note: As the link suggests, this is an issue affecting branch 0.3 only as master was rewritten

[Speedy37]

Nice catch, I didn’t saw it when I checked the code.

By looking at the PR there is another bug in the tuple layer as a tuple at the first level should not be surrounded by NESTED, NIL

[Aioxas]

so after the bug fix, it should go as [39, NESTED, 38, NIL] for the given test?

[Speedy37]

Yeah

[Aioxas]

Figured out encoding, trying to figure out decoding without causing too much hackiness

[Aioxas]

Now I'm confused, the documentation says having the tuples as &[NESTED, 39, NESTED, 38, NIL, NIL] is okay according to https://github.com/apple/foundationdb/blob/master/design/tuple.md

[Brandon225]

My bool test for element inside of foundationdb/src/tuple/element.rs

 fn test_bool() {
        assert_eq!(
            Element::decode(&[TRUE], TupleDepth(0)).unwrap(),
            (Element::Bool(true), 1)
        );

        assert_eq!(
            Element::decode(&[FALSE], TupleDepth(0)).unwrap(),
            (Element::Bool(false), 1)
        );
    }
}

This test fails for TRUE.
Passes with

 assert_eq!(
            Element::decode(&[TRUE], TupleDepth(0)).unwrap(),
            (Element::Bool(false), 1)
        );

[Speedy37]

@Aioxas yeah, but if you look at the official python, go and java bindings, this only apply for nested tuple. At level 0 there is no need for NESTED, ..., NIL. python would decode it as ((...),).
This property at level 0 is quite important to get the rest of the "tuple" layer to work (i.e. subspace, directory, versionstamp, ...)

The things is, I can't fix it in 0.3 without it being a breaking change.

[Aioxas]

So is this PR still good for merging whenever 0.3.1 is ready, right? While the tuple breaking change will be addressed in a different PR?

Which we can start a new issue convo for instead of hijacking this one?

[Speedy37]

Yeah the PR is still good for merging. I don't know, it's easy to fix, but it would break working code and 0.4 (async/await) is not affected.

Let's open a new issue, and keep this one for the Element::Bool error