Fix bug for FILTER with negated expressions
#1587
Conversation
For expression results represented as a set of intervals, negation can lead to an "infinitely" large interval end. This was not adequately handled in the implementation of `FILTER`, which led to a `vector::reserve` exception. This is now fixed. Use the occasion to improve the documentation of the code.
Conformance check passed ✅Test Status Changes 📊
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1587 +/- ##
==========================================
+ Coverage 89.04% 89.05% +0.01%
==========================================
Files 368 368
Lines 33972 34047 +75
Branches 3839 3846 +7
==========================================
+ Hits 30249 30321 +72
Misses 2471 2471
- Partials 1252 1255 +3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Looks good overall, this bug must've been there for ages unnoticed.
I think it's kind of odd that the values are clamped in two loops, but I'm not sure if there's a better way. Maybe a foreach at the start that does it once? Not sure if that helps readability though. Regardless it would be nice having a unit test testing this exact bug to prevent future regressions.
|
|
@RobinTF The reason for having to do this twice in the code is simple: The first part counts the size of the result (so that we can reserve the appropriate amount of space), the second part computes it. A cleaner solution would be to avoid having these "infinite" interval ends in the first place. But Johannes and I figured that would take more work. What you see here is the quick fix. |
For expression results represented as a set of intervals, negation can lead to an "infinitely" large interval end. This was not adequately handled in the implementation of
FILTER, which led to avector::reserveexception. This is now fixed. Use the occasion to improve the documentation of the code.