Refactor expressions by sundy-li · Pull Request #550 · databendlabs/databend

sundy-li · 2021-05-15T03:51:57Z

Summary

RFC: https://github.com/sundy-li/fuse-query/blob/action_dag/website/datafuse/docs/rfcs/query/2021-05-22-plan-expression-refactor.md

split ScalarFunction and AggregateFunction
expression rewriter and expression visitor
expression chain in AST to plan builder (simple version as ActionsDag)
testing

A complex Example

EXPLAIN actions = 1
SELECT
    number + 1 AS number,
    number + 2 AS a,
    sum(number % 3) + 4 AS b
FROM numbers(10)
GROUP BY
    number,
    a
HAVING (b + 1) > 3

Query id: 9675ed9e-81fc-40dc-b9c7-5c33746db915

┌─explain───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Expression (Projection)                                                                                                                                   │
│ Actions: PROJECT plus(number, 1) AS number, plus(plus(number, 1), 2) AS a, plus(sum(modulo(plus(number, 1), 3)), 4) AS b                                  │
│   Expression (Before ORDER BY and SELECT)                                                                                                                 │
│   Actions: PROJECT plus(number, 1), plus(plus(number, 1), 2), plus(sum(modulo(plus(number, 1), 3)), 4)                                                    │
│     Filter (HAVING)                                                                                                                                       │
│     Filter column: greater(plus(plus(sum(modulo(plus(number, 1), 3)), 4), 1), 3)                                                                          │
│     Actions: PROJECT plus(number, 1), plus(plus(number, 1), 2), sum(modulo(plus(number, 1), 3))                                                           │
│              ADD 4 UInt8 Const(UInt8)                                                                                                                     │
│              ADD 1 UInt8 Const(UInt8)                                                                                                                     │
│              ADD 3 UInt8 Const(UInt8)                                                                                                                     │
│              FUNCTION plus(sum(modulo(plus(number, 1), 3)), 4) UInt64 = plus(sum(modulo(plus(number, 1), 3)), 4)                                          │
│              REMOVE sum(modulo(plus(number, 1), 3))                                                                                                       │
│              REMOVE 4                                                                                                                                     │
│              FUNCTION plus(plus(sum(modulo(plus(number, 1), 3)), 4), 1) UInt64 = plus(plus(sum(modulo(plus(number, 1), 3)), 4), 1)                        │
│              REMOVE 1                                                                                                                                     │
│              FUNCTION greater(plus(plus(sum(modulo(plus(number, 1), 3)), 4), 1), 3) UInt8 = greater(plus(plus(sum(modulo(plus(number, 1), 3)), 4), 1), 3) │
│              REMOVE plus(plus(sum(modulo(plus(number, 1), 3)), 4), 1)                                                                                     │
│              REMOVE 3                                                                                                                                     │
│       Aggregating                                                                                                                                         │
│       Keys: plus(number, 1), plus(plus(number, 1), 2)                                                                                                     │
│       Aggregates:                                                                                                                                         │
│           sum(modulo(plus(number, 1), 3))                                                                                                                 │
│             Function: sum(UInt8) → UInt64                                                                                                                 │
│             Arguments: modulo(plus(number, 1), 3)                                                                                                         │
│             Argument positions: 2                                                                                                                         │
│         Expression (Before GROUP BY)                                                                                                                      │
│         Actions: ADD 1 UInt8 Const(UInt8)                                                                                                                 │
│                  ADD 2 UInt8 Const(UInt8)                                                                                                                 │
│                  ADD 3 UInt8 Const(UInt8)                                                                                                                 │
│                  FUNCTION plus(number, 1) UInt64 = plus(number, 1)                                                                                        │
│                  REMOVE number                                                                                                                            │
│                  REMOVE 1                                                                                                                                 │
│                  FUNCTION plus(plus(number, 1), 2) UInt64 = plus(plus(number, 1), 2)                                                                      │
│                  REMOVE 2                                                                                                                                 │
│                  FUNCTION modulo(plus(number, 1), 3) UInt8 = modulo(plus(number, 1), 3)                                                                   │
│                  REMOVE 3                                                                                                                                 │
│           SettingQuotaAndLimits (Set limits and quota after reading from storage)                                                                         │
│             ReadFromStorage (SystemNumbers)                                                                                                               │
└───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

38 rows in set. Elapsed: 0.014 sec.

There are some main steps to go:

Alias extracting and expression rewritten by aliases, results will be

SELECT
    number + 1 AS number,
    (number + 1) + 2 AS a,
    sum((number+1)% 3) + 4  as b
FROM numbers(10) group by (number + 1), (number + 1) + 2  having (sum((number+1) % 3) + 4) + 1 > 3

Apply expressions before group by, visit all expressions except aggregate functions.

eg expressions:

plus(number, 1)
plus(plus(number, 1), 2)
modulo(plus(number, 1), 3)

If we already applied expressions, we can make expressions materialized as a Column expression for downstream to get from the block.

ExpressionFunction :  plus(number, 1)     ---> ExpressionColumn:  plus(number, 1)

Apply expressions to the group by, visit all expressions to find aggregate functions.

 Keys: plus(number, 1), plus(plus(number, 1), 2)
 Aggregates:
    sum(modulo(plus(number, 1), 3))

Apply expressions after group by

eg:

FUNCTION plus(sum(modulo(plus(number, 1), 3)), 4)

Changelog

Improvement

Related Issues

Test Plan

databend-bot · 2021-05-21T14:38:23Z

Thanks for the contribution!
I have applied any labels matching special text in your title and description.

Please review the labels and make any necessary changes.

codecov-commenter · 2021-05-25T01:33:08Z

Codecov Report

Merging #550 (af0226b) into master (8563bfd) will decrease coverage by 1%.
The diff coverage is 82%.

@@           Coverage Diff           @@
##           master    #550    +/-   ##
=======================================
- Coverage      81%     80%    -2%     
=======================================
  Files         292     288     -4     
  Lines       13849   13758    -91     
=======================================
- Hits        11290   11028   -262     
- Misses       2559    2730   +171

Impacted Files	Coverage Δ
common/datavalues/src/data_array.rs	`100% <ø> (ø)`
common/datavalues/src/data_value.rs	`27% <ø> (-3%)`	⬇️
common/datavalues/src/data_value_kernel.rs	`37% <ø> (ø)`
common/datavalues/src/macros.rs	`86% <ø> (+27%)`	⬆️
common/exception/src/exception.rs	`42% <ø> (ø)`
common/functions/src/function_alias.rs	`0% <0%> (-88%)`	⬇️
common/functions/src/function_literal.rs	`0% <0%> (-94%)`	⬇️
common/functions/src/strings/string.rs	`83% <ø> (ø)`
common/planners/src/plan_display_test.rs	`92% <ø> (ø)`
common/planners/src/plan_expression_action_test.rs	`95% <0%> (ø)`
... and 100 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8563bfd...af0226b. Read the comment docs.

jyizheng · 2021-05-25T05:26:50Z

        plan: Result<PlanNode>,
-        expect: &'static str
+        expect: &'static str,
+        err: &'static str


It is neat to add the expected error here.

err is expected error msg, empty str for normal return.

zhang2014 · 2021-05-25T07:16:11Z

                                    "{:?}:{:?}",
                                    plan.exprs[i],
-                                    plan.schema().fields()[i].data_type()
+                                    plan.exprs[i].to_data_type(&plan.input.schema()).unwrap()


unwrap can cause panic. use ? operator

It's for display, may need ErrorCodes to fmt error. We can make another pr to improve this.

zhang2014 · 2021-05-25T07:23:03Z

+use crate::test::Test;
+use crate::*;
+
 // Copyright 2020-2021 The Datafuse Authors.


Should be move to file header

zhang2014 · 2021-05-25T07:43:41Z

+/// ExpressionExecutor is a helper struct for expressions and projections
+/// Aggregate functions is not covered, because all expressions in aggregate functions functions are executed.
+#[derive(Debug, Clone)]
+pub struct ExpressionExecutor {


👍 Great work.

zhang2014

LGTM

ives9638 · 2021-05-25T09:28:48Z

Feat!

github-actions Bot added the fusequery label May 15, 2021

sundy-li mentioned this pull request May 18, 2021

Alias push down optimization seems to be logically wrong #574

Closed

leiysky mentioned this pull request May 18, 2021

Allow PlanNode to have multiple children #563

Merged

sundy-li added 9 commits May 20, 2021 15:53

wip

5382192

wip

46429dd

[expressions] refactor

59d9584

[functions] rafactor IFunction trait

5318b6b

[aggregate] split aggregat functions

7f0c04c

[functions] remove FunctionCtx

8b188b3

[planner] add plan_expression_chain

32ca730

[transform] wip

c95ec97

[functions] make functions passed build

b912d33

sundy-li force-pushed the action_dag branch from e19ab1d to b912d33 Compare May 20, 2021 08:24

sundy-li mentioned this pull request May 20, 2021

[feature]Function arguments validate #591

Closed

sundy-li added 2 commits May 21, 2021 13:37

[fusequery] make query build ok

09c0fc0

[fusequery] make build happy

6daefb5

databend-bot added the pr-improvement label May 21, 2021

sundy-li added 3 commits May 23, 2021 19:57

[agg] Refactor aggregate transform

bec80c3

[agg] Refactor sql planner builder

cf74dc0

[tests] rebase

ee37b41

sundy-li mentioned this pull request May 24, 2021

add NOT expression #600

Merged

sundy-li added 5 commits May 24, 2021 17:41

[fusequery] make tests happy

148c9e0

[fusequery] make tests happy

5bdff5c

[fusequery] seems everything is done

1f0c6e4

Merge branch 'master' into action_dag

862cefb

[fusequery] seems everything is done

004500a

sundy-li mentioned this pull request May 25, 2021

support substring function with minus argument #611

Merged

[docs] add rfcs

cea6296

jyizheng reviewed May 25, 2021

View reviewed changes

Comment thread website/datafuse/docs/rfcs/query/2021-05-22-plan-expression-refactor.md Outdated

jyizheng reviewed May 25, 2021

View reviewed changes

Comment thread common/aggregate_functions/src/aggregate_avg.rs Outdated

sundy-li added 2 commits May 25, 2021 13:57

[docs] fix typos

78e42a5

[planner] merge master

cdd0e75

sundy-li marked this pull request as ready for review May 25, 2021 06:47

sundy-li requested review from bohutang and zhang2014 May 25, 2021 06:53

zhang2014 reviewed May 25, 2021

View reviewed changes

Comment thread common/aggregate_functions/src/aggregate_function_factory.rs Outdated

zhang2014 reviewed May 25, 2021

View reviewed changes

Comment thread common/aggregate_functions/src/test.rs Outdated

zhang2014 reviewed May 25, 2021

View reviewed changes

Comment thread common/functions/src/test.rs Outdated

[aggr] remove unused files

af0226b

zhang2014 reviewed May 25, 2021

View reviewed changes

zhang2014 approved these changes May 25, 2021

View reviewed changes

[query] better codes

335ac5e

bohutang merged commit be3cce6 into databendlabs:master May 25, 2021

This was referenced May 25, 2021

ExpressionAction: Group By #505

Closed

Function Error: 'AggregatorMaxFunction' eval unimplemented #538

Closed

Split IFunction to IFunction and IAggregateFunction #480

Closed

lideen999 mentioned this pull request May 25, 2021

After merging Refactor expressions #550, the performance of aggregate is low. #619

Closed

sundy-li mentioned this pull request May 26, 2021

[groupby] fix some hotpath codes for performance #631

Merged

Uh oh!

Conversation

sundy-li commented May 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

A complex Example

Changelog

Related Issues

Test Plan

Uh oh!

databend-bot commented May 21, 2021

Uh oh!

codecov-commenter commented May 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

jyizheng May 25, 2021

Choose a reason for hiding this comment

Uh oh!

sundy-li May 25, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zhang2014 May 25, 2021

Choose a reason for hiding this comment

Uh oh!

sundy-li May 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhang2014 May 25, 2021

Choose a reason for hiding this comment

Uh oh!

zhang2014 May 25, 2021

Choose a reason for hiding this comment

Uh oh!

zhang2014 left a comment

Choose a reason for hiding this comment

Uh oh!

ives9638 commented May 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

sundy-li commented May 15, 2021 •

edited

Loading

codecov-commenter commented May 25, 2021 •

edited

Loading

sundy-li May 25, 2021 •

edited

Loading