Skip to content

Commit ea1ce87

Browse files
onlyjackfrostonlyjackfrost
authored
Merge pull request #332 from Canner/fix/duckdb-recursive-query
Fix: fix incorrect condition to use restDataStream
2 parents ab6bb8e + 8482e16 commit ea1ce87

File tree

3 files changed

+108
-98
lines changed

3 files changed

+108
-98
lines changed

‎packages/extension-api-caller/test/restApiCaller.spec.ts

+102-95
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { getTestCompiler } from '@vulcan-sql/test-utility';
22
import * as path from 'path';
33

4-
54
describe('Test "rest_api" filter', () => {
65
it(
76
'Should throw error when not pass the "url" argument',
@@ -16,9 +15,7 @@ describe('Test "rest_api" filter', () => {
1615
await compileAndLoad(sql);
1716

1817
// Assert
19-
await expect(execute({})).rejects.toThrow(
20-
'url is required'
21-
);
18+
await expect(execute({})).rejects.toThrow('url is required');
2219
},
2320
50 * 1000
2421
);
@@ -36,19 +33,20 @@ describe('Test "rest_api" filter', () => {
3633
stock: 94,
3734
brand: 'Apple',
3835
category: 'smartphones',
39-
thumbnail: 'https://i.dummyjson.com/data/products/1/thumbnail.jpg',
36+
thumbnail: 'https://cdn.dummyjson.com/product-images/1/thumbnail.jpg',
4037
images: [
41-
'https://i.dummyjson.com/data/products/1/1.jpg',
42-
'https://i.dummyjson.com/data/products/1/2.jpg',
43-
'https://i.dummyjson.com/data/products/1/3.jpg',
44-
'https://i.dummyjson.com/data/products/1/4.jpg',
45-
'https://i.dummyjson.com/data/products/1/thumbnail.jpg'
46-
]
38+
'https://cdn.dummyjson.com/product-images/1/1.jpg',
39+
'https://cdn.dummyjson.com/product-images/1/2.jpg',
40+
'https://cdn.dummyjson.com/product-images/1/3.jpg',
41+
'https://cdn.dummyjson.com/product-images/1/4.jpg',
42+
'https://cdn.dummyjson.com/product-images/1/thumbnail.jpg',
43+
],
4744
});
4845

49-
const { compileAndLoad, execute, getExecutedQueries, getCreatedBinding } = await getTestCompiler({
50-
extensions: { rest_api: path.join(__dirname, '..', 'src') },
51-
});
46+
const { compileAndLoad, execute, getExecutedQueries, getCreatedBinding } =
47+
await getTestCompiler({
48+
extensions: { rest_api: path.join(__dirname, '..', 'src') },
49+
});
5250

5351
const sql = `{% set value = { "path": { "id": 1 } } %}SELECT {{ value | rest_api(url='https://dummyjson.com/products/:id') }}`;
5452

@@ -64,96 +62,104 @@ describe('Test "rest_api" filter', () => {
6462
expect(bindings[0].get('$1')).toEqual(expected);
6563
},
6664
50 * 1000
67-
)
65+
);
6866

6967
it(
7068
'Should work with template engine and issue a GET request using the query parameter in value',
7169
async () => {
7270
const expected = JSON.stringify({
73-
"products": [
71+
products: [
7472
{
75-
"id": 1,
76-
"title": "iPhone 9",
77-
"description": "An apple mobile which is nothing like apple",
78-
"price": 549,
79-
"discountPercentage": 12.96,
80-
"rating": 4.69,
81-
"stock": 94,
82-
"brand": "Apple",
83-
"category": "smartphones",
84-
"thumbnail": "https://i.dummyjson.com/data/products/1/thumbnail.jpg",
85-
"images": [
86-
"https://i.dummyjson.com/data/products/1/1.jpg",
87-
"https://i.dummyjson.com/data/products/1/2.jpg",
88-
"https://i.dummyjson.com/data/products/1/3.jpg",
89-
"https://i.dummyjson.com/data/products/1/4.jpg",
90-
"https://i.dummyjson.com/data/products/1/thumbnail.jpg"
91-
]
73+
id: 1,
74+
title: 'iPhone 9',
75+
description: 'An apple mobile which is nothing like apple',
76+
price: 549,
77+
discountPercentage: 12.96,
78+
rating: 4.69,
79+
stock: 94,
80+
brand: 'Apple',
81+
category: 'smartphones',
82+
thumbnail:
83+
'https://cdn.dummyjson.com/product-images/1/thumbnail.jpg',
84+
images: [
85+
'https://cdn.dummyjson.com/product-images/1/1.jpg',
86+
'https://cdn.dummyjson.com/product-images/1/2.jpg',
87+
'https://cdn.dummyjson.com/product-images/1/3.jpg',
88+
'https://cdn.dummyjson.com/product-images/1/4.jpg',
89+
'https://cdn.dummyjson.com/product-images/1/thumbnail.jpg',
90+
],
9291
},
9392
{
94-
"id": 2,
95-
"title": "iPhone X",
96-
"description": "SIM-Free, Model A19211 6.5-inch Super Retina HD display with OLED technology A12 Bionic chip with ...",
97-
"price": 899,
98-
"discountPercentage": 17.94,
99-
"rating": 4.44,
100-
"stock": 34,
101-
"brand": "Apple",
102-
"category": "smartphones",
103-
"thumbnail": "https://i.dummyjson.com/data/products/2/thumbnail.jpg",
104-
"images": [
105-
"https://i.dummyjson.com/data/products/2/1.jpg",
106-
"https://i.dummyjson.com/data/products/2/2.jpg",
107-
"https://i.dummyjson.com/data/products/2/3.jpg",
108-
"https://i.dummyjson.com/data/products/2/thumbnail.jpg"
109-
]
93+
id: 2,
94+
title: 'iPhone X',
95+
description:
96+
'SIM-Free, Model A19211 6.5-inch Super Retina HD display with OLED technology A12 Bionic chip with ...',
97+
price: 899,
98+
discountPercentage: 17.94,
99+
rating: 4.44,
100+
stock: 34,
101+
brand: 'Apple',
102+
category: 'smartphones',
103+
thumbnail:
104+
'https://cdn.dummyjson.com/product-images/2/thumbnail.jpg',
105+
images: [
106+
'https://cdn.dummyjson.com/product-images/2/1.jpg',
107+
'https://cdn.dummyjson.com/product-images/2/2.jpg',
108+
'https://cdn.dummyjson.com/product-images/2/3.jpg',
109+
'https://cdn.dummyjson.com/product-images/2/thumbnail.jpg',
110+
],
110111
},
111112
{
112-
"id": 71,
113-
"title": "Women Shoulder Bags",
114-
"description": "LouisWill Women Shoulder Bags Long Clutches Cross Body Bags Phone Bags PU Leather Hand Bags Large Capacity Card Holders Zipper Coin Purses Fashion Crossbody Bags for Girls Ladies",
115-
"price": 46,
116-
"discountPercentage": 14.65,
117-
"rating": 4.71,
118-
"stock": 17,
119-
"brand": "LouisWill",
120-
"category": "womens-bags",
121-
"thumbnail": "https://i.dummyjson.com/data/products/71/thumbnail.jpg",
122-
"images": [
123-
"https://i.dummyjson.com/data/products/71/1.jpg",
124-
"https://i.dummyjson.com/data/products/71/2.jpg",
125-
"https://i.dummyjson.com/data/products/71/3.webp",
126-
"https://i.dummyjson.com/data/products/71/thumbnail.jpg"
127-
]
113+
id: 71,
114+
title: 'Women Shoulder Bags',
115+
description:
116+
'LouisWill Women Shoulder Bags Long Clutches Cross Body Bags Phone Bags PU Leather Hand Bags Large Capacity Card Holders Zipper Coin Purses Fashion Crossbody Bags for Girls Ladies',
117+
price: 46,
118+
discountPercentage: 14.65,
119+
rating: 4.71,
120+
stock: 17,
121+
brand: 'LouisWill',
122+
category: 'womens-bags',
123+
thumbnail:
124+
'https://cdn.dummyjson.com/product-images/71/thumbnail.jpg',
125+
images: [
126+
'https://cdn.dummyjson.com/product-images/71/1.jpg',
127+
'https://cdn.dummyjson.com/product-images/71/2.jpg',
128+
'https://cdn.dummyjson.com/product-images/71/3.webp',
129+
'https://cdn.dummyjson.com/product-images/71/thumbnail.jpg',
130+
],
128131
},
129132
{
130-
"id": 86,
131-
"title": "Bluetooth Aux",
132-
"description": "Bluetooth Aux Bluetooth Car Aux Car Bluetooth Transmitter Aux Audio Receiver Handfree Car Bluetooth Music Receiver Universal 3.5mm Streaming A2DP Wireless Auto AUX Audio Adapter With Mic For Phone MP3",
133-
"price": 25,
134-
"discountPercentage": 10.56,
135-
"rating": 4.57,
136-
"stock": 22,
137-
"brand": "Car Aux",
138-
"category": "automotive",
139-
"thumbnail": "https://i.dummyjson.com/data/products/86/thumbnail.jpg",
140-
"images": [
141-
"https://i.dummyjson.com/data/products/86/1.jpg",
142-
"https://i.dummyjson.com/data/products/86/2.webp",
143-
"https://i.dummyjson.com/data/products/86/3.jpg",
144-
"https://i.dummyjson.com/data/products/86/4.jpg",
145-
"https://i.dummyjson.com/data/products/86/thumbnail.jpg"
146-
]
147-
}
133+
id: 86,
134+
title: 'Bluetooth Aux',
135+
description:
136+
'Bluetooth Aux Bluetooth Car Aux Car Bluetooth Transmitter Aux Audio Receiver Handfree Car Bluetooth Music Receiver Universal 3.5mm Streaming A2DP Wireless Auto AUX Audio Adapter With Mic For Phone MP3',
137+
price: 25,
138+
discountPercentage: 10.56,
139+
rating: 4.57,
140+
stock: 22,
141+
brand: 'Car Aux',
142+
category: 'automotive',
143+
thumbnail:
144+
'https://cdn.dummyjson.com/product-images/86/thumbnail.jpg',
145+
images: [
146+
'https://cdn.dummyjson.com/product-images/86/1.jpg',
147+
'https://cdn.dummyjson.com/product-images/86/2.webp',
148+
'https://cdn.dummyjson.com/product-images/86/3.jpg',
149+
'https://cdn.dummyjson.com/product-images/86/4.jpg',
150+
'https://cdn.dummyjson.com/product-images/86/thumbnail.jpg',
151+
],
152+
},
148153
],
149-
"total": 4,
150-
"skip": 0,
151-
"limit": 4
154+
total: 4,
155+
skip: 0,
156+
limit: 4,
152157
});
153158

154-
const { compileAndLoad, execute, getExecutedQueries, getCreatedBinding } = await getTestCompiler({
155-
extensions: { rest_api: path.join(__dirname, '..', 'src') },
156-
});
159+
const { compileAndLoad, execute, getExecutedQueries, getCreatedBinding } =
160+
await getTestCompiler({
161+
extensions: { rest_api: path.join(__dirname, '..', 'src') },
162+
});
157163

158164
const sql = `{% set value = { "query": { "q": "phone" } } %}SELECT {{ value | rest_api(url='https://dummyjson.com/products/search') }}`;
159165

@@ -169,19 +175,20 @@ describe('Test "rest_api" filter', () => {
169175
expect(bindings[0].get('$1')).toEqual(expected);
170176
},
171177
50 * 1000
172-
)
178+
);
173179

174180
it(
175181
'Should work with template engine and issue a POST request with body and header in value',
176182
async () => {
177183
const expected = JSON.stringify({
178184
id: 101,
179-
title: 'BMW Pencil'
185+
title: 'BMW Pencil',
180186
});
181187

182-
const { compileAndLoad, execute, getExecutedQueries, getCreatedBinding } = await getTestCompiler({
183-
extensions: { rest_api: path.join(__dirname, '..', 'src') },
184-
});
188+
const { compileAndLoad, execute, getExecutedQueries, getCreatedBinding } =
189+
await getTestCompiler({
190+
extensions: { rest_api: path.join(__dirname, '..', 'src') },
191+
});
185192

186193
const sql = `{% set value = { "body": { "title": "BMW Pencil" }, "headers": { "Content-Type": "application/json" } } %}SELECT {{ value | rest_api(url='https://dummyjson.com/products/add', method='POST') }}`;
187194

@@ -197,5 +204,5 @@ describe('Test "rest_api" filter', () => {
197204
expect(bindings[0].get('$1')).toEqual(expected);
198205
},
199206
50 * 1000
200-
)
207+
);
201208
});

‎packages/extension-driver-duckdb/src/lib/duckdbDataSource.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ export class DuckDBDataSource extends DataSource<any, DuckDBOptions> {
100100
const { db, configurationParameters, ...options } =
101101
this.dbMapping.get(profileName)!;
102102
const [firstDataSQL, restDataSQL] = buildSQL(sql, operations);
103-
104103
// create new connection for each query
105104
const parameters = Array.from(bindParams.values());
106105
this.logRequest(firstDataSQL, parameters, options);
@@ -144,7 +143,7 @@ export class DuckDBDataSource extends DataSource<any, DuckDBOptions> {
144143
this.push(null);
145144
},
146145
});
147-
if (firstData.length >= chunkSize) {
146+
if (firstData.length >= chunkSize && restDataStream) {
148147
readable._read = async function () {
149148
if (restDataStream) {
150149
for await (const row of restDataStream) {

‎packages/extension-driver-duckdb/src/lib/sqlBuilder.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,11 @@ export const buildSQL = (
3535
sql: string,
3636
operations: Partial<Parameterized<SQLClauseOperation>>
3737
): string[] => {
38-
if (isNoOP(operations) && !/^select/.test(sql.toLowerCase()))
38+
if (
39+
isNoOP(operations) &&
40+
!/^select/.test(sql.toLowerCase()) &&
41+
!/^with recursive/.test(sql.toLowerCase())
42+
)
3943
return [sql, ''];
4044
let builtSQL = '';
4145
builtSQL += `SELECT * FROM (${removeEndingSemiColon(sql)})`;

0 commit comments

Comments
 (0)