Skip to content

Commit 9ba8157

Browse files
CoollZzzCoollZzz
committed
Fix statistical aggregation semantics and reviewer issues
Use sample-corrected skewness to align with kurtosis, avoid long overflow in kurtosis formulas, normalize tree-model central moments after removal, and clean up correlation/type-inference related reviewer comments.
1 parent 4080987 commit 9ba8157

11 files changed

Lines changed: 43 additions & 80 deletions

File tree

‎integration-test/src/test/java/org/apache/iotdb/db/it/aggregation/IoTDBStatisticalAggregationIT.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,7 @@ public void testMomentsWithDifferentTypes() {
508508
};
509509
String[] retArray =
510510
new String[] {
511-
"0.0,0.4082482904638631,-1.2000000000000002,-3.333333333333333,0.0,0.4082482904638631,-1.2000000000000002,-3.333333333333333,"
511+
"0.0,0.6085806194501846,-1.2000000000000002,-3.333333333333333,0.0,0.6085806194501846,-1.2000000000000002,-3.333333333333333,"
512512
};
513513
resultSetEqualTest(
514514
"select skewness(s1),skewness(s2),kurtosis(s1),kurtosis(s2),"
@@ -520,7 +520,7 @@ public void testMomentsWithDifferentTypes() {
520520
new String[] {
521521
skewness("root.db.d1.s1"), skewness("root.db.d1.s2"), kurtosis("root.db.d1.s1")
522522
};
523-
retArray = new String[] {"0.0,-0.7071067811865475,null,"};
523+
retArray = new String[] {"0.0,-1.7320508075688774,null,"};
524524
resultSetEqualTest(
525525
"select skewness(s1),skewness(s2),kurtosis(s1) from root.db.d1 where time < 10",
526526
expectedHeader,
@@ -534,13 +534,13 @@ public void testMomentsAlignByDevice() {
534534
DEVICE, skewness("s1"), skewness("s2"), kurtosis("s1"), kurtosis("s2"),
535535
};
536536
String[] retArray =
537-
new String[] {"root.db.d1,0.0,0.4082482904638631,-1.2000000000000002,-3.333333333333333,"};
537+
new String[] {"root.db.d1,0.0,0.6085806194501846,-1.2000000000000002,-3.333333333333333,"};
538538
resultSetEqualTest(
539539
"select skewness(s1),skewness(s2),kurtosis(s1),kurtosis(s2) from root.db.d1 align by device",
540540
expectedHeader,
541541
retArray);
542542

543-
retArray = new String[] {"root.db.d1,0.0,-0.7071067811865475,null,null,"};
543+
retArray = new String[] {"root.db.d1,0.0,-1.7320508075688774,null,null,"};
544544
resultSetEqualTest(
545545
"select skewness(s1),skewness(s2),kurtosis(s1),kurtosis(s2) from root.db.d1 "
546546
+ "where time < 10 align by device",
@@ -551,7 +551,7 @@ DEVICE, skewness("s1"), skewness("s2"), kurtosis("s1"), kurtosis("s2"),
551551
@Test
552552
public void testMomentsInHaving() {
553553
String[] expectedHeader = new String[] {skewness("root.db.d1.s2"), kurtosis("root.db.d1.s2")};
554-
String[] retArray = new String[] {"0.4082482904638631,-3.333333333333333,"};
554+
String[] retArray = new String[] {"0.6085806194501846,-3.333333333333333,"};
555555
resultSetEqualTest(
556556
"select skewness(s2),kurtosis(s2) from root.db.d1 having skewness(s2) > 0",
557557
expectedHeader,
@@ -569,7 +569,7 @@ public void testMomentsWithGroupByLevel() {
569569
};
570570
String[] retArray =
571571
new String[] {
572-
"1.0606601717798214,0.25714285714285623,-0.8728715609439697,-1.224489795918367,"
572+
"1.2577882373436315,0.25714285714285623,-1.0350983390135313,-1.224489795918367,"
573573
};
574574
resultSetEqualTest(
575575
"select skewness(s1),kurtosis(s1),skewness(s2),kurtosis(s2) from root.db.* group by level = 0",

‎iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/execution/aggregation/CentralMomentAccumulator.java‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
public class CentralMomentAccumulator implements Accumulator {
2929

3030
private static final int INTERMEDIATE_SIZE = Long.BYTES + 4 * Double.BYTES;
31+
private static final double EPSILON = 1e-12;
3132

3233
public enum MomentType {
3334
SKEWNESS,
@@ -173,7 +174,7 @@ public void outputIntermediate(ColumnBuilder[] columnBuilders) {
173174

174175
@Override
175176
public void outputFinal(ColumnBuilder columnBuilder) {
176-
if (count == 0 || m2 == 0) {
177+
if (count == 0 || Math.abs(m2) < EPSILON) {
177178
columnBuilder.appendNull();
178179
return;
179180
}
@@ -183,18 +184,17 @@ public void outputFinal(ColumnBuilder columnBuilder) {
183184
columnBuilder.appendNull();
184185
return;
185186
}
186-
double result = Math.sqrt((double) count) * m3 / Math.pow(m2, 1.5);
187+
double n = count;
188+
double result = n * Math.sqrt(n - 1) * m3 / ((n - 2) * Math.pow(m2, 1.5));
187189
columnBuilder.writeDouble(result);
188190
} else {
189191
if (count < 4) {
190192
columnBuilder.appendNull();
191193
} else {
192-
194+
double n = count;
193195
double variance = m2 / (count - 1);
194-
double term1 =
195-
(count * (count + 1) * m4)
196-
/ ((count - 1) * (count - 2) * (count - 3) * variance * variance);
197-
double term2 = (3 * Math.pow(count - 1, 2)) / ((count - 2) * (count - 3));
196+
double term1 = (n * (n + 1) * m4) / ((n - 1) * (n - 2) * (n - 3) * variance * variance);
197+
double term2 = (3 * (n - 1) * (n - 1)) / ((n - 2) * (n - 3));
198198
columnBuilder.writeDouble(term1 - term2);
199199
}
200200
}
@@ -259,9 +259,9 @@ public void removeIntermediate(Column[] input) {
259259

260260
count = nA;
261261
mean = meanA;
262-
m2 = m2A;
263-
m3 = m3A;
264-
m4 = m4A;
262+
m2 = normalizeZero(m2A);
263+
m3 = normalizeZero(m3A);
264+
m4 = normalizeZero(m4A);
265265
}
266266

267267
@Override
@@ -281,6 +281,10 @@ public void reset() {
281281
m4 = 0;
282282
}
283283

284+
private double normalizeZero(double value) {
285+
return Math.abs(value) < EPSILON ? 0 : value;
286+
}
287+
284288
@Override
285289
public boolean hasFinalResult() {
286290
return false;

‎iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/execution/aggregation/CorrelationAccumulator.java‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,7 @@ public class CorrelationAccumulator implements Accumulator {
3434

3535
private static final int INTERMEDIATE_SIZE = Long.BYTES + 5 * Double.BYTES;
3636

37-
public enum CorrelationType {
38-
CORR
39-
}
40-
4137
private final TSDataType[] seriesDataTypes;
42-
private final CorrelationType correlationType;
4338

4439
private long count;
4540
private double meanX;
@@ -48,9 +43,8 @@ public enum CorrelationType {
4843
private double m2Y;
4944
private double c2;
5045

51-
public CorrelationAccumulator(TSDataType[] seriesDataTypes, CorrelationType correlationType) {
46+
public CorrelationAccumulator(TSDataType[] seriesDataTypes) {
5247
this.seriesDataTypes = seriesDataTypes;
53-
this.correlationType = correlationType;
5448
}
5549

5650
@Override
@@ -177,10 +171,6 @@ public void outputIntermediate(ColumnBuilder[] columnBuilders) {
177171

178172
@Override
179173
public void outputFinal(ColumnBuilder columnBuilder) {
180-
if (correlationType != CorrelationType.CORR) {
181-
throw new UnsupportedOperationException("Unknown type: " + correlationType);
182-
}
183-
184174
double result = c2 / Math.sqrt(m2X * m2Y);
185175
if (Double.isFinite(result)) {
186176
columnBuilder.writeDouble(result);

‎iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/execution/operator/source/relational/aggregation/AccumulatorFactory.java‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
package org.apache.iotdb.calc.execution.operator.source.relational.aggregation;
2121

2222
import org.apache.iotdb.calc.execution.aggregation.CentralMomentAccumulator;
23-
import org.apache.iotdb.calc.execution.aggregation.CorrelationAccumulator;
2423
import org.apache.iotdb.calc.execution.aggregation.CovarianceAccumulator;
2524
import org.apache.iotdb.calc.execution.aggregation.RegressionAccumulator;
2625
import org.apache.iotdb.calc.execution.aggregation.VarianceAccumulator;
@@ -299,10 +298,7 @@ private static GroupedAccumulator createBuiltinGroupedAccumulator(
299298
return new GroupedApproxPercentileWithWeightAccumulator(inputDataTypes.get(0));
300299
}
301300
case CORR:
302-
return new GroupedCorrelationAccumulator(
303-
inputDataTypes.get(0),
304-
inputDataTypes.get(1),
305-
CorrelationAccumulator.CorrelationType.CORR);
301+
return new GroupedCorrelationAccumulator(inputDataTypes.get(0), inputDataTypes.get(1));
306302
case COVAR_POP:
307303
return new GroupedCovarianceAccumulator(
308304
inputDataTypes.get(0),
@@ -393,10 +389,7 @@ public static TableAccumulator createBuiltinAccumulator(
393389
return new ApproxPercentileWithWeightAccumulator(inputDataTypes.get(0));
394390
}
395391
case CORR:
396-
return new TableCorrelationAccumulator(
397-
inputDataTypes.get(0),
398-
inputDataTypes.get(1),
399-
CorrelationAccumulator.CorrelationType.CORR);
392+
return new TableCorrelationAccumulator(inputDataTypes.get(0), inputDataTypes.get(1));
400393
case COVAR_POP:
401394
return new TableCovarianceAccumulator(
402395
inputDataTypes.get(0),

‎iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/execution/operator/source/relational/aggregation/TableCentralMomentAccumulator.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ public void evaluateIntermediate(ColumnBuilder columnBuilder) {
193193

194194
@Override
195195
public void evaluateFinal(ColumnBuilder columnBuilder) {
196-
if (count == 0 || m2 == 0) {
196+
if (count == 0 || Math.abs(m2) < EPSILON) {
197197
columnBuilder.appendNull();
198198
return;
199199
}
@@ -203,17 +203,17 @@ public void evaluateFinal(ColumnBuilder columnBuilder) {
203203
columnBuilder.appendNull();
204204
return;
205205
}
206-
double result = Math.sqrt((double) count) * m3 / Math.pow(m2, 1.5);
206+
double n = count;
207+
double result = n * Math.sqrt(n - 1) * m3 / ((n - 2) * Math.pow(m2, 1.5));
207208
columnBuilder.writeDouble(result);
208209
} else {
209210
if (count < 4) {
210211
columnBuilder.appendNull();
211212
} else {
213+
double n = count;
212214
double variance = m2 / (count - 1);
213-
double term1 =
214-
(count * (count + 1) * m4)
215-
/ ((count - 1) * (count - 2) * (count - 3) * variance * variance);
216-
double term2 = (3 * Math.pow(count - 1, 2)) / ((count - 2) * (count - 3));
215+
double term1 = (n * (n + 1) * m4) / ((n - 1) * (n - 2) * (n - 3) * variance * variance);
216+
double term2 = (3 * (n - 1) * (n - 1)) / ((n - 2) * (n - 3));
217217
columnBuilder.writeDouble(term1 - term2);
218218
}
219219
}

‎iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/execution/operator/source/relational/aggregation/TableCorrelationAccumulator.java‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414

1515
package org.apache.iotdb.calc.execution.operator.source.relational.aggregation;
1616

17-
import org.apache.iotdb.calc.execution.aggregation.CorrelationAccumulator;
18-
1917
import org.apache.tsfile.block.column.Column;
2018
import org.apache.tsfile.block.column.ColumnBuilder;
2119
import org.apache.tsfile.enums.TSDataType;
@@ -39,7 +37,6 @@ public class TableCorrelationAccumulator implements TableAccumulator {
3937
private static final double EPSILON = 1e-12;
4038
private final TSDataType xDataType;
4139
private final TSDataType yDataType;
42-
private final CorrelationAccumulator.CorrelationType correlationType;
4340

4441
private long count;
4542
private double meanX;
@@ -48,13 +45,9 @@ public class TableCorrelationAccumulator implements TableAccumulator {
4845
private double m2Y;
4946
private double c2;
5047

51-
public TableCorrelationAccumulator(
52-
TSDataType xDataType,
53-
TSDataType yDataType,
54-
CorrelationAccumulator.CorrelationType correlationType) {
48+
public TableCorrelationAccumulator(TSDataType xDataType, TSDataType yDataType) {
5549
this.xDataType = xDataType;
5650
this.yDataType = yDataType;
57-
this.correlationType = correlationType;
5851
}
5952

6053
@Override
@@ -64,7 +57,7 @@ public long getEstimatedSize() {
6457

6558
@Override
6659
public TableAccumulator copy() {
67-
return new TableCorrelationAccumulator(xDataType, yDataType, correlationType);
60+
return new TableCorrelationAccumulator(xDataType, yDataType);
6861
}
6962

7063
@Override
@@ -249,10 +242,6 @@ public void evaluateIntermediate(ColumnBuilder columnBuilder) {
249242

250243
@Override
251244
public void evaluateFinal(ColumnBuilder columnBuilder) {
252-
if (correlationType != CorrelationAccumulator.CorrelationType.CORR) {
253-
throw new UnsupportedOperationException("Unknown type: " + correlationType);
254-
}
255-
256245
double result = c2 / Math.sqrt(m2X * m2Y);
257246
if (Double.isFinite(result)) {
258247
columnBuilder.writeDouble(result);

‎iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/execution/operator/source/relational/aggregation/grouped/GroupedCentralMomentAccumulator.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public class GroupedCentralMomentAccumulator implements GroupedAccumulator {
4343
private static final long INSTANCE_SIZE =
4444
RamUsageEstimator.shallowSizeOfInstance(GroupedCentralMomentAccumulator.class);
4545
private static final int INTERMEDIATE_SIZE = Long.BYTES + 4 * Double.BYTES;
46+
private static final double EPSILON = 1e-12;
4647

4748
private final TSDataType seriesDataType;
4849
private final CentralMomentAccumulator.MomentType momentType;
@@ -227,7 +228,7 @@ public void evaluateFinal(int groupId, ColumnBuilder columnBuilder) {
227228
long count = counts.get(groupId);
228229
double m2 = m2s.get(groupId);
229230

230-
if (count == 0 || m2 == 0) {
231+
if (count == 0 || Math.abs(m2) < EPSILON) {
231232
columnBuilder.appendNull();
232233
return;
233234
}
@@ -238,18 +239,18 @@ public void evaluateFinal(int groupId, ColumnBuilder columnBuilder) {
238239
return;
239240
}
240241
double m3 = m3s.get(groupId);
241-
double result = Math.sqrt((double) count) * m3 / Math.pow(m2, 1.5);
242+
double n = count;
243+
double result = n * Math.sqrt(n - 1) * m3 / ((n - 2) * Math.pow(m2, 1.5));
242244
columnBuilder.writeDouble(result);
243245
} else {
244246
if (count < 4) {
245247
columnBuilder.appendNull();
246248
} else {
249+
double n = count;
247250
double m4 = m4s.get(groupId);
248251
double variance = m2 / (count - 1);
249-
double term1 =
250-
(count * (count + 1) * m4)
251-
/ ((count - 1) * (count - 2) * (count - 3) * variance * variance);
252-
double term2 = (3 * Math.pow(count - 1, 2)) / ((count - 2) * (count - 3));
252+
double term1 = (n * (n + 1) * m4) / ((n - 1) * (n - 2) * (n - 3) * variance * variance);
253+
double term2 = (3 * (n - 1) * (n - 1)) / ((n - 2) * (n - 3));
253254
columnBuilder.writeDouble(term1 - term2);
254255
}
255256
}

‎iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/execution/operator/source/relational/aggregation/grouped/GroupedCorrelationAccumulator.java‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919

2020
package org.apache.iotdb.calc.execution.operator.source.relational.aggregation.grouped;
2121

22-
import org.apache.iotdb.calc.execution.aggregation.CorrelationAccumulator;
2322
import org.apache.iotdb.calc.execution.operator.source.relational.aggregation.AggregationMask;
2423
import org.apache.iotdb.calc.execution.operator.source.relational.aggregation.grouped.array.DoubleBigArray;
2524
import org.apache.iotdb.calc.execution.operator.source.relational.aggregation.grouped.array.LongBigArray;
@@ -45,7 +44,6 @@ public class GroupedCorrelationAccumulator implements GroupedAccumulator {
4544
private static final int INTERMEDIATE_SIZE = Long.BYTES + 5 * Double.BYTES;
4645
private final TSDataType xDataType;
4746
private final TSDataType yDataType;
48-
private final CorrelationAccumulator.CorrelationType correlationType;
4947

5048
private final LongBigArray counts = new LongBigArray();
5149
private final DoubleBigArray meanXs = new DoubleBigArray();
@@ -54,13 +52,9 @@ public class GroupedCorrelationAccumulator implements GroupedAccumulator {
5452
private final DoubleBigArray m2Ys = new DoubleBigArray();
5553
private final DoubleBigArray c2s = new DoubleBigArray();
5654

57-
public GroupedCorrelationAccumulator(
58-
TSDataType xDataType,
59-
TSDataType yDataType,
60-
CorrelationAccumulator.CorrelationType correlationType) {
55+
public GroupedCorrelationAccumulator(TSDataType xDataType, TSDataType yDataType) {
6156
this.xDataType = xDataType;
6257
this.yDataType = yDataType;
63-
this.correlationType = correlationType;
6458
}
6559

6660
@Override
@@ -229,10 +223,6 @@ public void evaluateIntermediate(int groupId, ColumnBuilder columnBuilder) {
229223

230224
@Override
231225
public void evaluateFinal(int groupId, ColumnBuilder columnBuilder) {
232-
if (correlationType != CorrelationAccumulator.CorrelationType.CORR) {
233-
throw new UnsupportedOperationException("Unknown type: " + correlationType);
234-
}
235-
236226
double result = c2s.get(groupId) / Math.sqrt(m2Xs.get(groupId) * m2Ys.get(groupId));
237227
if (Double.isFinite(result)) {
238228
columnBuilder.writeDouble(result);

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/aggregation/AccumulatorFactory.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,7 @@ public static Accumulator createBuiltinMultiInputAccumulator(
104104
case CORR:
105105
checkState(inputDataTypes.size() == 2, "Wrong inputDataTypes size.");
106106
return new CorrelationAccumulator(
107-
new TSDataType[] {inputDataTypes.get(0), inputDataTypes.get(1)},
108-
CorrelationAccumulator.CorrelationType.CORR);
107+
new TSDataType[] {inputDataTypes.get(0), inputDataTypes.get(1)});
109108
case COVAR_POP:
110109
checkState(inputDataTypes.size() == 2, "Wrong inputDataTypes size.");
111110
return new CovarianceAccumulator(

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/analyze/ExpressionTypeAnalyzer.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,6 @@ public TSDataType visitFunctionExpression(
361361
}
362362

363363
if (functionExpression.isBuiltInAggregationFunctionExpression()) {
364-
365364
return setExpressionType(
366365
functionExpression,
367366
TypeInferenceUtils.getBuiltinAggregationDataType(

0 commit comments

Comments
 (0)