Skip to content

Commit deb62b6

Browse files
paveltiunovpaveltiunov
committed
fix: Strict date range and rollup granularity alignment check
Fixes #103
1 parent b57e5db commit deb62b6

File tree

5 files changed

+141
-7
lines changed

5 files changed

+141
-7
lines changed

‎packages/cubejs-schema-compiler/adapter/BaseFilter.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ class BaseFilter extends BaseDimension {
245245
return `${date}T23:59:59.999`;
246246
}
247247
if (!date) {
248-
return moment.tz(date, this.query.timezone).format('YYYY-MM-DD 23:59:59');
248+
return moment.tz(date, this.query.timezone).format('YYYY-MM-DDT23:59:59.999');
249249
}
250250
return moment.tz(date, this.query.timezone).format(moment.HTML5_FMT.DATETIME_LOCAL_MS);
251251
}

‎packages/cubejs-schema-compiler/adapter/BaseQuery.js

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ class BaseQuery {
123123
this.initUngrouped();
124124
}
125125

126-
cacheValue(key, fn, { contextPropNames, inputProps, cache }) {
126+
cacheValue(key, fn, { contextPropNames, inputProps, cache } = {}) {
127127
const currentContext = this.safeEvaluateSymbolContext();
128128
if (contextPropNames) {
129129
const contextKey = {};
@@ -540,6 +540,9 @@ class BaseQuery {
540540
if (!granularityB) {
541541
return granularityA;
542542
}
543+
if (granularityA === granularityB) {
544+
return granularityA;
545+
}
543546
const aHierarchy = R.reverse(this.granularityParentHierarchy(granularityA));
544547
const bHierarchy = R.reverse(this.granularityParentHierarchy(granularityB));
545548
let lastIndex = Math.max(
@@ -1707,6 +1710,60 @@ class BaseQuery {
17071710
return this.floorSql(`${this.unixTimestampSql()} / ${this.parseSecondDuration(interval)}`);
17081711
}
17091712

1713+
granularityFor(momentDate) {
1714+
const obj = momentDate.toObject();
1715+
const weekDay = momentDate.isoWeekday();
1716+
if (
1717+
obj.months === 0 &&
1718+
obj.date === 1 &&
1719+
obj.hours === 0 &&
1720+
obj.minutes === 0 &&
1721+
obj.seconds === 0 &&
1722+
obj.milliseconds === 0
1723+
) {
1724+
return 'year';
1725+
} else if (
1726+
obj.date === 1 &&
1727+
obj.hours === 0 &&
1728+
obj.minutes === 0 &&
1729+
obj.seconds === 0 &&
1730+
obj.milliseconds === 0
1731+
) {
1732+
return 'month';
1733+
} else if (
1734+
weekDay === 1 &&
1735+
obj.hours === 0 &&
1736+
obj.minutes === 0 &&
1737+
obj.seconds === 0 &&
1738+
obj.milliseconds === 0
1739+
) {
1740+
return 'week';
1741+
} else if (
1742+
obj.hours === 0 &&
1743+
obj.minutes === 0 &&
1744+
obj.seconds === 0 &&
1745+
obj.milliseconds === 0
1746+
) {
1747+
return 'day';
1748+
} else if (
1749+
obj.minutes === 0 &&
1750+
obj.seconds === 0 &&
1751+
obj.milliseconds === 0
1752+
) {
1753+
return 'hour';
1754+
} else if (
1755+
obj.seconds === 0 &&
1756+
obj.milliseconds === 0
1757+
) {
1758+
return 'minute';
1759+
} else if (
1760+
obj.milliseconds === 0
1761+
) {
1762+
return 'second';
1763+
}
1764+
return 'second'; // TODO return 'millisecond';
1765+
}
1766+
17101767
parseSecondDuration(interval) {
17111768
const intervalMatch = interval.match(/^(\d+) (second|minute|hour|day|week)s?$/);
17121769
if (!intervalMatch) {

‎packages/cubejs-schema-compiler/adapter/BaseTimeDimension.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,29 @@ class BaseTimeDimension extends BaseFilter {
138138
);
139139
}
140140

141+
dateRangeGranularity() {
142+
if (!this.dateRange) {
143+
return null;
144+
}
145+
const msFrom = moment.tz(this.dateFromFormatted(), this.query.timezone);
146+
const msTo = moment.tz(this.dateToFormatted(), this.query.timezone).add(1, 'ms');
147+
return this.query.minGranularity(
148+
this.query.granularityFor(msFrom),
149+
this.query.granularityFor(msTo),
150+
);
151+
}
152+
153+
rollupGranularity() {
154+
if (!this.rollupGranularityValue) {
155+
this.rollupGranularityValue =
156+
this.query.cacheValue(
157+
['rollupGranularity', this.granularity].concat(this.dateRange),
158+
() => this.query.minGranularity(this.granularity, this.dateRangeGranularity())
159+
);
160+
}
161+
return this.rollupGranularityValue;
162+
}
163+
141164
timeSeries() {
142165
if (!this.dateRange) {
143166
throw new UserError(`Time series queries without dateRange aren't supported`);

‎packages/cubejs-schema-compiler/adapter/PreAggregations.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ class PreAggregations {
176176
function sortTimeDimensions(timeDimensions) {
177177
return timeDimensions && R.sortBy(
178178
R.prop(0),
179-
timeDimensions.map(d => [d.dimension, d.granularity || 'day'])
179+
timeDimensions.map(d => [d.dimension, d.rollupGranularity()])
180180
) || [];
181181
}
182182

@@ -245,7 +245,7 @@ class PreAggregations {
245245
function sortTimeDimensions(timeDimensions) {
246246
return timeDimensions && R.sortBy(
247247
d => d.join('.'),
248-
timeDimensions.map(d => [d.dimension, d.granularity || 'day'])
248+
timeDimensions.map(d => [d.dimension, d.granularity || 'day']) // TODO granularity shouldn't be null?
249249
) || [];
250250
}
251251
// TimeDimension :: [Dimension, Granularity]
@@ -517,6 +517,7 @@ class PreAggregations {
517517
preAggregationForQuery.preAggregation.measures :
518518
this.evaluateAllReferences(preAggregationForQuery.cube, preAggregationForQuery.preAggregation).measures);
519519

520+
// TODO granularity shouldn't be null?
520521
const rollupGranularity = this.castGranularity(preAggregationForQuery.preAggregation.granularity) || 'day';
521522

522523
return this.query.evaluateSymbolSqlWithContext(

‎packages/cubejs-schema-compiler/test/PreAggregationsTest.js

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,7 @@ describe('PreAggregations', function test() {
667667
timeDimensions: [{
668668
dimension: 'visitors.createdAt',
669669
granularity: 'month',
670-
dateRange: ['2017-01-01', '2017-01-30']
670+
dateRange: ['2017-01-01', '2017-01-31']
671671
}],
672672
order: [{
673673
id: 'visitors.createdAt'
@@ -814,6 +814,59 @@ describe('PreAggregations', function test() {
814814
});
815815
});
816816

817+
it('not aligned time dimension', () => {
818+
return compiler.compile().then(() => {
819+
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
820+
measures: [
821+
'visitors.checkinsTotal'
822+
],
823+
dimensions: [
824+
'visitors.source'
825+
],
826+
timezone: 'UTC',
827+
preAggregationsSchema: '',
828+
timeDimensions: [{
829+
dimension: 'visitors.createdAt',
830+
granularity: 'hour',
831+
dateRange: ['2017-01-02T00:00:00.000', '2017-01-05T00:15:59.999']
832+
}],
833+
order: [{
834+
id: 'visitors.createdAt'
835+
}],
836+
});
837+
838+
const queryAndParams = query.buildSqlAndParams();
839+
console.log(queryAndParams);
840+
const preAggregationsDescription = query.preAggregations.preAggregationsDescription();
841+
console.log(preAggregationsDescription);
842+
preAggregationsDescription.length.should.be.equal(2);
843+
844+
const queries = tempTablePreAggregations(preAggregationsDescription);
845+
846+
console.log(JSON.stringify(queries.concat(queryAndParams)));
847+
848+
return dbRunner.testQueries(
849+
queries.concat([queryAndParams]).map(q => replaceTableName(q, preAggregationsDescription, 342))
850+
).then(res => {
851+
console.log(JSON.stringify(res));
852+
res.should.be.deepEqual(
853+
[
854+
{
855+
"visitors__source": "some",
856+
"visitors__created_at_hour": "2017-01-03T00:00:00.000Z",
857+
"visitors__checkins_total": "3"
858+
},
859+
{
860+
"visitors__source": "some",
861+
"visitors__created_at_hour": "2017-01-05T00:00:00.000Z",
862+
"visitors__checkins_total": "2"
863+
}
864+
]
865+
);
866+
});
867+
});
868+
});
869+
817870
it('segment', () => {
818871
return compiler.compile().then(() => {
819872
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
@@ -827,7 +880,7 @@ describe('PreAggregations', function test() {
827880
timeDimensions: [{
828881
dimension: 'visitors.createdAt',
829882
granularity: 'week',
830-
dateRange: ['2016-12-30', '2017-01-05']
883+
dateRange: ['2016-12-26', '2017-01-08']
831884
}],
832885
order: [{
833886
id: 'visitors.createdAt'
@@ -1041,7 +1094,7 @@ describe('PreAggregations in time hierarchy', function test() {
10411094
timeDimensions: [{
10421095
dimension: 'visitors.createdAt',
10431096
granularity: 'year',
1044-
dateRange: ['2016-12-30', '2018-12-30']
1097+
dateRange: ['2016-12-01', '2018-12-31']
10451098
}],
10461099
preAggregationsSchema: '',
10471100
order: [],

0 commit comments

Comments
 (0)