Skip to content

Commit 20dd8ad

Browse files
KSDaemonKSDaemon
authored
fix(query-orchestrator): Correct local date parsing for partition start/end queries (#9543)
* code polish * Refactor a bit * Add tests for daily partitions * refactor parseLocalDate to use UTC during parsing and then input timezone for result shifting * tests * fix tests * fix test * another test fix
1 parent 00a589f commit 20dd8ad

File tree

8 files changed

+243
-87
lines changed

8 files changed

+243
-87
lines changed

‎packages/cubejs-api-gateway/src/gateway.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import structuredClone from '@ungap/structured-clone';
99
import {
1010
getEnv,
1111
getRealType,
12-
parseLocalDate,
12+
parseUtcIntoLocalDate,
1313
QueryAlias,
1414
} from '@cubejs-backend/shared';
1515
import {
@@ -921,8 +921,8 @@ class ApiGateway {
921921
// It's expected that selector.dateRange is provided in local time (without timezone)
922922
// At the same time it is ok to get timestamps with `Z` (in UTC).
923923
if (selector.dateRange) {
924-
const start = parseLocalDate([{ val: selector.dateRange[0] }], 'UTC');
925-
const end = parseLocalDate([{ val: selector.dateRange[1] }], 'UTC');
924+
const start = parseUtcIntoLocalDate([{ val: selector.dateRange[0] }], 'UTC');
925+
const end = parseUtcIntoLocalDate([{ val: selector.dateRange[1] }], 'UTC');
926926
if (!start || !end) {
927927
throw new UserError(`Cannot parse selector date range ${selector.dateRange}`);
928928
}

‎packages/cubejs-backend-shared/src/time.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ export const utcToLocalTimeZone = (timezone: string, timestampFormat: string, ti
251251
return moment.tz(timestamp, 'UTC').tz(timezone).format(timestampFormat);
252252
};
253253

254-
export const parseLocalDate = (data: { [key: string]: string }[] | null | undefined, timezone: string, timestampFormat: string = 'YYYY-MM-DDTHH:mm:ss.SSS'): string | null => {
254+
export const parseUtcIntoLocalDate = (data: { [key: string]: string }[] | null | undefined, timezone: string, timestampFormat: string = 'YYYY-MM-DDTHH:mm:ss.SSS'): string | null => {
255255
if (!data) {
256256
return null;
257257
}
@@ -278,11 +278,12 @@ export const parseLocalDate = (data: { [key: string]: string }[] | null | undefi
278278
let parsedMoment;
279279

280280
if (value.includes('Z') || /([+-]\d{2}:?\d{2})$/.test(value.trim())) {
281-
// We have timezone info
281+
// We have timezone info encoded in the value string
282282
parsedMoment = moment(value, formats, true);
283283
} else {
284-
// If no tz info - use provided timezone
285-
parsedMoment = moment.tz(value, formats, true, timezone);
284+
// If no tz info - use UTC as cube expects data source connection to be in UTC timezone
285+
// and so date functions (e.g. `now()`) would return timestamps in UTC.
286+
parsedMoment = moment.tz(value, formats, true, 'UTC');
286287
}
287288

288289
if (!parsedMoment.isValid()) {

‎packages/cubejs-backend-shared/test/time.test.ts‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {
44
timeSeries,
55
isPredefinedGranularity,
66
timeSeriesFromCustomInterval,
7-
parseLocalDate,
7+
parseUtcIntoLocalDate,
88
utcToLocalTimeZone,
99
addSecondsToLocalTimestamp,
1010
reformatInIsoLocal,
@@ -264,38 +264,38 @@ describe('extractDate', () => {
264264
const timezone = 'Europe/Kiev';
265265

266266
it('should return null if data is empty', () => {
267-
expect(parseLocalDate(null, timezone)).toBeNull();
268-
expect(parseLocalDate(undefined, timezone)).toBeNull();
269-
expect(parseLocalDate([], timezone)).toBeNull();
267+
expect(parseUtcIntoLocalDate(null, timezone)).toBeNull();
268+
expect(parseUtcIntoLocalDate(undefined, timezone)).toBeNull();
269+
expect(parseUtcIntoLocalDate([], timezone)).toBeNull();
270270
});
271271

272272
it('should return null if no valid date is found in data', () => {
273-
expect(parseLocalDate([{}], timezone)).toBeNull();
274-
expect(parseLocalDate([{ someKey: 'invalid date' }], timezone)).toBeNull();
273+
expect(parseUtcIntoLocalDate([{}], timezone)).toBeNull();
274+
expect(parseUtcIntoLocalDate([{ someKey: 'invalid date' }], timezone)).toBeNull();
275275
});
276276

277277
it('should throw an error for unknown timezone', () => {
278278
const input = [{ date: '2025-02-28T12:00:00+03:00' }];
279-
expect(() => parseLocalDate(input, 'Invalid/Timezone'))
279+
expect(() => parseUtcIntoLocalDate(input, 'Invalid/Timezone'))
280280
.toThrowError('Unknown timezone: Invalid/Timezone');
281281
});
282282

283283
it('should parse a date with UTC timezone', () => {
284284
const input = [{ date: '2025-02-28T12:00:00Z' }];
285-
const result = parseLocalDate(input, timezone);
285+
const result = parseUtcIntoLocalDate(input, timezone);
286286
expect(result).toBe('2025-02-28T14:00:00.000');
287287
});
288288

289289
it('should parse a date with an offset timezone', () => {
290290
const input = [{ date: '2025-02-28T12:00:00+03:00' }];
291-
const result = parseLocalDate(input, timezone);
291+
const result = parseUtcIntoLocalDate(input, timezone);
292292
expect(result).toBe('2025-02-28T11:00:00.000');
293293
});
294294

295295
it('should parse a date without timezone as UTC', () => {
296296
const input = [{ date: '2025-02-28 12:00:00' }];
297-
const result = parseLocalDate(input, timezone);
298-
expect(result).toBe('2025-02-28T12:00:00.000');
297+
const result = parseUtcIntoLocalDate(input, timezone);
298+
expect(result).toBe('2025-02-28T14:00:00.000');
299299
});
300300

301301
it('should handle multiple formats', () => {
@@ -304,10 +304,10 @@ describe('extractDate', () => {
304304
const input3 = [{ date: '2025-02-28T12:00:00Z' }];
305305
const input4 = [{ date: '2025-02-28T12:00:00+03:00' }];
306306

307-
expect(parseLocalDate(input1, timezone)).toBe('2025-02-28T12:00:00.000');
308-
expect(parseLocalDate(input2, timezone)).toBe('2025-02-28T12:00:00.000');
309-
expect(parseLocalDate(input3, timezone)).toBe('2025-02-28T14:00:00.000');
310-
expect(parseLocalDate(input4, timezone)).toBe('2025-02-28T11:00:00.000');
307+
expect(parseUtcIntoLocalDate(input1, timezone)).toBe('2025-02-28T14:00:00.000');
308+
expect(parseUtcIntoLocalDate(input2, timezone)).toBe('2025-02-28T14:00:00.000');
309+
expect(parseUtcIntoLocalDate(input3, timezone)).toBe('2025-02-28T14:00:00.000');
310+
expect(parseUtcIntoLocalDate(input4, timezone)).toBe('2025-02-28T11:00:00.000');
311311
});
312312
});
313313

‎packages/cubejs-query-orchestrator/src/orchestrator/PreAggregationPartitionRangeLoader.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
utcToLocalTimeZone,
1010
timeSeries,
1111
localTimestampToUtc,
12-
parseLocalDate,
12+
parseUtcIntoLocalDate,
1313
} from '@cubejs-backend/shared';
1414
import { InlineTable, TableStructure } from '@cubejs-backend/base-driver';
1515
import { DriverFactory } from './DriverFactory';
@@ -526,7 +526,7 @@ export class PreAggregationPartitionRangeLoader {
526526
}
527527

528528
public static extractDate(data: any, timezone: string, timestampFormat: string = DEFAULT_TS_FORMAT): string {
529-
return parseLocalDate(data, timezone, timestampFormat);
529+
return parseUtcIntoLocalDate(data, timezone, timestampFormat);
530530
}
531531

532532
public static readonly FROM_PARTITION_RANGE = FROM_PARTITION_RANGE;

‎packages/cubejs-query-orchestrator/src/orchestrator/PreAggregations.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -718,7 +718,7 @@ export class PreAggregations {
718718
public async getVersionEntries(preAggregations: PreAggregationDescription[], requestId): Promise<VersionEntry[][]> {
719719
const loadCacheByDataSource = {};
720720

721-
const getLoadCacheByDataSource = (dataSource = 'default', preAggregationSchema) => {
721+
const getLoadCacheByDataSource = (preAggregationSchema, dataSource = 'default') => {
722722
if (!loadCacheByDataSource[`${dataSource}_${preAggregationSchema}`]) {
723723
loadCacheByDataSource[`${dataSource}_${preAggregationSchema}`] =
724724
new PreAggregationLoadCache(
@@ -740,9 +740,9 @@ export class PreAggregations {
740740
preAggregations.map(
741741
async preAggregation => {
742742
const { dataSource, preAggregationsSchema } = preAggregation;
743-
const cacheKey = getLoadCacheByDataSource(dataSource, preAggregationsSchema).tablesCachePrefixKey(preAggregation);
743+
const cacheKey = getLoadCacheByDataSource(preAggregationsSchema, dataSource).tablesCachePrefixKey(preAggregation);
744744
if (!firstByCacheKey[cacheKey]) {
745-
firstByCacheKey[cacheKey] = getLoadCacheByDataSource(dataSource, preAggregationsSchema).getVersionEntries(preAggregation);
745+
firstByCacheKey[cacheKey] = getLoadCacheByDataSource(preAggregationsSchema, dataSource).getVersionEntries(preAggregation);
746746
const res = await firstByCacheKey[cacheKey];
747747
return res.versionEntries;
748748
}

0 commit comments

Comments
 (0)