Fix bug that <=> operator and in operator get wrong result #1516
Conversation
be/src/exprs/binary_predicate.h
Outdated
| Status codegen_compare_fn( | ||
| RuntimeState* state, llvm::Function** fn, llvm::CmpInst::Predicate pred); | ||
|
|
||
| bool get_result_for_null(const AnyVal& v1, const AnyVal& v2, BooleanVal* result) { |
There was a problem hiding this comment.
please add comment to explain the meaning of the return value of this function.
be/src/exprs/binary_predicate.h
Outdated
| RuntimeState* state, llvm::Function** fn, llvm::CmpInst::Predicate pred); | ||
|
|
||
| bool get_result_for_null(const AnyVal& v1, const AnyVal& v2, BooleanVal* result) { | ||
| if (is_safe_for_null) { \ |
There was a problem hiding this comment.
| if (is_safe_for_null) { \ | |
| if (is_safe_for_null) { |
be/src/exprs/binary_predicate.cpp
Outdated
| if (v2.is_null) { \ | ||
| return BooleanVal::null(); \ | ||
| BooleanVal result; \ | ||
| if (get_result_for_null(v1, v2, &result)) { \ |
There was a problem hiding this comment.
I think it's not a good way to add safe is null. I prefer to adding another function to handle this. For performance we should avoid branch and function call here. So you can find the proper function to handle this in Frontend when analyzing.
There was a problem hiding this comment.
it's inappropriate to add an new operator for BinaryPredicate, because in most conditions we can't know whether the children is null when analyze BinaryPredicate.
| return BooleanVal(false); \ | ||
| } \ | ||
| return BooleanVal(v1.val OP v2.val); \ | ||
| } \ |
There was a problem hiding this comment.
remove the \ at the end of MACRO
There was a problem hiding this comment.
Ok, i will remove it.
* Fix bug that <=> operator and in operator get wrong result * Add some comment to get_result_for_null * Add an new Binary Operator to replace is_safe_for_null for handleing '<=>' operator * Add EQ_FOR_NULL to TExprOpcode * Remove macro definition last backslash
3 participants
#1492