feat: add convert case when predicate for simplify exprs #1469

fhkong · 2023-07-14T10:06:35Z

Task Description

When users write SQL, there maybe some redundant case when expressions. We can rewrite these redundant case when expressions according to rewrite rules. Such that, we can use an index on the column of the case when expression other than full table scan.

Solution Description

According to rewrite rules, the case when expressions that can be rewritten in following condition.
case when exp1 then exp2 else exp3 end ~ exp4
When exp3 ~ exp4 is static const expr, we can remove case when expression.
Such as case when c1 > 0 then c2 else 3 end > 4; ==> c1 > 0 and c2 > 4

Passed Regressions

tools/deploy/record/transformer/r/mysql/transformer_convert_case_when_predicate.record

Upgrade Compatibility

Other Information

Release Note

link-zt · 2023-07-17T05:53:53Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ LOG_WARN("unexpected param null", K(parent_expr), K(case_expr), K(sibling_expr), K(ret));
+ } else {
+ case_expr = static_cast<ObCaseOpRawExpr*>(case_when_expr);


这里还是应该检查一下 case_when_expr 的类型。不能假设上面的传参是对的。

或者就是函数直接传一个 ObCaseOpRawExpr 的指针下来

link-zt · 2023-07-17T05:55:03Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ }
+ if (OB_SUCC(ret)) {
+ if (case_expr->get_when_expr_size() != case_expr->get_then_expr_size()) {


OB_UNLIKELY

chimyue · 2023-07-20T12:01:51Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+int ObTransformSimplifyExpr::convert_case_when_predicate(
+ ObRawExpr *&expr,
+ const bool &is_scala_group_by,


不需要传 const bool 的引用吧

chimyue · 2023-07-20T12:02:29Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ ObOpRawExpr *op_expr = NULL;
+ ObRawExpr *child_0 = NULL, *child_1 = NULL;
+ if (OB_FALSE_IT(op_expr = static_cast<ObOpRawExpr *>(expr))) {


chimyue · 2023-07-20T12:08:02Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ } else if (OB_FALSE_IT(case_expr = static_cast<ObCaseOpRawExpr *>(expr))) {
+ ret = OB_ERR_UNEXPECTED;
+ LOG_WARN("failed convert to case when expr");


这个是恒false的，只是为了代码逻辑这样写，不可能抛出错误

chimyue · 2023-07-20T12:15:58Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ } else if (is_uncalculable) {
+ case_expr->get_when_param_exprs().assign(when_exprs);
+ case_expr->get_then_param_exprs().assign(then_exprs);


吞错误码

替换完要formalize

chimyue · 2023-07-20T12:25:55Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ }
+ has_trans = true;
+ }


这一部分逻辑写的太分散了

chimyue · 2023-07-20T12:29:39Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+// IF case_at_left is false, parent_expr := sibling_expr op case_when_expr
+int ObTransformSimplifyExpr::do_convert_case_when_predicate(
+ ObRawExpr *&parent_expr,


ObTransformSimplifyExpr::inner_convert_case_when_predicate 和 ObTransformSimplifyExpr::do_convert_case_when_predicate 不看注释不知道在干什么，还以为是调用关系

chimyue · 2023-07-20T12:30:22Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ /* do nothing */
+ } else if (OB_FALSE_IT(case_expr = static_cast<ObCaseOpRawExpr *>(case_when_expr))) {
+ ret = OB_ERR_UNEXPECTED;


错误码走不到

chimyue · 2023-07-20T12:41:56Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ // LOG_WARN("failed to add cast for replace", K(ret));
+ } else if (OB_FAIL(ObRawExprUtils::create_double_op_expr(*(ctx_->expr_factory_), 
+ ctx_->session_info_,


在可改写条件判断时就创建大量表达式？

chimyue · 2023-07-27T01:56:37Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ if (OB_ISNULL(stmt)) {
+ ret = OB_ERR_UNEXPECTED;
+ LOG_WARN("stmt is NULL", K(*stmt));


print *stmt after check stmt is null? just print ret

chimyue · 2023-07-27T01:59:20Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ and ({expr1 to exprn} \ exprk) op const_value is all false, then 
+ expr can be rewritten to lnnvl(cond1) and .. and lnnvl(condk-1) and condk and (exprk op const_value).
+ (If k == n, condk is ignored.)


too long comments, just show a simple transformation example

chimyue · 2023-07-27T02:02:28Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ ObRawExpr *child_0 = NULL;
+ ObRawExpr *child_1 = NULL;
+ if (OB_FALSE_IT(op_expr = static_cast<ObOpRawExpr *>(expr))) {


if (OB_FALSE_IT(op_expr = static_cast<ObOpRawExpr *>(expr))) { is unnecessary. just set op_expr when define it.

chimyue · 2023-07-27T02:07:12Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ } else { /* do nothing */}
+
+ for (int64_t i = 0; OB_SUCC(ret) && i < expr->get_param_count(); ++i) {


if trans_happened is true, is it necessary to try transform recursively？

chimyue · 2023-07-27T02:29:53Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ ret = OB_ERR_UNEXPECTED;
+ LOG_WARN("default expr is NULL", K(ret));
+ } else if (OB_UNLIKELY(false_null_cnt != when_cnt)) {


convert_case_when_predicate_by_when_exprs is too complex.
try convert the first when directly, and do this convert for the same expr repeatedly if transform happened.

chimyue · 2023-07-27T02:36:33Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ ObRawExpr *default_expr = NULL;
+ if (OB_ISNULL(default_expr = case_expr->get_default_param_expr())) {
+ ret = OB_ERR_UNEXPECTED;


just enum_exprs.assign(get_then_param_exprs()) and enum_exprs.push_back() ?
enum_exprs is uncessary, just use get_then_param_exprs() and get_default_param_expr() in the loop.

chimyue · 2023-07-28T08:52:54Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ } else {
+ // rewrite case_when_expr with lnnvl(...)
+ if (true_null_uncalc_idx > case_expr->get_then_expr_size() ||


OB_UNLIKELY

chimyue · 2023-07-28T08:53:58Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ }
+ // push back condk, if k != n
+ if (OB_SUCC(ret) && true_null_uncalc_idx < case_expr->get_then_expr_size()) {


use if else for all the operators below

chimyue · 2023-07-28T08:59:04Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ LOG_WARN("failed to add false constraint", K(ret));
+ } else if (OB_FAIL(add_true_and_null_constraint(
+ is_uncalculable,


merge add_false_or_false_constraint and add_true_and_null_constraint as one function

chimyue · 2023-07-28T09:03:47Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+
+ for (int64_t i = 0; OB_SUCC(ret) && -1 == first_non_false_idx && i < when_cnt; ++i) {
+ ObRawExpr *when_expr = NULL;


put this for loop into check_when_exprs_validity？

chimyue · 2023-07-28T09:06:36Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ } else if (0 == first_non_false_idx && is_uncalc) {
+ } else if (OB_FAIL(add_false_or_null_constraint(false_null_exprs, 
+ has_null))) {


adjust function name for assign_case_when_expr.

chimyue · 2023-07-31T08:16:09Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+
+int ObTransformSimplifyExpr::add_true_and_true_constraint(
+ const ObIArray<ObRawExpr *> &true_exprs) {


remove this unused function: ObTransformSimplifyExpr::add_true_and_true_constraint

chimyue · 2023-07-31T08:18:00Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+}
+
+int ObTransformSimplifyExpr::add_true_constraint(ObRawExpr *true_expr) {


ObTransformSimplifyExpr::add_true_constraint，need create this function？

chimyue · 2023-07-31T08:46:32Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ if (OB_FAIL(ret)) {
+ } else if (OB_UNLIKELY(idx < when_cnt && !is_true && !is_uncalc)) {
+ } else if (OB_FAIL(add_false_or_false_constraint(false_exprs))) {


add and use add_constraint_for_convert_case_when to add constraint for convert_case_when_predicate_by_when_exprs and convert_case_when_predicate_by_then_exprs ？

link-zt · 2023-07-24T03:41:17Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ ObRawExpr *child_0 = NULL;
+ ObRawExpr *child_1 = NULL;
+ if (OB_FALSE_IT(op_expr = static_cast<ObOpRawExpr *>(expr))) {


这里套 if OB_FAISE_IT 做什么？直接做个变量赋值不就好了吗

link-zt · 2023-07-24T03:43:39Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ }
+ } else if (T_OP_CASE == expr->get_expr_type()) {
+ if (is_scala_group_by && expr->has_flag(CNT_AGG)) {


这个判断在反复出现？为什么不干脆放到最前面？或者直接放到 2595 行，这样 is_scalar_group_by 也不用传参了

link-zt · 2023-08-03T07:37:19Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ int64_t uncalc_idx = -1;
+ // do pre check 
+ for (int64_t i = 0; OB_SUCC(ret) && i < enum_value_cnt; ++i) {


enum_value_cnt 这种变量也没必要定义。

直接诶用 enum_exprs.count() 就好了

link-zt · 2023-08-03T08:40:59Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ * @param[in][out] trans_happened: whether rewrite is happened
+ */
+int ObTransformSimplifyExpr::convert_case_when_predicate(


这个函数里的递归顺序有点奇怪。
逻辑上这里有两个变化。
一个是，直接简化 case when 表达式。
另外一个是简化 case_expr = const 。

逻辑上，递归方式不应该是先递归每个 param expr。

然后检查当前 expr 是否为 case 表达式；

如果是 case 的话，那么尝试用策略1 简化
如果是 case = const 这种形态的，就尝试用策略 2 简化。

link-zt · 2023-08-03T08:55:38Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ ObSEArray<ObRawExpr*, 2> false_exprs; // used to add constraints. nvl(expr, false)
+
+ for (idx = 0; OB_SUCC(ret) && idx < when_cnt; ++idx) {


这里的逻辑是不是搞得太复杂了。

直接 for loop case when 的每一个分支。

如果 when 是 false，就构造约束并继续 loop；
如果 when 是 true，那么保留 then expr；并构造约束；跳出循环；
如果 when 是 unkown，那么保留后续的 when/then 分支；跳出循环。

循环外，构造最终的改写表达式

link-zt · 2023-08-03T09:02:00Z

src/sql/resolver/expr/ob_raw_expr_util.cpp

@@ -5296,6 +5296,37 @@ int ObRawExprUtils::build_nvl_expr(ObRawExprFactory &expr_factory, const ColumnI
 return ret;
 }

+int ObRawExprUtils::build_nvl_expr(ObRawExprFactory &expr_factory, 


为啥 transform_simpilfy_expr 里加了一个 build_nvl

这里又加了一个

link-zt · 2023-08-03T11:28:03Z

src/sql/rewrite/ob_transform_simplify_expr.cpp

+ const bool case_at_left,
+ bool &trans_happened) 
+{


假如，最终 case_expr op const 这个谓词的结果是 true。我理解现在的变化是没问题的。

但假如，判定结果为 false。有没有可能 transform 之后得到的结果为 NULL ？目前从逻辑上，看是没有去区分 NULL 和 false 的。

同理，如果判定结果为 NULL，会不会混淆成 false

CLAassistant · 2023-08-15T07:15:25Z

All committers have signed the CLA.

… add more testcases fix: modify then expr rewrite at the root of where/having clause

feat: add convert case when predicate for simplify exprs

365076a

link-zt reviewed Jul 17, 2023

View reviewed changes

fhkong added 3 commits July 20, 2023 17:57

feat: add convert case when predicate for simplify exprs

7f601b1

Merge branch 'master' into feat

f89d212

docs: update some comments and code style

e4023e4

chimyue reviewed Jul 21, 2023

View reviewed changes

chimyue reviewed Jul 27, 2023

View reviewed changes

chimyue reviewed Jul 28, 2023

View reviewed changes

chimyue reviewed Jul 31, 2023

View reviewed changes

link-zt reviewed Aug 3, 2023

View reviewed changes

fhkong force-pushed the feat branch 3 times, most recently from 9f01e13 to 612316d Compare August 15, 2023 10:54

fhkong added 2 commits August 15, 2023 18:57

feat: add convert case when predicate for simplify exprs

da40712

fix: false/null error in convert_case_when_predicate_by_then_exprs &&…

35cb40d

… add more testcases fix: modify then expr rewrite at the root of where/having clause

fhkong force-pushed the feat branch from 612316d to 35cb40d Compare August 15, 2023 11:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add convert case when predicate for simplify exprs #1469

feat: add convert case when predicate for simplify exprs #1469

fhkong commented Jul 14, 2023

link-zt Jul 17, 2023

link-zt Jul 17, 2023

chimyue Jul 20, 2023

chimyue Jul 20, 2023

chimyue Jul 20, 2023

chimyue Jul 20, 2023

chimyue Jul 20, 2023

chimyue Jul 20, 2023

chimyue Jul 20, 2023

chimyue Jul 20, 2023

chimyue Jul 20, 2023

chimyue Jul 27, 2023

chimyue Jul 27, 2023

chimyue Jul 27, 2023

chimyue Jul 27, 2023

chimyue Jul 27, 2023

chimyue Jul 27, 2023

chimyue Jul 28, 2023

chimyue Jul 28, 2023

chimyue Jul 28, 2023

chimyue Jul 28, 2023

chimyue Jul 28, 2023

chimyue Jul 31, 2023

chimyue Jul 31, 2023

chimyue Jul 31, 2023

link-zt Jul 24, 2023

link-zt Jul 24, 2023

link-zt Aug 3, 2023

link-zt Aug 3, 2023

link-zt Aug 3, 2023

link-zt Aug 3, 2023

link-zt Aug 3, 2023

CLAassistant commented Aug 15, 2023 •

edited

		} else { /* do nothing */}

		for (int64_t i = 0; OB_SUCC(ret) && i < expr->get_param_count(); ++i) {


		for (int64_t i = 0; OB_SUCC(ret) && -1 == first_non_false_idx && i < when_cnt; ++i) {
		ObRawExpr *when_expr = NULL;


		int ObTransformSimplifyExpr::add_true_and_true_constraint(
		const ObIArray<ObRawExpr *> &true_exprs) {

		}

		int ObTransformSimplifyExpr::add_true_constraint(ObRawExpr *true_expr) {

		ObSEArray<ObRawExpr*, 2> false_exprs; // used to add constraints. nvl(expr, false)

		for (idx = 0; OB_SUCC(ret) && idx < when_cnt; ++idx) {

feat: add convert case when predicate for simplify exprs #1469

Are you sure you want to change the base?

feat: add convert case when predicate for simplify exprs #1469

Conversation

fhkong commented Jul 14, 2023

Task Description

Solution Description

Passed Regressions

Upgrade Compatibility

Other Information

Release Note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CLAassistant commented Aug 15, 2023 • edited

CLAassistant commented Aug 15, 2023 •

edited