Skip to content

Commit

Permalink
[Improvement](expr) fold child when const expr not folded (#38493)
Browse files Browse the repository at this point in the history
## Proposed changes
1. fold child when const expr not folded
2. do not fold function `sleep`
3. move all exceptional expression into shouldSkipFold

before
```sql
mysql [test]>explain select sleep(sign(1)*100);
+-----------------------------------------------+
| Explain String(Nereids Planner)               |
+-----------------------------------------------+
| PLAN FRAGMENT 0                               |
|   OUTPUT EXPRS:                               |
|     sleep(cast((sign(1.0) * 100) as INT))[#0] |
|   PARTITION: UNPARTITIONED                    |
|                                               |
|   HAS_COLO_PLAN_NODE: false                   |
|                                               |
|   VRESULT SINK                                |
|      MYSQL_PROTOCAL                           |
|                                               |
|   0:VUNION(32)                                |
|      constant exprs:                          |
|          sleep(CAST((sign(1) * 100) AS int))  |
+-----------------------------------------------+
13 rows in set (15.02 sec)

mysql [test]>select sleep(sign(1)*100);
+-----------------------------------------------------+
| sleep(cast((sign(cast(1 as DOUBLE)) * 100) as INT)) |
+-----------------------------------------------------+
|                                                   1 |
+-----------------------------------------------------+
1 row in set (1 min 55.34 sec)
```

after
```sql
mysql [test]>explain select sleep(sign(1)*100);
+---------------------------------+
| Explain String(Nereids Planner) |
+---------------------------------+
| PLAN FRAGMENT 0                 |
|   OUTPUT EXPRS:                 |
|     sleep(100)[#0]              |
|   PARTITION: UNPARTITIONED      |
|                                 |
|   HAS_COLO_PLAN_NODE: false     |
|                                 |
|   VRESULT SINK                  |
|      MYSQL_PROTOCAL             |
|                                 |
|   0:VUNION(32)                  |
|      constant exprs:            |
|          sleep(100)             |
+---------------------------------+
13 rows in set (0.23 sec)

mysql [test]> select sleep(sign(1)*100);
+-----------------------------------------------------+
| sleep(cast((sign(cast(1 as DOUBLE)) * 100) as INT)) |
+-----------------------------------------------------+
|                                                   1 |
+-----------------------------------------------------+
1 row in set (1 min 40.37 sec)

```
  • Loading branch information
BiteTheDDDDt authored and dataroaring committed Aug 16, 2024
1 parent 5f71e60 commit 723dfea
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import org.apache.doris.nereids.trees.expressions.Match;
import org.apache.doris.nereids.trees.expressions.functions.BoundFunction;
import org.apache.doris.nereids.trees.expressions.functions.generator.TableGeneratingFunction;
import org.apache.doris.nereids.trees.expressions.functions.scalar.NonNullable;
import org.apache.doris.nereids.trees.expressions.functions.scalar.Nullable;
import org.apache.doris.nereids.trees.expressions.functions.scalar.Sleep;
import org.apache.doris.nereids.trees.expressions.literal.ArrayLiteral;
import org.apache.doris.nereids.trees.expressions.literal.BigIntLiteral;
Expand All @@ -56,7 +58,6 @@
import org.apache.doris.nereids.trees.expressions.literal.Literal;
import org.apache.doris.nereids.trees.expressions.literal.MapLiteral;
import org.apache.doris.nereids.trees.expressions.literal.NullLiteral;
import org.apache.doris.nereids.trees.expressions.literal.NumericLiteral;
import org.apache.doris.nereids.trees.expressions.literal.SmallIntLiteral;
import org.apache.doris.nereids.trees.expressions.literal.StringLiteral;
import org.apache.doris.nereids.trees.expressions.literal.StructLiteral;
Expand Down Expand Up @@ -161,7 +162,11 @@ private static Expression replace(
Expression root, Map<String, Expression> constMap, Map<String, Expression> resultMap) {
for (Entry<String, Expression> entry : constMap.entrySet()) {
if (entry.getValue().equals(root)) {
return resultMap.get(entry.getKey());
if (resultMap.containsKey(entry.getKey())) {
return resultMap.get(entry.getKey());
} else {
return root;
}
}
}
List<Expression> newChildren = new ArrayList<>();
Expand All @@ -178,38 +183,7 @@ private static Expression replace(

private static void collectConst(Expression expr, Map<String, Expression> constMap,
Map<String, TExpr> tExprMap, IdGenerator<ExprId> idGenerator) {
if (expr.isConstant()) {
// Do not constant fold cast(null as dataType) because we cannot preserve the
// cast-to-types and that can lead to query failures, e.g., CTAS
if (expr instanceof Cast) {
if (((Cast) expr).child().isNullLiteral()) {
return;
}
if (shouldSkipFold(((Cast) expr).child())) {
return;
}
}
// skip literal expr
if (expr.isLiteral()) {
return;
}
// eg: avg_state(1) return is agg function serialize data
// and some type can't find a literal to represent.
// time type: need add a time literal in nereids
// IPv6 type: need get a library to output the compressed address format
if (expr.getDataType().isAggStateType() || expr.getDataType().isObjectType()
|| expr.getDataType().isVariantType() || expr.getDataType().isTimeLikeType()
|| expr.getDataType().isIPv6Type()) {
return;
}
// first need pass PlanTranslatorContext value,
// and ArrayItemReference translate, can't findColumnRef
// Match need give more info rather then as left child a NULL, in
// match_phrase_prefix/MATCH_PHRASE/MATCH_PHRASE/MATCH_ANY
if (shouldSkipFold(expr) || (expr instanceof TableGeneratingFunction)
|| (expr instanceof ArrayItemReference) || (expr instanceof Match)) {
return;
}
if (expr.isConstant() && !shouldSkipFold(expr)) {
String id = idGenerator.getNextId().toString();
constMap.put(id, expr);
Expr staleExpr;
Expand All @@ -233,23 +207,58 @@ private static void collectConst(Expression expr, Map<String, Expression> constM
}
}

// if sleep(5) will cause rpc timeout
// Some expressions should not do constant folding
private static boolean shouldSkipFold(Expression expr) {
// Skip literal expr
if (expr.isLiteral()) {
return true;
}

// Frontend can not represent those types
if (expr.getDataType().isAggStateType() || expr.getDataType().isObjectType()
|| expr.getDataType().isVariantType() || expr.getDataType().isTimeLikeType()
|| expr.getDataType().isIPv6Type()) {
return true;
}

// Frontend can not represent geo types
if (expr instanceof BoundFunction && ((BoundFunction) expr).getName().toLowerCase().startsWith("st_")) {
return true;
}

// TableGeneratingFunction need pass PlanTranslatorContext value
if (expr instanceof TableGeneratingFunction) {
return true;
}

// ArrayItemReference translate can't findColumnRef
if (expr instanceof ArrayItemReference) {
return true;
}

// Match need give more info rather then as left child a NULL, in
// match_phrase_prefix/MATCH_PHRASE/MATCH_PHRASE/MATCH_ANY
if (expr instanceof Match) {
return true;
}

// sleep will cause rpc timeout
if (expr instanceof Sleep) {
Expression param = expr.child(0);
if (param instanceof Cast) {
param = param.child(0);
}
if (param instanceof NumericLiteral) {
return ((NumericLiteral) param).getDouble() >= 5.0;
}
} else if (expr instanceof BoundFunction
&& ((BoundFunction) expr).getName().toLowerCase().startsWith("st_")) {
// FIXME: remove this when we fixed serde of Geo types
LOG.warn("GeoFunction {} now don't support constant fold on BE",
((BoundFunction) expr).getName());
return true;
}

// Do not constant fold cast(null as dataType) because we cannot preserve the
// cast-to-types and that can lead to query failures, e.g., CTAS
if (expr instanceof Cast && ((Cast) expr).child().isNullLiteral()) {
return true;
}

// This kind of function is often used to change the attributes of columns.
// Folding will make it impossible to construct columns such as nullable(1).
if (expr instanceof Nullable || expr instanceof NonNullable) {
return true;
}

return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,21 @@
-- !sql_1 --
80000

-- !sql --
PLAN FRAGMENT 0
OUTPUT EXPRS:
sleep(100)[#0]
PARTITION: UNPARTITIONED

HAS_COLO_PLAN_NODE: false

VRESULT SINK
MYSQL_PROTOCAL

0:VUNION(32)
constant exprs:
sleep(100)

-- !sql --
true

Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,8 @@ suite("fold_constant_by_be") {
def res2 = sql " select /*+SET_VAR(enable_fold_constant_by_be=false)*/ ST_CIRCLE(121.510651, 31.234391, 1918.0); "
log.info("result: {}, {}", res1, res2)
assertEquals(res1[0][0], res2[0][0])

qt_sql "explain select sleep(sign(1)*100);"
sql 'set query_timeout=12;'
qt_sql "select sleep(sign(1)*10);"
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ suite("select_random_distributed_tbl") {
"replication_allocation" = "tag.location.default: 1"
);
"""

sql 'set enable_fold_constant_by_be=true'
sql """ insert into ${tableName} values(1,"a",1,1,1,avg_state(1),hll_hash(1),bitmap_hash(1),to_quantile_state(1, 2048)) """
sql 'set enable_fold_constant_by_be=false'
sql """ insert into ${tableName} values(1,"a",2,2,2,avg_state(2),hll_hash(2),bitmap_hash(2),to_quantile_state(2, 2048)) """
sql """ insert into ${tableName} values(1,"a",3,3,3,avg_state(3),hll_hash(3),bitmap_hash(3),to_quantile_state(3, 2048)) """
sql """ insert into ${tableName} values(2,"b",4,4,4,avg_state(4),hll_hash(4),bitmap_hash(4),to_quantile_state(4, 2048)) """
Expand Down

0 comments on commit 723dfea

Please sign in to comment.