fix: Compound filter operators with relations #1855
Conversation
islamaliev
added
bug
There was a problem hiding this comment.
I've gone over everything outside the filter package, which I'll try and do later/tomorrow depending on how much time/energy I have left.
I like how you have done it from what I have seen so far, my requests are all fairly localised I think. If I haven't finished my review by Friday afternoon and someone else has approved (and you have responded to all my comments so far) please feel very free to dismiss my request for changes and merge without my approval.
planner/filter/copy_field.go
Outdated
|
|
||
| // CopyField copies the given field from the provided filter. | ||
| // The result filter preserves the structure of the original filter. | ||
| func CopyField(filter *mapper.Filter, field mapper.Field) *mapper.Filter { |
There was a problem hiding this comment.
suggestion: This looks like it could be made private?
There was a problem hiding this comment.
This is actually a question: what makes a function a good candidate for being public? Is it just usage from another package? Some function are just nice utilities that can be used later.
Codecov ReportPatch coverage:
@@ Coverage Diff @@
## develop #1855 +/- ##
===========================================
+ Coverage 70.18% 70.45% +0.28%
===========================================
Files 225 232 +7
Lines 23987 24184 +197
===========================================
+ Hits 16833 17038 +205
+ Misses 5996 5991 -5
+ Partials 1158 1155 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
islamaliev
force-pushed
the
fix/islam/compound-filter-with-relation
branch
from
222b8d4 to
a35edfc
Compare
## Relevant issue(s) Resolves sourcenetwork#1808 ## Description Any time a filter with `_and` or `_or` operator that includes relations is applied it will fail. There were 2 problems: - filter is not recursively analysed during mapping phase which would result in some relational fields not being mapped and later panic upon attempt to access them. - the filter is not properly split between the scan and select nodes. It would just analyse and split top-level fields. These problems are fixed.
Relevant issue(s)
Resolves #1808
Description
Any time a filter with
_andor_oroperator that includes relations is applied it will fail.There were 2 problems:
These problems are fixed.
Tasks
How has this been tested?
Integration tests
Specify the platform(s) on which this was tested: