Skip to content

Commit 6d82f18

Browse files
sumeetgajjarKSDaemonRusovDmitriyigorlukaninMazterQyou
authored
fix(schema-compiler): fix FILTER_PARAMS to populate set and notSet filters in cube's SQL query (#8937)
* fix(schema-compiler): fix FILTER_PARAMS to populate set and notSet filters in cube's SQL query * Fix {DATE_OPERATORS}Where methods to return '1 = 1' when the required parameters are unavailable * fix(ksql-driver): Kafka, list of brokers (#9009) * docs: A few notes * chore(cubesql): Do not call async Node functions while planning (#8793) * Add postgress integration tests --------- Co-authored-by: Konstantin Burkalev <[email protected]> Co-authored-by: Dmitriy Rusov <[email protected]> Co-authored-by: Igor Lukanin <[email protected]> Co-authored-by: Alex Qyoun-ae <[email protected]>
1 parent 8f45afa commit 6d82f18

File tree

4 files changed

+291
-16
lines changed

4 files changed

+291
-16
lines changed

packages/cubejs-schema-compiler/src/adapter/BaseFilter.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ const dateTimeLocalURegex = /^\d\d\d\d-\d\d-\d\dT\d\d:\d\d:\d\d\.\d\d\d\d\d\d$/;
1212
const dateRegex = /^\d\d\d\d-\d\d-\d\d$/;
1313

1414
export class BaseFilter extends BaseDimension {
15+
public static readonly ALWAYS_TRUE: string = '1 = 1';
16+
1517
public readonly measure: any;
1618

1719
public readonly operator: any;
@@ -323,36 +325,57 @@ export class BaseFilter extends BaseDimension {
323325

324326
public inDateRangeWhere(column) {
325327
const [from, to] = this.allocateTimestampParams();
328+
if (!from || !to) {
329+
return BaseFilter.ALWAYS_TRUE;
330+
}
326331
return this.query.timeRangeFilter(column, from, to);
327332
}
328333

329334
public notInDateRangeWhere(column) {
330335
const [from, to] = this.allocateTimestampParams();
336+
if (!from || !to) {
337+
return BaseFilter.ALWAYS_TRUE;
338+
}
331339
return this.query.timeNotInRangeFilter(column, from, to);
332340
}
333341

334342
public onTheDateWhere(column) {
335343
const [from, to] = this.allocateTimestampParams();
344+
if (!from || !to) {
345+
return BaseFilter.ALWAYS_TRUE;
346+
}
336347
return this.query.timeRangeFilter(column, from, to);
337348
}
338349

339350
public beforeDateWhere(column) {
340351
const [before] = this.allocateTimestampParams();
352+
if (!before) {
353+
return BaseFilter.ALWAYS_TRUE;
354+
}
341355
return this.query.beforeDateFilter(column, before);
342356
}
343357

344358
public beforeOrOnDateWhere(column) {
345359
const [before] = this.allocateTimestampParams();
360+
if (!before) {
361+
return BaseFilter.ALWAYS_TRUE;
362+
}
346363
return this.query.beforeOrOnDateFilter(column, before);
347364
}
348365

349366
public afterDateWhere(column) {
350367
const [after] = this.allocateTimestampParams();
368+
if (!after) {
369+
return BaseFilter.ALWAYS_TRUE;
370+
}
351371
return this.query.afterDateFilter(column, after);
352372
}
353373

354374
public afterOrOnDateWhere(column) {
355375
const [after] = this.allocateTimestampParams();
376+
if (!after) {
377+
return BaseFilter.ALWAYS_TRUE;
378+
}
356379
return this.query.afterOrOnDateFilter(column, after);
357380
}
358381

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

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3805,7 +3805,7 @@ export class BaseQuery {
38053805

38063806
static renderFilterParams(filter, filterParamArgs, allocateParam, newGroupFilter, aliases) {
38073807
if (!filter) {
3808-
return '1 = 1';
3808+
return BaseFilter.ALWAYS_TRUE;
38093809
}
38103810

38113811
if (filter.operator === 'and' || filter.operator === 'or') {
@@ -3830,21 +3830,20 @@ export class BaseQuery {
38303830
if (!filterParamArg) {
38313831
throw new Error(`FILTER_PARAMS arg not found for ${filter.measure || filter.dimension}`);
38323832
}
3833-
if (
3834-
filterParams && filterParams.length
3835-
) {
3836-
if (typeof filterParamArg.__column() === 'function') {
3837-
// eslint-disable-next-line prefer-spread
3838-
return filterParamArg.__column().apply(
3839-
null,
3840-
filterParams.map(allocateParam),
3841-
);
3842-
} else {
3843-
return filter.conditionSql(filterParamArg.__column());
3844-
}
3845-
} else {
3846-
return '1 = 1';
3833+
3834+
if (typeof filterParamArg.__column() !== 'function') {
3835+
return filter.conditionSql(filterParamArg.__column());
3836+
}
3837+
3838+
if (!filterParams || !filterParams.length) {
3839+
return BaseFilter.ALWAYS_TRUE;
38473840
}
3841+
3842+
// eslint-disable-next-line prefer-spread
3843+
return filterParamArg.__column().apply(
3844+
null,
3845+
filterParams.map(allocateParam),
3846+
);
38483847
}
38493848

38503849
filterGroupFunction() {

packages/cubejs-schema-compiler/test/integration/postgres/sql-generation.test.ts

Lines changed: 176 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ describe('SQL Generation', () => {
407407
408408
cube('visitor_checkins_sources', {
409409
sql: \`
410-
select id, source from visitor_checkins WHERE \${FILTER_PARAMS.visitor_checkins_sources.source.filter('source')}
410+
select id, visitor_id, source from visitor_checkins WHERE \${FILTER_PARAMS.visitor_checkins_sources.source.filter('source')}
411411
\`,
412412
413413
rewriteQueries: true,
@@ -419,12 +419,22 @@ describe('SQL Generation', () => {
419419
}
420420
},
421421
422+
measures: {
423+
count: {
424+
type: 'count'
425+
}
426+
},
427+
422428
dimensions: {
423429
id: {
424430
type: 'number',
425431
sql: 'id',
426432
primaryKey: true
427433
},
434+
visitor_id: {
435+
type: 'number',
436+
sql: 'visitor_id'
437+
},
428438
source: {
429439
type: 'string',
430440
sql: 'source'
@@ -1897,6 +1907,171 @@ describe('SQL Generation', () => {
18971907
])
18981908
);
18991909

1910+
it(
1911+
'equals NULL filter',
1912+
() => runQueryTest({
1913+
measures: [
1914+
'visitor_checkins_sources.count'
1915+
],
1916+
dimensions: [
1917+
'visitor_checkins_sources.visitor_id'
1918+
],
1919+
timeDimensions: [],
1920+
timezone: 'America/Los_Angeles',
1921+
filters: [{
1922+
dimension: 'visitor_checkins_sources.source',
1923+
operator: 'equals',
1924+
values: [null]
1925+
}],
1926+
order: [{
1927+
id: 'visitor_checkins_sources.visitor_id'
1928+
}]
1929+
}, [
1930+
{
1931+
visitor_checkins_sources__visitor_id: 1,
1932+
visitor_checkins_sources__count: '2'
1933+
},
1934+
{
1935+
visitor_checkins_sources__visitor_id: 2,
1936+
visitor_checkins_sources__count: '2'
1937+
},
1938+
{
1939+
visitor_checkins_sources__visitor_id: 3,
1940+
visitor_checkins_sources__count: '1'
1941+
}
1942+
])
1943+
);
1944+
1945+
it(
1946+
'notSet(IS NULL) filter',
1947+
() => runQueryTest({
1948+
measures: [
1949+
'visitor_checkins_sources.count'
1950+
],
1951+
dimensions: [
1952+
'visitor_checkins_sources.visitor_id'
1953+
],
1954+
timeDimensions: [],
1955+
timezone: 'America/Los_Angeles',
1956+
filters: [{
1957+
dimension: 'visitor_checkins_sources.source',
1958+
operator: 'notSet',
1959+
}],
1960+
order: [{
1961+
id: 'visitor_checkins_sources.visitor_id'
1962+
}]
1963+
}, [
1964+
{
1965+
visitor_checkins_sources__visitor_id: 1,
1966+
visitor_checkins_sources__count: '2'
1967+
},
1968+
{
1969+
visitor_checkins_sources__visitor_id: 2,
1970+
visitor_checkins_sources__count: '2'
1971+
},
1972+
{
1973+
visitor_checkins_sources__visitor_id: 3,
1974+
visitor_checkins_sources__count: '1'
1975+
}
1976+
])
1977+
);
1978+
1979+
it(
1980+
'notEquals NULL filter',
1981+
() => runQueryTest({
1982+
measures: [
1983+
'visitor_checkins_sources.count'
1984+
],
1985+
dimensions: [
1986+
'visitor_checkins_sources.visitor_id'
1987+
],
1988+
timeDimensions: [],
1989+
timezone: 'America/Los_Angeles',
1990+
filters: [{
1991+
dimension: 'visitor_checkins_sources.source',
1992+
operator: 'notEquals',
1993+
values: [null]
1994+
}],
1995+
order: [{
1996+
id: 'visitor_checkins_sources.visitor_id'
1997+
}]
1998+
}, [
1999+
{
2000+
visitor_checkins_sources__visitor_id: 1,
2001+
visitor_checkins_sources__count: '1'
2002+
}
2003+
])
2004+
);
2005+
2006+
it(
2007+
'set(IS NOT NULL) filter',
2008+
() => runQueryTest({
2009+
measures: [
2010+
'visitor_checkins_sources.count'
2011+
],
2012+
dimensions: [
2013+
'visitor_checkins_sources.visitor_id'
2014+
],
2015+
timeDimensions: [],
2016+
timezone: 'America/Los_Angeles',
2017+
filters: [{
2018+
dimension: 'visitor_checkins_sources.source',
2019+
operator: 'set',
2020+
}],
2021+
order: [{
2022+
id: 'visitor_checkins_sources.visitor_id'
2023+
}]
2024+
}, [
2025+
{
2026+
visitor_checkins_sources__visitor_id: 1,
2027+
visitor_checkins_sources__count: '1'
2028+
}
2029+
])
2030+
);
2031+
2032+
it(
2033+
'source is notSet(IS NULL) "or" source is google filter',
2034+
() => runQueryTest({
2035+
measures: [
2036+
'visitor_checkins_sources.count'
2037+
],
2038+
dimensions: [
2039+
'visitor_checkins_sources.visitor_id'
2040+
],
2041+
timeDimensions: [],
2042+
timezone: 'America/Los_Angeles',
2043+
filters: [{
2044+
or: [
2045+
{
2046+
dimension: 'visitor_checkins_sources.source',
2047+
operator: 'notSet',
2048+
},
2049+
{
2050+
dimension: 'visitor_checkins_sources.source',
2051+
operator: 'equals',
2052+
values: ['google']
2053+
}
2054+
]
2055+
}],
2056+
order: [{
2057+
id: 'visitor_checkins_sources.visitor_id'
2058+
}]
2059+
}, [
2060+
{
2061+
visitor_checkins_sources__visitor_id: 1,
2062+
visitor_checkins_sources__count: '3'
2063+
},
2064+
{
2065+
visitor_checkins_sources__visitor_id: 2,
2066+
visitor_checkins_sources__count: '2'
2067+
},
2068+
{
2069+
visitor_checkins_sources__visitor_id: 3,
2070+
visitor_checkins_sources__count: '1'
2071+
}
2072+
])
2073+
);
2074+
19002075
it('year granularity', () => runQueryTest({
19012076
measures: [
19022077
'visitors.visitor_count'

packages/cubejs-schema-compiler/test/unit/base-query.test.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1784,6 +1784,84 @@ describe('SQL Generation', () => {
17841784
expect(cubeSQL).toMatch(/\(\s*\(.*type\s*=\s*\$\d\$.*OR.*type\s*=\s*\$\d\$.*\)\s*AND\s*\(.*type\s*=\s*\$\d\$.*OR.*type\s*=\s*\$\d\$.*\)\s*\)/);
17851785
});
17861786

1787+
it('equals NULL filter', async () => {
1788+
await compilers.compiler.compile();
1789+
const query = new BaseQuery(compilers, {
1790+
measures: ['Order.count'],
1791+
filters: [
1792+
{
1793+
and: [
1794+
{
1795+
member: 'Order.type',
1796+
operator: 'equals',
1797+
values: [null],
1798+
},
1799+
]
1800+
}
1801+
],
1802+
});
1803+
const cubeSQL = query.cubeSql('Order');
1804+
expect(cubeSQL).toContain('where (((type IS NULL)))');
1805+
});
1806+
1807+
it('notSet(IS NULL) filter', async () => {
1808+
await compilers.compiler.compile();
1809+
const query = new BaseQuery(compilers, {
1810+
measures: ['Order.count'],
1811+
filters: [
1812+
{
1813+
and: [
1814+
{
1815+
member: 'Order.type',
1816+
operator: 'notSet',
1817+
},
1818+
]
1819+
}
1820+
],
1821+
});
1822+
const cubeSQL = query.cubeSql('Order');
1823+
expect(cubeSQL).toContain('where (((type IS NULL)))');
1824+
});
1825+
1826+
it('notEquals NULL filter', async () => {
1827+
await compilers.compiler.compile();
1828+
const query = new BaseQuery(compilers, {
1829+
measures: ['Order.count'],
1830+
filters: [
1831+
{
1832+
and: [
1833+
{
1834+
member: 'Order.type',
1835+
operator: 'notEquals',
1836+
values: [null],
1837+
},
1838+
]
1839+
}
1840+
],
1841+
});
1842+
const cubeSQL = query.cubeSql('Order');
1843+
expect(cubeSQL).toContain('where (((type IS NOT NULL)))');
1844+
});
1845+
1846+
it('set(IS NOT NULL) filter', async () => {
1847+
await compilers.compiler.compile();
1848+
const query = new BaseQuery(compilers, {
1849+
measures: ['Order.count'],
1850+
filters: [
1851+
{
1852+
and: [
1853+
{
1854+
member: 'Order.type',
1855+
operator: 'set',
1856+
},
1857+
]
1858+
}
1859+
],
1860+
});
1861+
const cubeSQL = query.cubeSql('Order');
1862+
expect(cubeSQL).toContain('where (((type IS NOT NULL)))');
1863+
});
1864+
17871865
it('propagate filter params from view into cube\'s query', async () => {
17881866
await compilers.compiler.compile();
17891867
const query = new BaseQuery(compilers, {

0 commit comments

Comments
 (0)