grouped_by panics if key is not in Parent array

nishtahir commented 4 years ago

Setup

Versions

Rust: nightly-2019-12-22
Diesel: version = "1.4.3", features = ["postgres", "r2d2"]
Database: Postgres
Operating System: macOS

Feature Flags

diesel:

Problem Description

thread '<unnamed>' panicked at 'no entry found for key', src/libcore/option.rs:1188:5
2020-02-12T12:59:33.327706314Z app[web.1]: stack backtrace:
2020-02-12T12:59:33.329244468Z app[web.1]:    0: backtrace::backtrace::libunwind::trace
2020-02-12T12:59:33.329267263Z app[web.1]:              at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/libunwind.rs:88
2020-02-12T12:59:33.329297828Z app[web.1]:    1: backtrace::backtrace::trace_unsynchronized
2020-02-12T12:59:33.329303867Z app[web.1]:              at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/mod.rs:66
2020-02-12T12:59:33.329309380Z app[web.1]:    2: std::sys_common::backtrace::_print_fmt
2020-02-12T12:59:33.329314516Z app[web.1]:              at src/libstd/sys_common/backtrace.rs:84
2020-02-12T12:59:33.329319686Z app[web.1]:    3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
2020-02-12T12:59:33.329325237Z app[web.1]:              at src/libstd/sys_common/backtrace.rs:61
2020-02-12T12:59:33.329330381Z app[web.1]:    4: core::fmt::write
2020-02-12T12:59:33.329335406Z app[web.1]:              at src/libcore/fmt/mod.rs:1057
2020-02-12T12:59:33.329340266Z app[web.1]:    5: std::io::Write::write_fmt
2020-02-12T12:59:33.329345062Z app[web.1]:              at src/libstd/io/mod.rs:1426
2020-02-12T12:59:33.329349777Z app[web.1]:    6: std::sys_common::backtrace::_print
2020-02-12T12:59:33.329354460Z app[web.1]:              at src/libstd/sys_common/backtrace.rs:65
2020-02-12T12:59:33.329359447Z app[web.1]:    7: std::sys_common::backtrace::print
2020-02-12T12:59:33.329381300Z app[web.1]:              at src/libstd/sys_common/backtrace.rs:50
2020-02-12T12:59:33.329386245Z app[web.1]:    8: std::panicking::default_hook::{{closure}}
2020-02-12T12:59:33.329391124Z app[web.1]:              at src/libstd/panicking.rs:193
2020-02-12T12:59:33.329395879Z app[web.1]:    9: std::panicking::default_hook
2020-02-12T12:59:33.329400595Z app[web.1]:              at src/libstd/panicking.rs:210
2020-02-12T12:59:33.329405309Z app[web.1]:   10: std::panicking::rust_panic_with_hook
2020-02-12T12:59:33.329410008Z app[web.1]:              at src/libstd/panicking.rs:471
2020-02-12T12:59:33.329414696Z app[web.1]:   11: rust_begin_unwind
2020-02-12T12:59:33.329419340Z app[web.1]:              at src/libstd/panicking.rs:375
2020-02-12T12:59:33.329424127Z app[web.1]:   12: core::panicking::panic_fmt
2020-02-12T12:59:33.329428850Z app[web.1]:              at src/libcore/panicking.rs:84
2020-02-12T12:59:33.329433580Z app[web.1]:   13: core::option::expect_failed
2020-02-12T12:59:33.329473776Z app[web.1]:              at src/libcore/option.rs:1188
2020-02-12T12:59:33.329479450Z app[web.1]:   14: <Iter as diesel::associations::belongs_to::GroupedBy<Parent>>::grouped_by

I believe the problem is with the map access here https://docs.diesel.rs/src/diesel/associations/belongs_to.rs.html#128

What are you trying to accomplish?

I'm trying to group the results by a given list of model IDs. I think that IDs that are not represented should be ignored but at the very least should not cause a panic

What is the expected output?

It should not panic

What is the actual output?

It currently panics with the backtrace provided above

Checklist

[x] I have already looked over the issue tracker for similar issues.
[x] This issue can be reproduced on Rust's stable channel. (Your issue will be closed if this is not the case)

weiznich commented 4 years ago

Can you please provide a minimal example how to trigger this?

nishtahir commented 4 years ago

@weiznich i've put together a sample demonstrating the issue here https://github.com/nishtahir/diesel-issues-2298

weiznich commented 4 years ago

@nishtahir Thanks for putting together the reproducible example. I think .grouped_by is designed to be called with the same list of elements passed to belongs_to, so that this is just a use case we did'nt design the API for. That said it is probably useful to also support this, but I need to think about what's the right behaviour here.

diesel-rs / diesel