expression: support agg function var_pop #7718

mccxj · 2018-09-16T14:15:47Z

What problem does this PR solve?

What is changed and how it works?

mysql> create table t(a int(11), b float, c double, d decimal(6,2));
Query OK, 0 rows affected (0.01 sec)

mysql> select var_pop(a), var_pop(b), var_pop(c), var_pop(d) from t;
+------------+------------+------------+------------+
| var_pop(a) | var_pop(b) | var_pop(c) | var_pop(d) |
+------------+------------+------------+------------+
|       NULL |       NULL |       NULL |       NULL |
+------------+------------+------------+------------+
1 row in set (0.00 sec)

mysql> insert into t values(1, 2.5, 3.5, 6.25), (2, 3.5, 4.5, 8.55);
Query OK, 2 rows affected (0.00 sec)

mysql> select var_pop(a), var_pop(b), var_pop(c), var_pop(d) from t;
+------------+------------+------------+------------+
| var_pop(a) | var_pop(b) | var_pop(c) | var_pop(d) |
+------------+------------+------------+------------+
|       0.25 |       0.25 |       0.25 |       1.32 |
+------------+------------+------------+------------+
1 row in set (0.01 sec)

mysql> select var_pop(a), var_pop(b), variance(a), variance(b) from t;
+------------+------------+-------------+-------------+
| var_pop(a) | var_pop(b) | variance(a) | variance(b) |
+------------+------------+-------------+-------------+
|       0.25 |       0.25 |        0.25 |        0.25 |
+------------+------------+-------------+-------------+
1 row in set (0.01 sec)

mysql> select var_pop(a), var_pop(b), var_pop(c), var_pop(d) from t group by a;
+------------+------------+------------+------------+
| var_pop(a) | var_pop(b) | var_pop(c) | var_pop(d) |
+------------+------------+------------+------------+
|          0 |          0 |          0 |       0.00 |
|          0 |          0 |          0 |       0.00 |
+------------+------------+------------+------------+
2 rows in set (0.00 sec)

mysql> insert into t values(1, 3.5, 4.5, 5.25);
Query OK, 1 row affected (0.00 sec)

-- run with --column-type-info
mysql> select a, var_pop(a), var_pop(b), var_pop(c), var_pop(d) from t group by a;
Field   1:  `a`
Catalog:    `def`
Database:   `test`
Table:      `t`
Org_table:  `t`
Type:       LONG
Collation:  binary (63)
Length:     11
Max_length: 1
Decimals:   0
Flags:      NUM 

Field   2:  `var_pop(a)`
Catalog:    `def`
Database:   ``
Table:      ``
Org_table:  ``
Type:       DOUBLE
Collation:  binary (63)
Length:     23
Max_length: 1
Decimals:   0
Flags:      BINARY NUM 

Field   3:  `var_pop(b)`
Catalog:    `def`
Database:   ``
Table:      ``
Org_table:  ``
Type:       DOUBLE
Collation:  binary (63)
Length:     23
Max_length: 4
Decimals:   31
Flags:      BINARY NUM 

Field   4:  `var_pop(c)`
Catalog:    `def`
Database:   ``
Table:      ``
Org_table:  ``
Type:       DOUBLE
Collation:  binary (63)
Length:     23
Max_length: 4
Decimals:   31
Flags:      BINARY NUM 

Field   5:  `var_pop(d)`
Catalog:    `def`
Database:   ``
Table:      ``
Org_table:  ``
Type:       DOUBLE
Collation:  binary (63)
Length:     23
Max_length: 4
Decimals:   2
Flags:      BINARY NUM 


+------+------------+------------+------------+------------+
| a    | var_pop(a) | var_pop(b) | var_pop(c) | var_pop(d) |
+------+------------+------------+------------+------------+
|    2 |          0 |          0 |          0 |       0.00 |
|    1 |          0 |       0.25 |       0.25 |       0.25 |
+------+------------+------------+------------+------------+
2 rows in set (0.00 sec)

Check List

Tests

Unit test
Integration test

Code changes

Has exported function/method change
Has exported variable/fields change

This change is

mccxj · 2018-09-18T01:39:41Z

@zz-jason @jackysp @imtbkcat PTAL

jackysp · 2018-09-20T02:30:04Z

Great work! Thanks @mccxj !

expression/aggregation/var_pop.go

jackysp · 2018-09-20T02:50:01Z

PTAL @lamxTyler .

expression/aggregation/var_pop.go

executor/aggfuncs/func_var_pop.go

expression/aggregation/descriptor.go

winoros

Will you implement it on tikv too?

executor/aggfuncs/builder.go

executor/aggfuncs/func_var_pop.go

expression/aggregation/aggregation.go

executor/aggfuncs/func_var_pop.go

winoros · 2018-09-20T10:03:28Z

https://arxiv.org/pdf/1509.04349.pdf
related paper

mccxj · 2018-09-20T10:22:35Z

@winoros thanks, I got it, the algorithm works fine.

mccxj · 2018-09-22T08:17:49Z

@jackysp @zz-jason @winoros @lamxTyler PTAL

executor/aggfuncs/func_var_pop.go

parser/parser.y

parser/parser_test.go

expression/aggregation/variance.go

expression/aggregation/descriptor.go

expression/aggregation/aggregation.go

winoros · 2018-09-25T04:50:24Z

types/helper.go

+// This algorithm was proven to be numerically stable by J.L. Barlow in
+// "Error analysis of a pairwise summation algorithm to compute sample variance"
+// Numer. Math, 58 (1991) pp. 583--590
+func CalculateMerge(partialCount int64, mergeCount int64, partialSum float64, mergeSum float64, partialVariance float64, mergeVariance float64) float64 {


Why put it in types package? Let it be a member function of baseVarianceFunction is better?

types/helper.go

plan/rule_aggregation_push_down.go

executor/aggfuncs/func_variance.go

expression/aggregation/descriptor.go

zz-jason · 2018-10-09T02:46:55Z

@mccxj any update?

mccxj · 2018-11-27T01:17:15Z

PR for parser pingcap/parser#53

mccxj · 2018-11-27T01:49:46Z

PTAL @jackysp @zz-jason @winoros @XuHuaiyu @lamxTyler

zz-jason

This PR contains two main changes:

implement var_pop function in the TiDB layer, which located in "executor/aggfuncs/"
implement var_pop function in the mocktikv coprocessor layer, which located in "expression/aggregation/"
support the aggregate function var_pop() to be push down to tikv coprocessor.

Can you split this PR to three PRs:

the first PR to implement the var_pop function in the TiDB layer
the second PR to var_pop function in the mocktikv coprocessor layer, which located in "expression/aggregation/" and add some unit tests.
the third PR to support the aggregate function var_pop() to be push down to tikv coprocessor.

Before the third PR, we should also add some integration tests to test tidb+tikv, after all the integration tests are passed, we can safely push down the var_pop function to the tikv layer.

zz-jason · 2018-11-28T07:26:27Z

executor/aggfuncs/func_variance.go

+type partialResult4VarianceFloat64 struct {
+	count    int64
+	sum      float64
+	variance float64 // $$\sum_{k=i}^{j}{(a_k-avg_{ij})^2}$$ (this is actually n times the variance)


Uh.. How about use the normal form to express the math formula? For example:

sum((a[i]-avg)^2)

zz-jason · 2018-11-28T07:28:57Z

executor/aggfuncs/func_variance.go

+		chk.AppendFloat64(e.ordinal, p.variance/float64(p.count))
+	default:
+		// TODO support more population standard deviation/sample variance
+		panic("Not implemented")


we can replace the panic with an error and return that error back.

zz-jason · 2018-11-28T07:44:01Z

planner/core/rule_aggregation_push_down.go

@@ -36,7 +36,7 @@ type aggregationPushDownSolver struct {
 // Currently we don't support avg and concat.
 func (a *aggregationPushDownSolver) isDecomposable(fun *aggregation.AggFuncDesc) bool {
 	switch fun.Name {
-	case ast.AggFuncAvg, ast.AggFuncGroupConcat:
+	case ast.AggFuncAvg, ast.AggFuncGroupConcat, ast.AggFuncVarPop:
 		// TODO: Support avg push down.


Seems this TODO message needs to be modified.

zz-jason · 2018-11-28T07:45:15Z

types/helper.go

+// This algorithm was proven to be numerically stable by J.L. Barlow in
+// "Error analysis of a pairwise summation algorithm to compute sample variance"
+// Numer. Math, 58 (1991) pp. 583--590
+func CalculateMerge(partialCount int64, mergeCount int64, partialSum float64, mergeSum float64, partialVariance float64, mergeVariance float64) float64 {


zz-jason · 2019-08-03T06:06:46Z

@mccxj friendly ping, any update?

sre-bot · 2019-08-03T06:06:52Z

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

zz-jason · 2019-08-19T02:46:05Z

@mccxj I'm going to close this PR due to outdated, feel free to reopen it if needed.

imtbkcat added the component/expression label Sep 17, 2018

zz-jason mentioned this pull request Sep 17, 2018

Call For Participation: finish the unsupported aggregate functions #7623

Closed

6 tasks

mccxj force-pushed the var_pop branch 2 times, most recently from 630b54c to 403d894 Compare September 17, 2018 14:25

mccxj changed the title ~~expression: support agg function var_pop(WIP)~~ expression: support agg function var_pop Sep 17, 2018

mccxj force-pushed the var_pop branch from 403d894 to 99c7f3a Compare September 17, 2018 14:30

jackysp reviewed Sep 20, 2018

View reviewed changes

expression/aggregation/var_pop.go Outdated Show resolved Hide resolved

jackysp reviewed Sep 20, 2018

View reviewed changes

expression/aggregation/var_pop.go Outdated Show resolved Hide resolved

jackysp reviewed Sep 20, 2018

View reviewed changes

executor/aggfuncs/func_var_pop.go Outdated Show resolved Hide resolved

jackysp reviewed Sep 20, 2018

View reviewed changes

expression/aggregation/descriptor.go Outdated Show resolved Hide resolved

winoros reviewed Sep 20, 2018

View reviewed changes

executor/aggfuncs/builder.go Show resolved Hide resolved

alivxxx reviewed Sep 20, 2018

View reviewed changes

executor/aggfuncs/func_var_pop.go Outdated Show resolved Hide resolved

expression/aggregation/aggregation.go Outdated Show resolved Hide resolved

zz-jason reviewed Sep 20, 2018

View reviewed changes

executor/aggfuncs/func_var_pop.go Outdated Show resolved Hide resolved

zz-jason added type/enhancement The issue or PR belongs to an enhancement. contribution This PR is from a community contributor. sig/execution SIG execution labels Sep 20, 2018

zz-jason reviewed Sep 23, 2018

View reviewed changes

zz-jason mentioned this pull request Sep 23, 2018

expression: add new aggregate functions "stddev_pop" and "stddev_samp" #7766

Closed

zz-jason reviewed Sep 25, 2018

View reviewed changes

winoros reviewed Sep 25, 2018

View reviewed changes

XuHuaiyu reviewed Sep 27, 2018

View reviewed changes

expression/aggregation/descriptor.go Show resolved Hide resolved

expression: support agg function var_pop

b2035de

mccxj force-pushed the var_pop branch from f5dd198 to b2035de Compare November 25, 2018 09:18

mccxj added 4 commits November 26, 2018 16:20

Merge branch 'master' into var_pop

9210fb6

Update go.sum

aa4ada9

Update go.mod

69a0d83

Update aggregation_test.go

c4b8f48

Update descriptor.go

ce8f27b

mccxj mentioned this pull request Nov 27, 2018

*: add agg function var_pop tikv/tikv#3835

Closed

zz-jason reviewed Nov 28, 2018

View reviewed changes

mccxj mentioned this pull request Dec 2, 2018

expression: support agg function var_pop part 1 #8542

Closed

zz-jason added the status/ReqChange label Aug 3, 2019

zz-jason closed this Aug 19, 2019

zz-jason added closed/outdated and removed status/ReqChange labels Aug 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expression: support agg function var_pop #7718

expression: support agg function var_pop #7718

mccxj commented Sep 16, 2018 •

edited

Loading

mccxj commented Sep 18, 2018

jackysp commented Sep 20, 2018 •

edited

Loading

jackysp commented Sep 20, 2018

winoros left a comment

winoros commented Sep 20, 2018

mccxj commented Sep 20, 2018

mccxj commented Sep 22, 2018

winoros Sep 25, 2018

zz-jason Nov 28, 2018

zz-jason commented Oct 9, 2018

mccxj commented Nov 27, 2018

mccxj commented Nov 27, 2018

zz-jason left a comment

zz-jason Nov 28, 2018

zz-jason Nov 28, 2018

zz-jason Nov 28, 2018

zz-jason Nov 28, 2018

zz-jason commented Aug 3, 2019

sre-bot commented Aug 3, 2019

zz-jason commented Aug 19, 2019

expression: support agg function var_pop #7718

expression: support agg function var_pop #7718

Conversation

mccxj commented Sep 16, 2018 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

mccxj commented Sep 18, 2018

jackysp commented Sep 20, 2018 • edited Loading

jackysp commented Sep 20, 2018

winoros left a comment

Choose a reason for hiding this comment

winoros commented Sep 20, 2018

mccxj commented Sep 20, 2018

mccxj commented Sep 22, 2018

winoros Sep 25, 2018

Choose a reason for hiding this comment

zz-jason Nov 28, 2018

Choose a reason for hiding this comment

zz-jason commented Oct 9, 2018

mccxj commented Nov 27, 2018

mccxj commented Nov 27, 2018

zz-jason left a comment

Choose a reason for hiding this comment

zz-jason Nov 28, 2018

Choose a reason for hiding this comment

zz-jason Nov 28, 2018

Choose a reason for hiding this comment

zz-jason Nov 28, 2018

Choose a reason for hiding this comment

zz-jason Nov 28, 2018

Choose a reason for hiding this comment

zz-jason commented Aug 3, 2019

sre-bot commented Aug 3, 2019

zz-jason commented Aug 19, 2019

mccxj commented Sep 16, 2018 •

edited

Loading

jackysp commented Sep 20, 2018 •

edited

Loading