Skip to content

Commit ba20364

Browse files
committed
feat(duckdb-driver): Fix numeric filter comparisons in DuckDB
Fixes [#9281](#9281) by implementing explicit CAST for numeric filters in the DuckDB driver. This ensures correct numeric comparisons and prevents mismatches caused by string-to-number conversions. Added test cases to validate casting behavior for different filter scenarios.
1 parent e470778 commit ba20364

File tree

3 files changed

+126
-1
lines changed

3 files changed

+126
-1
lines changed

packages/cubejs-duckdb-driver/src/DuckDBQuery.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ const GRANULARITY_TO_INTERVAL: Record<string, (date: string) => string> = {
1212
};
1313

1414
class DuckDBFilter extends BaseFilter {
15+
public castParameter() {
16+
if (this.measure || this.definition().type === 'number') {
17+
return 'CAST(? AS DOUBLE)';
18+
}
19+
return '?';
20+
}
1521
}
1622

1723
export class DuckDBQuery extends BaseQuery {

packages/cubejs-testing/birdbox-fixtures/duckdb/schema/Orders.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,5 +29,9 @@ cube(`Orders`, {
2929
sql: `status`,
3030
type: `string`,
3131
},
32+
amount: {
33+
sql: `amount`,
34+
type: `number`,
35+
},
3236
},
3337
});

packages/cubejs-testing/test/smoke-duckdb.test.ts

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import cubejs, { CubeApi } from '@cubejs-client/core';
22
// eslint-disable-next-line import/no-extraneous-dependencies
3-
import { afterAll, beforeAll, jest } from '@jest/globals';
3+
import { afterAll, beforeAll, expect, jest, test } from '@jest/globals';
44
import { BirdBox, getBirdbox } from '../src';
55
import {
66
DEFAULT_API_TOKEN,
@@ -37,4 +37,119 @@ describe('duckdb', () => {
3737
}, JEST_AFTER_ALL_DEFAULT_TIMEOUT);
3838

3939
test('query measure', () => testQueryMeasure(client));
40+
41+
test('numeric measure filter - uses CAST', async () => {
42+
const response = await client.load({
43+
measures: [
44+
'Orders.count'
45+
],
46+
filters: [
47+
{
48+
member: 'Orders.totalAmount',
49+
operator: 'gte',
50+
values: ['300']
51+
}
52+
]
53+
});
54+
55+
// The HAVING clause applies after counting all records
56+
expect(response.rawData()[0]['Orders.count']).toBe('5');
57+
});
58+
59+
test('numeric dimension filter - uses CAST', async () => {
60+
// This test verifies that the WHERE clause for numeric dimensions uses CAST(? AS DOUBLE)
61+
const response = await client.load({
62+
measures: [
63+
'Orders.count'
64+
],
65+
filters: [
66+
{
67+
member: 'Orders.amount',
68+
operator: 'gte',
69+
values: ['300']
70+
}
71+
]
72+
});
73+
74+
// Only 3 orders have amount >= 300
75+
expect(response.rawData()[0]['Orders.count']).toBe('3');
76+
});
77+
78+
test('string filter - does NOT use CAST', async () => {
79+
// This test verifies that the WHERE clause for string dimensions does not use CAST
80+
const response = await client.load({
81+
measures: [
82+
'Orders.count'
83+
],
84+
filters: [
85+
{
86+
member: 'Orders.status',
87+
operator: 'equals',
88+
values: ['processed']
89+
}
90+
]
91+
});
92+
93+
// There are 2 'processed' orders
94+
expect(response.rawData()[0]['Orders.count']).toBe('2');
95+
});
96+
97+
test('numeric exact equality filter - also uses CAST', async () => {
98+
// This test verifies that even for equality comparisons on numeric values, CAST is used
99+
const response = await client.load({
100+
measures: [
101+
'Orders.count'
102+
],
103+
filters: [
104+
{
105+
member: 'Orders.amount',
106+
operator: 'equals',
107+
values: ['300']
108+
}
109+
]
110+
});
111+
112+
// Only 1 order has amount exactly 300
113+
expect(response.rawData()[0]['Orders.count']).toBe('1');
114+
});
115+
116+
test('numeric string comparison - values are properly handled with CAST', async () => {
117+
// This test verifies that numeric strings are properly cast as numbers
118+
// This is important because '100' and 100 are different in string comparisons
119+
// but should be treated the same with numeric CAST
120+
const response = await client.load({
121+
measures: [
122+
'Orders.count'
123+
],
124+
filters: [
125+
{
126+
member: 'Orders.amount',
127+
operator: 'equals',
128+
values: ['100'] // string '100' should be cast to number 100
129+
}
130+
]
131+
});
132+
133+
// Only 1 order has amount 100
134+
expect(response.rawData()[0]['Orders.count']).toBe('1');
135+
});
136+
137+
test('multiple string values in filter - none use CAST', async () => {
138+
// This test verifies that IN (...) clauses for string dimensions don't use CAST
139+
const response = await client.load({
140+
measures: [
141+
'Orders.count'
142+
],
143+
filters: [
144+
{
145+
member: 'Orders.status',
146+
operator: 'equals',
147+
values: ['new', 'processed']
148+
}
149+
]
150+
});
151+
152+
// There are 4 orders with status 'new' or 'processed'
153+
expect(response.rawData()[0]['Orders.count']).toBe('4');
154+
});
40155
});

0 commit comments

Comments
 (0)