Skip to content

Commit 7c8cab6

Browse files
Sylvestrembostock
Sylvestre
and
mbostock
authored
T-SQL syntax for LIMIT/OFFSET (#312)
* add mssql LIMIT/OFFSET syntax * tests * lint * correct ORDER BY injection position * more concise * test for no slice from * lint 2.0 * cleanup whitespace * escaping sort, filter and order by * throw if selected column is * with mssql * test for escaping * prettier table.mjs * prettier table-test.mjs * checking for operands in filter and throw if missing * do not mutate escape when needed * pass escaper as an argument (#314) * handling no columns correctly * use select.columns explicitely * make right query if no column but sorter specified * prettier Co-authored-by: Mike Bostock <[email protected]>
1 parent 6ba05ed commit 7c8cab6

File tree

2 files changed

+191
-21
lines changed

2 files changed

+191
-21
lines changed

‎src/table.mjs‎

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -248,20 +248,44 @@ export function makeQueryTemplate(operations, source) {
248248
];
249249
for (let i = 0; i < filter.length; ++i) {
250250
appendSql(i ? `\nAND ` : `\nWHERE `, args);
251-
appendWhereEntry(filter[i], args);
251+
appendWhereEntry(filter[i], args, escaper);
252252
}
253253
for (let i = 0; i < sort.length; ++i) {
254254
appendSql(i ? `, ` : `\nORDER BY `, args);
255-
appendOrderBy(sort[i], args);
255+
appendOrderBy(sort[i], args, escaper);
256256
}
257-
if (slice.to !== null || slice.from !== null) {
258-
appendSql(
259-
`\nLIMIT ${slice.to !== null ? slice.to - (slice.from || 0) : 1e9}`,
260-
args
261-
);
262-
}
263-
if (slice.from !== null) {
264-
appendSql(` OFFSET ${slice.from}`, args);
257+
if (source.dialect === "mssql") {
258+
if (slice.to !== null || slice.from !== null) {
259+
if (!sort.length) {
260+
if (!select.columns)
261+
throw new Error(
262+
"at least one column must be explicitly specified. Received '*'."
263+
);
264+
appendSql(`\nORDER BY `, args);
265+
appendOrderBy(
266+
{column: select.columns[0], direction: "ASC"},
267+
args,
268+
escaper
269+
);
270+
}
271+
appendSql(`\nOFFSET ${slice.from || 0} ROWS`, args);
272+
appendSql(
273+
`\nFETCH NEXT ${
274+
slice.to !== null ? slice.to - (slice.from || 0) : 1e9
275+
} ROWS ONLY`,
276+
args
277+
);
278+
}
279+
} else {
280+
if (slice.to !== null || slice.from !== null) {
281+
appendSql(
282+
`\nLIMIT ${slice.to !== null ? slice.to - (slice.from || 0) : 1e9}`,
283+
args
284+
);
285+
}
286+
if (slice.from !== null) {
287+
appendSql(` OFFSET ${slice.from}`, args);
288+
}
265289
}
266290
return args;
267291
}
@@ -282,16 +306,16 @@ function appendSql(sql, args) {
282306
strings[strings.length - 1] += sql;
283307
}
284308

285-
function appendOrderBy({column, direction}, args) {
286-
appendSql(`t.${column} ${direction.toUpperCase()}`, args);
309+
function appendOrderBy({column, direction}, args, escaper) {
310+
appendSql(`t.${escaper(column)} ${direction.toUpperCase()}`, args);
287311
}
288312

289-
function appendWhereEntry({type, operands}, args) {
313+
function appendWhereEntry({type, operands}, args, escaper) {
290314
if (operands.length < 1) throw new Error("Invalid operand length");
291315

292316
// Unary operations
293317
if (operands.length === 1) {
294-
appendOperand(operands[0], args);
318+
appendOperand(operands[0], args, escaper);
295319
switch (type) {
296320
case "n":
297321
appendSql(` IS NULL`, args);
@@ -310,7 +334,7 @@ function appendWhereEntry({type, operands}, args) {
310334
// Fallthrough to next parent block.
311335
} else if (["c", "nc"].includes(type)) {
312336
// TODO: Case (in)sensitive?
313-
appendOperand(operands[0], args);
337+
appendOperand(operands[0], args, escaper);
314338
switch (type) {
315339
case "c":
316340
appendSql(` LIKE `, args);
@@ -319,10 +343,10 @@ function appendWhereEntry({type, operands}, args) {
319343
appendSql(` NOT LIKE `, args);
320344
break;
321345
}
322-
appendOperand(likeOperand(operands[1]), args);
346+
appendOperand(likeOperand(operands[1]), args, escaper);
323347
return;
324348
} else {
325-
appendOperand(operands[0], args);
349+
appendOperand(operands[0], args, escaper);
326350
switch (type) {
327351
case "eq":
328352
appendSql(` = `, args);
@@ -345,13 +369,13 @@ function appendWhereEntry({type, operands}, args) {
345369
default:
346370
throw new Error("Invalid filter operation");
347371
}
348-
appendOperand(operands[1], args);
372+
appendOperand(operands[1], args, escaper);
349373
return;
350374
}
351375
}
352376

353377
// List operations
354-
appendOperand(operands[0], args);
378+
appendOperand(operands[0], args, escaper);
355379
switch (type) {
356380
case "in":
357381
appendSql(` IN (`, args);
@@ -366,9 +390,9 @@ function appendWhereEntry({type, operands}, args) {
366390
appendSql(")", args);
367391
}
368392

369-
function appendOperand(o, args) {
393+
function appendOperand(o, args, escaper) {
370394
if (o.type === "column") {
371-
appendSql(`t.${o.value}`, args);
395+
appendSql(`t.${escaper(o.value)}`, args);
372396
} else {
373397
args.push(o.value);
374398
args[0].push("");

‎test/table-test.mjs‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,53 @@ describe("makeQueryTemplate", () => {
106106
assert.deepStrictEqual(params, ["val1"]);
107107
});
108108

109+
it("makeQueryTemplate filter and escape filters column", () => {
110+
const source = {name: "db", dialect: "postgres", escape: (i) => `_${i}_`};
111+
const operations = {
112+
...baseOperations,
113+
filter: [
114+
{
115+
type: "eq",
116+
operands: [
117+
{type: "column", value: "col2"},
118+
{type: "resolved", value: "val1"}
119+
]
120+
}
121+
]
122+
};
123+
124+
const [parts, ...params] = makeQueryTemplate(operations, source);
125+
assert.deepStrictEqual(
126+
parts.join("?"),
127+
"SELECT t._col1_,t._col2_ FROM table1 t\nWHERE t._col2_ = ?"
128+
);
129+
assert.deepStrictEqual(params, ["val1"]);
130+
});
131+
132+
it("makeQueryTemplate filter and escape filters column only once", () => {
133+
const source = {name: "db", dialect: "postgres", escape: (i) => `_${i}_`};
134+
const operations = {
135+
...baseOperations,
136+
filter: [
137+
{
138+
type: "eq",
139+
operands: [
140+
{type: "column", value: "col2"},
141+
{type: "resolved", value: "val1"}
142+
]
143+
}
144+
]
145+
};
146+
147+
makeQueryTemplate(operations, source);
148+
const [parts, ...params] = makeQueryTemplate(operations, source);
149+
assert.deepStrictEqual(
150+
parts.join("?"),
151+
"SELECT t._col1_,t._col2_ FROM table1 t\nWHERE t._col2_ = ?"
152+
);
153+
assert.deepStrictEqual(params, ["val1"]);
154+
});
155+
109156
it("makeQueryTemplate filter list", () => {
110157
const source = {name: "db", dialect: "postgres"};
111158
const operations = {
@@ -164,6 +211,24 @@ describe("makeQueryTemplate", () => {
164211
assert.deepStrictEqual(params, []);
165212
});
166213

214+
it("makeQueryTemplate sort and escape sort column", () => {
215+
const source = {name: "db", dialect: "mysql", escape: (i) => `_${i}_`};
216+
const operations = {
217+
...baseOperations,
218+
sort: [
219+
{column: "col1", direction: "asc"},
220+
{column: "col2", direction: "desc"}
221+
]
222+
};
223+
224+
const [parts, ...params] = makeQueryTemplate(operations, source);
225+
assert.deepStrictEqual(
226+
parts.join("?"),
227+
"SELECT t._col1_,t._col2_ FROM table1 t\nORDER BY t._col1_ ASC, t._col2_ DESC"
228+
);
229+
assert.deepStrictEqual(params, []);
230+
});
231+
167232
it("makeQueryTemplate slice", () => {
168233
const source = {name: "db", dialect: "mysql"};
169234
const operations = {...baseOperations};
@@ -215,6 +280,87 @@ describe("makeQueryTemplate", () => {
215280
assert.deepStrictEqual(parts.join("?"), "SELECT t.col1,t.col2,t.col3 FROM table1 t\nWHERE t.col1 >= ?\nAND t.col2 = ?\nORDER BY t.col1 ASC\nLIMIT 90 OFFSET 10");
216281
assert.deepStrictEqual(params, ["val1", "val2"]);
217282
});
283+
284+
it("makeQueryTemplate select, slice and escape column name with mssql syntax", () => {
285+
const source = {name: "db", dialect: "mssql", escape: (i) => `_${i}_`};
286+
const operations = {
287+
...baseOperations,
288+
select: {
289+
columns: ["col1", "col2", "col3"]
290+
},
291+
slice: {to: 100}
292+
};
293+
294+
const [parts] = makeQueryTemplate(operations, source);
295+
assert.deepStrictEqual(
296+
parts.join("?"),
297+
"SELECT t._col1_,t._col2_,t._col3_ FROM table1 t\nORDER BY t._col1_ ASC\nOFFSET 0 ROWS\nFETCH NEXT 100 ROWS ONLY"
298+
);
299+
});
300+
301+
it("makeQueryTemplate select, sort, slice, filter indexed with mssql syntax", () => {
302+
const source = {name: "db", dialect: "mssql"};
303+
const operations = {
304+
...baseOperations,
305+
select: {
306+
columns: ["col1", "col2", "col3"]
307+
},
308+
sort: [{column: "col2", direction: "desc"}],
309+
slice: {from: 10, to: 100},
310+
filter: [
311+
{
312+
type: "gte",
313+
operands: [
314+
{type: "column", value: "col1"},
315+
{type: "resolved", value: "val1"}
316+
]
317+
},
318+
{
319+
type: "eq",
320+
operands: [
321+
{type: "column", value: "col2"},
322+
{type: "resolved", value: "val2"}
323+
]
324+
}
325+
]
326+
};
327+
328+
const [parts, ...params] = makeQueryTemplate(operations, source);
329+
assert.deepStrictEqual(parts.join("?"), "SELECT t.col1,t.col2,t.col3 FROM table1 t\nWHERE t.col1 >= ?\nAND t.col2 = ?\nORDER BY t.col2 DESC\nOFFSET 10 ROWS\nFETCH NEXT 90 ROWS ONLY");
330+
assert.deepStrictEqual(params, ["val1", "val2"]);
331+
});
332+
333+
it("makeQueryTemplate throw if no columns are explicitly specified for mssql dialect", () => {
334+
const source = {name: "db", dialect: "mssql"};
335+
const operations = {
336+
...baseOperations,
337+
select: {
338+
columns: null
339+
},
340+
sort: [],
341+
slice: {from: 10, to: 100}
342+
};
343+
344+
assert.throws(() => {
345+
makeQueryTemplate(operations, source);
346+
}, Error);
347+
});
348+
349+
it("makeQueryTemplate the sort and slice if no columns are explicitly BUT sort has value for mssql dialect", () => {
350+
const source = {name: "db", dialect: "mssql"};
351+
const operations = {
352+
...baseOperations,
353+
select: {
354+
columns: null
355+
},
356+
sort: [{column: "col2", direction: "desc"}],
357+
slice: {from: 10, to: 100}
358+
};
359+
360+
const [parts, ...params] = makeQueryTemplate(operations, source);
361+
assert.deepStrictEqual(parts.join("?"), "SELECT * FROM table1 t\nORDER BY t.col2 DESC\nOFFSET 10 ROWS\nFETCH NEXT 90 ROWS ONLY");
362+
assert.deepStrictEqual(params, []);
363+
});
218364
});
219365

220366
describe("__table", () => {

0 commit comments

Comments
 (0)