Add Boolean assertion transform functions. #11547
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11547 +/- ##
============================================
+ Coverage 63.05% 63.08% +0.02%
Complexity 1109 1109
============================================
Files 2320 2325 +5
Lines 124667 124723 +56
Branches 19033 19041 +8
============================================
+ Hits 78614 78682 +68
+ Misses 40458 40450 -8
+ Partials 5595 5591 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 8 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| import org.apache.pinot.core.operator.ColumnContext; | ||
|
|
||
|
|
||
| public class IsFalseTransformFunction extends IsTrueTransformFunction { |
There was a problem hiding this comment.
I don't think it is a good idea to extend IsTrueTransformFunction in order to completely change the semantic.
Instead we should have some kind of AbstractIsBooleanTransformFunction that IsTrueTransformFunction, IsFalseTransformFunction, IsNotFalseTransformFunction, etc extend
There was a problem hiding this comment.
+100 to the abstract class. Four implementations should just implement the Boolean operations.
| @Override | ||
| public void init(List<TransformFunction> arguments, Map<String, ColumnContext> columnContextMap) { | ||
| Preconditions.checkArgument(arguments.size() == 1, "Exact 1 argument is required for IS_NULL"); | ||
| _transformFunction = arguments.get(0); |
There was a problem hiding this comment.
This is more a question to understand our code than something to change in this PR. I guess all these functions require to have an argument whose type is boolean. Where do we enforce that type constraint?
There was a problem hiding this comment.
In V1 transform functions we use integer to represent boolean and use BaseTransformFunction to do casting. So we don't need to enforce the type constraint.
| if (returnsTrueWhenValueIsNull()) { | ||
| _intValuesSV[docId] = 1; | ||
| } | ||
| } else if (intValuesSV[docId] != 0) { |
There was a problem hiding this comment.
I feel we can combine returnsTrueWhenValueIsTrue and returnsTrueWhenValueIsFalse into one method like
if (evalBooleanAssertionFunc()) {
_intValuesSV[docId] = 1;
}
There was a problem hiding this comment.
Done.
Good suggestion!
This PR adds the following transform functions: