Commit d918d1f5 by Ryan McKinley Committed by Torkel Ödegaard

Reducers: consistent result for first/last reducer shortcut (#17911)

* find the last non-null value

* also fix first

* check empty first

* remove unused import

* adding lastNotNull
parent 33292095
......@@ -91,4 +91,58 @@ describe('Stats Calculators', () => {
expect(stats.step).toEqual(100);
expect(stats.delta).toEqual(300);
});
it('consistent results for first/last value with null', () => {
const info = [
{
rows: [[null], [200], [null]], // first/last value is null
result: 200,
},
{
rows: [[null], [null], [null]], // All null
result: undefined,
},
{
rows: [], // Empty row
result: undefined,
},
];
const fields = [{ name: 'A' }];
const stats = reduceField({
series: { rows: info[0].rows, fields },
fieldIndex: 0,
reducers: [ReducerID.first, ReducerID.last, ReducerID.firstNotNull, ReducerID.lastNotNull], // uses standard path
});
expect(stats[ReducerID.first]).toEqual(null);
expect(stats[ReducerID.last]).toEqual(null);
expect(stats[ReducerID.firstNotNull]).toEqual(200);
expect(stats[ReducerID.lastNotNull]).toEqual(200);
const reducers = [ReducerID.lastNotNull, ReducerID.firstNotNull];
for (const input of info) {
for (const reducer of reducers) {
const v1 = reduceField({
series: { rows: input.rows, fields },
fieldIndex: 0,
reducers: [reducer, ReducerID.mean], // uses standard path
})[reducer];
const v2 = reduceField({
series: { rows: input.rows, fields },
fieldIndex: 0,
reducers: [reducer], // uses optimized path
})[reducer];
if (v1 !== v2 || v1 !== input.result) {
const msg =
`Invalid ${reducer} result for: ` +
input.rows.join(', ') +
` Expected: ${input.result}` + // configured
` Recieved: Multiple: ${v1}, Single: ${v2}`;
expect(msg).toEqual(null);
}
}
}
});
});
......@@ -17,6 +17,9 @@ export enum ReducerID {
delta = 'delta',
step = 'step',
firstNotNull = 'firstNotNull',
lastNotNull = 'lastNotNull',
changeCount = 'changeCount',
distinctCount = 'distinctCount',
......@@ -132,14 +135,28 @@ function getById(id: string): FieldReducerInfo | undefined {
if (!hasBuiltIndex) {
[
{
id: ReducerID.lastNotNull,
name: 'Last (not null)',
description: 'Last non-null value',
standard: true,
alias: 'current',
reduce: calculateLastNotNull,
},
{
id: ReducerID.last,
name: 'Last',
description: 'Last Value (current)',
description: 'Last Value',
standard: true,
alias: 'current',
reduce: calculateLast,
},
{ id: ReducerID.first, name: 'First', description: 'First Value', standard: true, reduce: calculateFirst },
{
id: ReducerID.firstNotNull,
name: 'First (not null)',
description: 'First non-null value',
standard: true,
reduce: calculateFirstNotNull,
},
{ id: ReducerID.min, name: 'Min', description: 'Minimum Value', standard: true },
{ id: ReducerID.max, name: 'Max', description: 'Maximum Value', standard: true },
{ id: ReducerID.mean, name: 'Mean', description: 'Average Value', standard: true, alias: 'avg' },
......@@ -231,6 +248,8 @@ function doStandardCalcs(data: DataFrame, fieldIndex: number, ignoreNulls: boole
mean: null,
last: null,
first: null,
lastNotNull: undefined,
firstNotNull: undefined,
count: 0,
nonNullCount: 0,
allIsNull: true,
......@@ -246,6 +265,10 @@ function doStandardCalcs(data: DataFrame, fieldIndex: number, ignoreNulls: boole
for (let i = 0; i < data.rows.length; i++) {
let currentValue = data.rows[i][fieldIndex];
if (i === 0) {
calcs.first = currentValue;
}
calcs.last = currentValue;
if (currentValue === null) {
if (ignoreNulls) {
......@@ -257,9 +280,9 @@ function doStandardCalcs(data: DataFrame, fieldIndex: number, ignoreNulls: boole
}
if (currentValue !== null) {
const isFirst = calcs.first === null;
const isFirst = calcs.firstNotNull === undefined;
if (isFirst) {
calcs.first = currentValue;
calcs.firstNotNull = currentValue;
}
if (isNumber(currentValue)) {
......@@ -268,12 +291,12 @@ function doStandardCalcs(data: DataFrame, fieldIndex: number, ignoreNulls: boole
calcs.nonNullCount++;
if (!isFirst) {
const step = currentValue - calcs.last!;
const step = currentValue - calcs.lastNotNull!;
if (calcs.step > step) {
calcs.step = step; // the minimum interval
}
if (calcs.last! > currentValue) {
if (calcs.lastNotNull! > currentValue) {
// counter reset
calcs.previousDeltaUp = false;
if (i === data.rows.length - 1) {
......@@ -307,7 +330,7 @@ function doStandardCalcs(data: DataFrame, fieldIndex: number, ignoreNulls: boole
calcs.allIsZero = false;
}
calcs.last = currentValue;
calcs.lastNotNull = currentValue;
}
}
......@@ -331,10 +354,8 @@ function doStandardCalcs(data: DataFrame, fieldIndex: number, ignoreNulls: boole
calcs.range = calcs.max - calcs.min;
}
if (calcs.first !== null && calcs.last !== null) {
if (isNumber(calcs.first) && isNumber(calcs.last)) {
calcs.diff = calcs.last - calcs.first;
}
if (isNumber(calcs.firstNotNull) && isNumber(calcs.lastNotNull)) {
calcs.diff = calcs.lastNotNull - calcs.firstNotNull;
}
return calcs;
......@@ -344,10 +365,41 @@ function calculateFirst(data: DataFrame, fieldIndex: number, ignoreNulls: boolea
return { first: data.rows[0][fieldIndex] };
}
function calculateFirstNotNull(
data: DataFrame,
fieldIndex: number,
ignoreNulls: boolean,
nullAsZero: boolean
): FieldCalcs {
for (let idx = 0; idx < data.rows.length; idx++) {
const v = data.rows[idx][fieldIndex];
if (v != null) {
return { firstNotNull: v };
}
}
return { firstNotNull: undefined };
}
function calculateLast(data: DataFrame, fieldIndex: number, ignoreNulls: boolean, nullAsZero: boolean): FieldCalcs {
return { last: data.rows[data.rows.length - 1][fieldIndex] };
}
function calculateLastNotNull(
data: DataFrame,
fieldIndex: number,
ignoreNulls: boolean,
nullAsZero: boolean
): FieldCalcs {
let idx = data.rows.length - 1;
while (idx >= 0) {
const v = data.rows[idx--][fieldIndex];
if (v != null) {
return { lastNotNull: v };
}
}
return { lastNotNull: undefined };
}
function calculateChangeCount(
data: DataFrame,
fieldIndex: number,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment