Skip to content

Commit

Permalink
Merge pull request #55999 from yuzefovich/backport20.1-55957
Browse files Browse the repository at this point in the history
release-20.1: builtins: fix sqrdiff when used as a window function
  • Loading branch information
yuzefovich committed Oct 28, 2020
2 parents ce32adc + 21aebcb commit bd24aeb
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 2 deletions.
34 changes: 34 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/aggregate
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,40 @@ true
statement ok
DROP TABLE xyz

# SQRDIFF

statement ok
DROP TABLE IF EXISTS ifd;
CREATE TABLE ifd
(
i int,
f float,
d decimal
);
INSERT INTO ifd (i, f, d)
VALUES (1, 1.1, 1.1),
(2, 2.2, 2.2),
(5, 3.0, 3.0),
(10, 7.8, 7.8),
(11, 9.0, 9.0),
(18, 11.2, 11.2);

query RRR
select round(sqrdiff(i), 21), round(sqrdiff(f), 12), round(sqrdiff(d), 21)
from ifd
----
206.833333333333333333333 86.248333333333 86.248333333333333333333

query RRR
SELECT round(sqrdiff(i), 23), round(sqrdiff(f), 2), round(sqrdiff(d), 2)
FROM ifd
where i < 10
----
8.66666666666666666666667 1.82 1.82

statement ok
DROP TABLE ifd

# Numerical stability test for VARIANCE/STDDEV.
# See https://www.johndcook.com/blog/2008/09/28/theoretical-explanation-for-numerical-results.
# Avoid using random() since we do not have the deterministic option to specify a pseudo-random seed yet.
Expand Down
36 changes: 35 additions & 1 deletion pkg/sql/logictest/testdata/logic_test/window
Original file line number Diff line number Diff line change
Expand Up @@ -3686,7 +3686,7 @@ ORDER BY

# Check that telemetry is being collected on the window functions' usage.
query B
SELECT count(*) = 26 FROM crdb_internal.feature_usage WHERE feature_name LIKE 'sql.plan.window_function%' AND usage_count > 0
SELECT count(*) >= 26 FROM crdb_internal.feature_usage WHERE feature_name LIKE 'sql.plan.window_function%' AND usage_count > 0
----
true

Expand Down Expand Up @@ -3840,3 +3840,37 @@ statement ok
SELECT max(b::INT8) OVER (PARTITION BY b ORDER BY a RANGE 1 PRECEDING)
FROM t53442_b NATURAL JOIN t53442_a
WHERE false

# Regression test for window aggregate functions not making a deep copy of
# decimals on each iteration (#55944).
statement ok
CREATE TABLE t55944 (x decimal);
INSERT INTO t55944 (x)
VALUES (1.0),
(20.0),
(25.0),
(41.0),
(55.5),
(60.9),
(72.0),
(88.0),
(88.0),
(89.0);

query RRR
SELECT x,
round(sqrdiff(x) OVER (ORDER BY x), 20) as sqrdiff,
round(stddev_samp(x) OVER (ORDER BY x), 17) as stddev_samp
FROM t55944
ORDER BY x
----
1.0 0E-20 NULL
20.0 180.50000000000000000000 13.43502884254440296
25.0 320.66666666666666666667 12.66227994214838599
41.0 814.75000000000000000000 16.47978559731082586
55.5 1726.00000000000000000000 20.77257807784098810
60.9 2600.80000000000000000000 22.80701646423749132
72.0 3845.03714285714285714286 25.31480838974539443
88.0 7527.84222222222222222222 30.67540183563660797
88.0 7527.84222222222222222222 30.67540183563660797
89.0 8885.84400000000000000000 31.42158493774621802
3 changes: 2 additions & 1 deletion pkg/sql/sem/builtins/aggregate_builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -1864,7 +1864,8 @@ func (a *decimalSqrDiffAggregate) Result() (tree.Datum, error) {
if a.count.Cmp(decimalOne) < 0 {
return tree.DNull, nil
}
dd := &tree.DDecimal{Decimal: a.sqrDiff}
dd := &tree.DDecimal{}
dd.Set(&a.sqrDiff)
// Remove trailing zeros. Depending on the order in which the input
// is processed, some number of trailing zeros could be added to the
// output. Remove them so that the results are the same regardless of order.
Expand Down
39 changes: 39 additions & 0 deletions pkg/sql/sem/builtins/aggregate_builtins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,21 @@ func TestVarianceDecimalResultDeepCopy(t *testing.T) {
testAggregateResultDeepCopy(t, newDecimalVarianceAggregate, makeDecimalTestDatum(10))
}

func TestSqrDiffIntResultDeepCopy(t *testing.T) {
defer leaktest.AfterTest(t)()
testAggregateResultDeepCopy(t, newIntSqrDiffAggregate, makeIntTestDatum(10))
}

func TestSqrDiffFloatResultDeepCopy(t *testing.T) {
defer leaktest.AfterTest(t)()
testAggregateResultDeepCopy(t, newFloatSqrDiffAggregate, makeFloatTestDatum(10))
}

func TestSqrDiffDecimalResultDeepCopy(t *testing.T) {
defer leaktest.AfterTest(t)()
testAggregateResultDeepCopy(t, newDecimalSqrDiffAggregate, makeDecimalTestDatum(10))
}

func TestStdDevIntResultDeepCopy(t *testing.T) {
defer leaktest.AfterTest(t)()
testAggregateResultDeepCopy(t, newIntStdDevAggregate, makeIntTestDatum(10))
Expand Down Expand Up @@ -538,6 +553,30 @@ func BenchmarkVarianceAggregateDecimal(b *testing.B) {
}
}

func BenchmarkSqrDiffAggregateInt(b *testing.B) {
for _, count := range []int{1000} {
b.Run(fmt.Sprintf("count=%d", count), func(b *testing.B) {
runBenchmarkAggregate(b, newIntSqrDiffAggregate, makeIntTestDatum(count))
})
}
}

func BenchmarkSqrDiffAggregateFloat(b *testing.B) {
for _, count := range []int{1000} {
b.Run(fmt.Sprintf("count=%d", count), func(b *testing.B) {
runBenchmarkAggregate(b, newFloatSqrDiffAggregate, makeFloatTestDatum(count))
})
}
}

func BenchmarkSqrDiffAggregateDecimal(b *testing.B) {
for _, count := range []int{1000} {
b.Run(fmt.Sprintf("count=%d", count), func(b *testing.B) {
runBenchmarkAggregate(b, newDecimalSqrDiffAggregate, makeDecimalTestDatum(count))
})
}
}

func BenchmarkStdDevAggregateInt(b *testing.B) {
for _, count := range []int{1000} {
b.Run(fmt.Sprintf("count=%d", count), func(b *testing.B) {
Expand Down

0 comments on commit bd24aeb

Please sign in to comment.