Adjust granularity automatically only if necessary

[adrianmroz]

Right now when changing anything in time filter, granularity of time split is adjusted by RulesEvaluator and often it changes granularity unnecessarily.

Goal should be to:

1. If user changed granularity we should keep it as long as possible.
2. We should change it if it doesn't make sense
3. We should change it for performance reasons
4. We should change it if data granularity is bigger than granularity setting

At first, we need to check what's possible with current implementation of RulesEvaluator.

[adrianmroz]

Granularity is decided there:

turnilo/src/common/models/splits/splits.ts

Lines 123 to 153 in 08d1a16

	public updateWithFilter(filter: Filter, dimensions: Dimensions): Splits {
	const specificFilter = filter.getSpecificFilter(Timekeeper.globalNow(), Timekeeper.globalNow(), Timezone.UTC);
	return this.updateSplits(splits => splits.map(split => {
	const { bucket, reference } = split;
	if (bucket) return split;
	const splitDimension = dimensions.getDimensionByName(reference);
	const splitKind = splitDimension.kind;
	if (!splitDimension || !(splitKind === "time" || splitKind === "number") || !splitDimension.canBucketByDefault()) {
	return split;
	}
	if (splitKind === "time") {
	const clause = specificFilter.clauses.find(clause => clause instanceof FixedTimeFilterClause) as FixedTimeFilterClause;
	return split.changeBucket(clause
	? getBestBucketUnitForRange(clause.values.first(), false, splitDimension.bucketedBy, splitDimension.granularities)
	: getDefaultGranularityForKind("time", splitDimension.bucketedBy, splitDimension.granularities)
	);
	} else if (splitKind === "number") {
	const clause = specificFilter.clauses.find(clause => clause instanceof NumberFilterClause) as NumberFilterClause;
	return split.changeBucket(clause
	? getBestBucketUnitForRange(clause.values.first(), false, splitDimension.bucketedBy, splitDimension.granularities)
	: getDefaultGranularityForKind("number", splitDimension.bucketedBy, splitDimension.granularities)
	);
	}
	throw new Error("unknown extent type");
	}));
	}

This method is called whenever splits change or filters change. We should limit that.

[adrianmroz]

Hmm, looks like we have a bug there - we search clause by type, we should search by split reference and type.

turnilo/src/common/models/splits/splits.ts

Line 136 in 08d1a16

const clause = specificFilter.clauses.find(clause => clause instanceof FixedTimeFilterClause) as FixedTimeFilterClause;

[adrianmroz]

Observations:

• We don't use data values when finding best bucketing. That probably implies that bucketing should be changed only when changes filter on the same dimension.

• We don't have information about previous bucketing source. Was it calculated by turnilo or selected by user? Keeping such information wouldn't be trivial.

• Right now after changing filter, all splits that refers to changed clause have their bucketing set to null. Because of that, later in updateWithFilter we don't have previous bucketing and we couldn't compare it to newfound.

• Interesting tidbit: when converting to "specific" ("non relative") filter, we also reset bucketing. That's happening when sharing fixed url.

[adrianmroz]

Probably linked with #598