add expr and fix bugs #198
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2149
force-pushed
the
xxl_temp
branch
2 times, most recently
from
bf9747d
to
bf11551
Compare
src/sql/engine/expr/ob_expr_inet.cpp
Outdated
| LOG_WARN("ip format invalid", K(ret)); | ||
| } else if (dst + 2 > ipv6addr_end) { | ||
| ret = OB_INVALID_ARGUMENT; | ||
| LOG_WARN("ip format invalid", K(ret)); |
There was a problem hiding this comment.
可以多打印些有用的信息出来，比如 dst 和 ipv6addr_end，下同。
src/sql/engine/expr/ob_expr_inet.cpp
Outdated
| int ret = OB_SUCCESS; | ||
| if (OB_FAIL(expr.eval_param_value(ctx))) { | ||
| LOG_WARN("is_ipv6 expr eval param value failed", K(ret)); | ||
| } else if (ObVarcharType != expr.args_[0]->datum_meta_.type_) { |
2149
force-pushed
the
xxl_temp
branch
3 times, most recently
from
21a310d
to
92ad7f4
Compare
src/sql/engine/expr/ob_expr_inet.cpp
Outdated
| LOG_WARN("ip format invalid, too short or too long", K(ret), K(len)); | ||
| } else if (OB_ISNULL(str) || OB_ISNULL(ipv4addr)) { | ||
| ret = OB_INVALID_ARGUMENT; | ||
| LOG_WARN("ip str or ipv4addr dst is null", K(ret)); |
There was a problem hiding this comment.
K(str) 和 K(ipv4addr)，不然不知道是哪个出错了，下同。
src/sql/engine/expr/ob_expr_inet.cpp
Outdated
| CK(expr.res_buf_len_ >= MAX_IP_ADDR_LENGTH); | ||
| if (OB_FAIL(ret)) { | ||
| LOG_WARN("result buf size greater than MAX_IP_ADDR_LENGTH", K(ret)); | ||
| } |
There was a problem hiding this comment.
这里逻辑还是不对，吞错误码的问题没解决。
CK(expr.res_buf_len_ >= MAX_IP_ADDR_LENGTH);
if (OB_FAIL(ret)) {
} else {
// xxx
}
src/sql/engine/expr/ob_expr_inet.cpp
Outdated
| LOG_WARN("ip format invalid, end with x:", K(ret), K(i)); | ||
| } else if (dst + 2 > ipv6addr_end) { | ||
| ret = OB_INVALID_ARGUMENT; | ||
| LOG_WARN("ip format invalid", K(ret), K(i), K(dst - (char *) ipv6addr)); |
There was a problem hiding this comment.
感觉这里 K(dst - (char *) ipv6addr) 有点儿奇怪
src/sql/engine/expr/ob_expr_inet.cpp
Outdated
| return ret; | ||
| } | ||
|
|
||
| int ObExprInetCommon::str_to_ipv6(int len, const char *str, in6_addr *ipv6addr) |
There was a problem hiding this comment.
如果接口里从头到尾都只有一种特定的错误码 OB_INVALID_ARGUMENT，其实可以返回 void 类型，然后加一个 bool & is_ip_format_valid 参数记录是否格式非法。尽量不要让上层依赖特定错误码。
src/sql/engine/expr/ob_expr_inet.cpp
Outdated
| LOG_WARN("fail to convert ip to int", K(ret), K(m_text)); | ||
| } | ||
| } | ||
| if (OB_INVALID_ARGUMENT == ret) { |
There was a problem hiding this comment.
这里别依赖错误码了，还有这段代码是不是应该放到 if (OB_FAIL(ob_inet_aton(expr_datum, m_text))) 的花括号里面？下同。
src/sql/engine/expr/ob_expr_inet.cpp
Outdated
| const char *result = ip_str.ptr(); | ||
| const char *ip = ip_binary.ptr(); | ||
| if (OB_ISNULL(ip) || OB_ISNULL(result)) { | ||
| is_ip_format_invalid = true; |
There was a problem hiding this comment.
走到这里是因为 ip_format_invalid ？
src/sql/engine/expr/ob_expr_inet.cpp
Outdated
| if (!ob_is_varbinary_type(expr.args_[0]->datum_meta_.type_,expr.args_[0]->datum_meta_.cs_type_)) { | ||
| is_ip_format_invalid = true; | ||
| LOG_WARN("ip format invalid", K(ret), K(text)); | ||
| } else { |
There was a problem hiding this comment.
这段逻辑这样写吧
if (!ob_is_varbinary_type(expr.args_[0]->datum_meta_.type_,expr.args_[0]->datum_meta_.cs_type_)) {
is_ip_format_invalid = true;
LOG_WARN("ip format invalid", K(ret), K(text));
} else if (FALSE_IT(ObExprInetCommon::ip_to_str(text.get_string(), is_ip_format_invalid, ip_str))) {
} else if (is_ip_format_invalid) {
xxx
} else {
expr_datum.set_string(ip_str);
}
src/sql/engine/expr/ob_expr_inet.cpp
Outdated
| } | ||
| } | ||
| } else { | ||
| expr_datum.set_string(str_result); |
There was a problem hiding this comment.
这里有点儿奇怪，str_result 和 expr_datum.get_string() 里的 ptr 不是本来就一直是同一个指针吗？为啥还要来回 set？
src/sql/engine/expr/ob_expr_inet.cpp
Outdated
| if (text.is_null()) { | ||
| expr_datum.set_null(); | ||
| } else { | ||
| ObString str_result = expr_datum.get_string(); |
There was a problem hiding this comment.
expr_datum 表示计算结果，一般不需要调 get 接口，最后算出结果后 set_string 就好了。
src/sql/engine/expr/ob_expr_inet.cpp
Outdated
| LOG_WARN("ip format invalid", K(ip)); | ||
| } else { | ||
| char *result_buf = str_result.ptr(); | ||
| if (OB_ISNULL(result_buf)) { |
There was a problem hiding this comment.
感觉如果 result_buf 是 nullptr 的话，说明给计算结果未被 set 前的 ObString 里的 ptr 地址是非法的，和 ip_format 是否是 invalid 没关系吧？
| is_ip_format_invalid = true; | ||
| LOG_WARN("ip format invalid, end with '.'", K(i)); | ||
| } else { | ||
| byte_addr[dotcnt] = (unsigned char) byte; |
There was a problem hiding this comment.
这里内存的使用太诡异了，感觉可能会把内存给写乱的。可以参考下 ObExprConcat::eval_concat 是咋处理 string 类型结果的内存的。
2149
force-pushed
the
xxl_temp
branch
3 times, most recently
from
580acd2
to
74a18d2
Compare
liboyang0730
reviewed
Aug 2, 2021
| ObSQLUtils::get_default_cast_mode(session->get_stmt_type(), session, cast_mode); | ||
| if (OB_FAIL(calc(result, text, *expr_ctx.calc_buf_, cast_mode))) { | ||
| LOG_WARN("fail to calc", K(ret)); | ||
| } else if (OB_LIKELY(!result.is_null())) { |
LINxiansheng
approved these changes
Aug 2, 2021
2149
force-pushed
the
xxl_temp
branch
3 times, most recently
from
419de81
to
95402bc
Compare
2149
force-pushed
the
xxl_temp
branch
3 times, most recently
from
0eb53d6
to
3cffe23
Compare
LINxiansheng
force-pushed
the
master
branch
2 times, most recently
from
d3d5635
to
27c813d
Compare
LINxiansheng
approved these changes
Aug 23, 2021
inet_aton inet_ntoa inet6_aton inet6_ntoa is_ipv4 is_ipv6 is_ipv4_mapped is_ipv6_mapped fix: [PASS_BY_VALUE] /data/1/xiaoma/open_source/oceanbase/src/sql/engine/expr/ob_expr_operator.cpp [FLOATING_POINT_EQUALITY] /data/1/xiaoma/open_source/oceanbase/src/sql/engine/expr/ob_expr_mod.cpp [CHECKED_RETURN] /data/1/xiaoma/open_source/oceanbase/src/sql/engine/expr/ob_expr_operator.cpp [UNINIT_CTOR] /data/1/xiaoma/open_source/oceanbase/src/sql/engine/expr/ob_sql_expression.h [DEADCODE] /data/1/xiaoma/open_source/oceanbase/src/sql/engine/expr/ob_expr_operator.cpp
LINxiansheng
pushed a commit
that referenced
this pull request
Aug 31, 2021
|
merged. |
This was referenced Jul 14, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
add expr:
inet_aton、inet_ntoa、inet6_aton、inet6_ntoa、is_ipv4、 is_ipv6、is_ipv4_mapped、is_ipv6_mapped
fix bugs:
[PASS_BY_VALUE]
src/sql/engine/expr/ob_expr_operator.cpp
[FLOATING_POINT_EQUALITY]
src/sql/engine/expr/ob_expr_mod.cpp
[CHECKED_RETURN]
src/sql/engine/expr/ob_expr_operator.cpp
[UNINIT_CTOR]
src/sql/engine/expr/ob_sql_expression.h
[DEADCODE]
src/sql/engine/expr/ob_expr_operator.cpp
Why are the changes needed?
Will break the compatibility? How if so?
Does this PR introduce any user-facing change?
How was this patch tested?
add test file: test/mysql_test/test_suite/expr/t/expr_inet.test
add result file:test/mysql_test/test_suite/expr/r/mysql/expr_inet.result
Checklist