fix(prqlc): improve requirement logic#5357
Conversation
|
thanks @Fanaen !
could we add the case as a simple inline test in |
|
Thanks @max-sixty! I added the inline test as suggested. The PR is ready for review! |
…columns" This reverts commit c1d7069.
|
I've found that c1d7069, which I added in an attempt to get nicer SQL results, sometimes removes necessary columns for instance when referenced in a filter. As it's not necessary for this fix, I've removed it. |
|
great! thank you @Fanaen (for transparency: I tried this out but haven't thought through each line of code, so if we need to iterate more we can do so, thank you for your recent contributions!) |
|
Perhaps this ought to be mentioned in the CHANGELOG? |
|
fine to add things to CHANGELOG but no strong requirement! |
Fixes #5354
New result:
About the refactoring
get_requirementis quite logic-heavy.It made sense to separate column selection logic from complexities/
SELECTstatus logic when there the case were simpler. The special cases with multiple complexities now outnumber general cases.So I added
Requirements: a newtype forVec<Requirement>. It addsComplexity::Plainandselect: falseby default (the common case) and a few convenience methods to improve readbility.It's a separate commit in case we want to split the PR.