Closed dejankos closed 3 years ago
@dejankos Nice! I'll check the code a bit later but overall seems reasonable. Thank you!! :1st_place_medal:
cargo fmt
changed the two suggestions, so please apply them if that makes sense. As soon as that gets fixed, the PR is ready to be merged.
There is a failing test: But I've noticed that a couple of days ago, I think it's nothing related to this change. There has to be another PR fixing that.
@gitbuda Ah yes sorry about fmt, changes accepted. For that test since attributes size is asserted instead of attribute names I can only guess it's related to running Memgraph version.
This PR removes unnecessary panic for unreachable code and unifies behavior for lazy and eager data fetching.
Although std::unreachable!() could be used here and serve as self explanatory there is no reason for keeping this behavior in a lib API that returns a Result type.
Test is rewritten since as far as I can see it was just a c/p where no data was fetched in loop but code would panic on first fetchone fn invocations since result was explicitly set to None.