Skip to content

Commit b6ec2a0

Browse files
nenenene
authored
Merge PR #304: Fix parsing and formatting of LIMIT clause
2 parents 5a7fe1d + e091dc4 commit b6ec2a0

File tree

6 files changed

+136
-94
lines changed

6 files changed

+136
-94
lines changed

‎src/formatter/ExpressionFormatter.ts

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -160,18 +160,14 @@ export default class ExpressionFormatter {
160160
this.layout.add(WS.NEWLINE, WS.INDENT, this.show(node.limitToken));
161161
this.layout.indentation.increaseTopLevel();
162162

163-
if (node.offsetToken) {
164-
this.layout.add(
165-
WS.NEWLINE,
166-
WS.INDENT,
167-
this.show(node.offsetToken),
168-
',',
169-
WS.SPACE,
170-
this.show(node.countToken),
171-
WS.SPACE
172-
);
163+
if (node.offset) {
164+
this.layout.add(WS.NEWLINE, WS.INDENT);
165+
this.layout = this.formatSubExpression(node.offset);
166+
this.layout.add(WS.NO_SPACE, ',', WS.SPACE);
167+
this.layout = this.formatSubExpression(node.count);
173168
} else {
174-
this.layout.add(WS.NEWLINE, WS.INDENT, this.show(node.countToken), WS.SPACE);
169+
this.layout.add(WS.NEWLINE, WS.INDENT);
170+
this.layout = this.formatSubExpression(node.count);
175171
}
176172
this.layout.indentation.decreaseTopLevel();
177173
}

‎src/parser/Parser.ts

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,7 @@ export default class Parser {
6767
private clause(): Clause | undefined {
6868
if (this.look().type === TokenType.RESERVED_COMMAND) {
6969
const name = this.next();
70-
const children: AstNode[] = [];
71-
while (
72-
this.look().type !== TokenType.RESERVED_COMMAND &&
73-
this.look().type !== TokenType.RESERVED_BINARY_COMMAND &&
74-
this.look().type !== TokenType.EOF &&
75-
this.look().type !== TokenType.CLOSE_PAREN &&
76-
this.look().type !== TokenType.DELIMITER
77-
) {
78-
children.push(this.expression());
79-
}
70+
const children = this.expressionsUntilClauseEnd();
8071
return { type: NodeType.clause, nameToken: name, children };
8172
}
8273
return undefined;
@@ -85,16 +76,7 @@ export default class Parser {
8576
private binaryClause(): BinaryClause | undefined {
8677
if (this.look().type === TokenType.RESERVED_BINARY_COMMAND) {
8778
const name = this.next();
88-
const children: AstNode[] = [];
89-
while (
90-
this.look().type !== TokenType.RESERVED_COMMAND &&
91-
this.look().type !== TokenType.RESERVED_BINARY_COMMAND &&
92-
this.look().type !== TokenType.EOF &&
93-
this.look().type !== TokenType.CLOSE_PAREN &&
94-
this.look().value !== TokenType.DELIMITER
95-
) {
96-
children.push(this.expression());
97-
}
79+
const children = this.expressionsUntilClauseEnd();
9880
return { type: NodeType.binary_clause, nameToken: name, children };
9981
}
10082
return undefined;
@@ -162,20 +144,25 @@ export default class Parser {
162144
}
163145

164146
private limitClause(): LimitClause | undefined {
165-
if (isToken.LIMIT(this.look()) && this.look(2).type === TokenType.COMMA) {
166-
return {
167-
type: NodeType.limit_clause,
168-
limitToken: this.next(),
169-
offsetToken: this.next(),
170-
countToken: this.next() && this.next(), // Discard comma token
171-
};
172-
}
173147
if (isToken.LIMIT(this.look())) {
174-
return {
175-
type: NodeType.limit_clause,
176-
limitToken: this.next(),
177-
countToken: this.next(),
178-
};
148+
const limitToken = this.next();
149+
const expr1 = this.expressionsUntilClauseEnd(t => t.type === TokenType.COMMA);
150+
if (this.look().type === TokenType.COMMA) {
151+
this.next(); // Discard comma token
152+
const expr2 = this.expressionsUntilClauseEnd();
153+
return {
154+
type: NodeType.limit_clause,
155+
limitToken,
156+
offset: expr1,
157+
count: expr2,
158+
};
159+
} else {
160+
return {
161+
type: NodeType.limit_clause,
162+
limitToken,
163+
count: expr1,
164+
};
165+
}
179166
}
180167
return undefined;
181168
}
@@ -188,6 +175,23 @@ export default class Parser {
188175
return undefined;
189176
}
190177

178+
private expressionsUntilClauseEnd(
179+
extraPredicate: (token: Token) => boolean = () => false
180+
): AstNode[] {
181+
const children: AstNode[] = [];
182+
while (
183+
this.look().type !== TokenType.RESERVED_COMMAND &&
184+
this.look().type !== TokenType.RESERVED_BINARY_COMMAND &&
185+
this.look().type !== TokenType.EOF &&
186+
this.look().type !== TokenType.CLOSE_PAREN &&
187+
this.look().type !== TokenType.DELIMITER &&
188+
!extraPredicate(this.look())
189+
) {
190+
children.push(this.expression());
191+
}
192+
return children;
193+
}
194+
191195
// Returns current token without advancing the pointer
192196
private look(ahead = 0): Token {
193197
return this.tokens[this.index + ahead] || EOF_TOKEN;

‎src/parser/ast.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ export type BetweenPredicate = {
7171
export type LimitClause = {
7272
type: NodeType.limit_clause;
7373
limitToken: Token;
74-
countToken: Token;
75-
offsetToken?: Token;
74+
count: AstNode[];
75+
offset?: AstNode[];
7676
};
7777

7878
// The "*" operator used in SELECT *

‎test/behavesLikeSqlFormatter.ts

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import supportsLinesBetweenQueries from './options/linesBetweenQueries';
1515
import supportsNewlineBeforeSemicolon from './options/newlineBeforeSemicolon';
1616
import supportsLogicalOperatorNewline from './options/logicalOperatorNewline';
1717
import supportsTabulateAlias from './options/tabulateAlias';
18+
import supportsLimit from './features/limit';
1819

1920
/**
2021
* Core tests for all SQL formatters
@@ -34,6 +35,7 @@ export default function behavesLikeSqlFormatter(format: FormatFn) {
3435
supportsNewlineBeforeSemicolon(format);
3536
supportsCommaPosition(format);
3637
supportsLogicalOperatorNewline(format);
38+
supportsLimit(format);
3739

3840
it('formats simple SELECT query', () => {
3941
const result = format('SELECT count(*),Column1 FROM Table1;');
@@ -135,36 +137,6 @@ export default function behavesLikeSqlFormatter(format: FormatFn) {
135137
`);
136138
});
137139

138-
it('formats LIMIT with two comma-separated values on single line', () => {
139-
const result = format('LIMIT 5, 10;');
140-
expect(result).toBe(dedent`
141-
LIMIT
142-
5, 10;
143-
`);
144-
});
145-
146-
it('formats LIMIT of single value followed by another SELECT using commas', () => {
147-
const result = format('LIMIT 5; SELECT foo, bar;');
148-
expect(result).toBe(dedent`
149-
LIMIT
150-
5;
151-
152-
SELECT
153-
foo,
154-
bar;
155-
`);
156-
});
157-
158-
it('formats LIMIT of single value and OFFSET', () => {
159-
const result = format('LIMIT 5 OFFSET 8;');
160-
expect(result).toBe(dedent`
161-
LIMIT
162-
5
163-
OFFSET
164-
8;
165-
`);
166-
});
167-
168140
it('formats SELECT query with SELECT query inside it', () => {
169141
const result = format(
170142
'SELECT *, SUM(*) AS total FROM (SELECT * FROM Posts LIMIT 30) WHERE a > b'

‎test/features/limit.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import dedent from 'dedent-js';
2+
3+
import { FormatFn } from 'src/sqlFormatter';
4+
5+
export default function supportsLimit(format: FormatFn) {
6+
it('formats LIMIT with two comma-separated values on single line', () => {
7+
const result = format('LIMIT 5, 10;');
8+
expect(result).toBe(dedent`
9+
LIMIT
10+
5, 10;
11+
`);
12+
});
13+
14+
it('formats LIMIT of single value followed by another SELECT using commas', () => {
15+
const result = format('LIMIT 5; SELECT foo, bar;');
16+
expect(result).toBe(dedent`
17+
LIMIT
18+
5;
19+
20+
SELECT
21+
foo,
22+
bar;
23+
`);
24+
});
25+
26+
it('formats LIMIT of single value and OFFSET', () => {
27+
const result = format('LIMIT 5 OFFSET 8;');
28+
expect(result).toBe(dedent`
29+
LIMIT
30+
5
31+
OFFSET
32+
8;
33+
`);
34+
});
35+
36+
// Regression test for #303
37+
it('formats LIMIT with complex expressions', () => {
38+
const result = format('LIMIT abs(-5) - 1, (2 + 3) * 5;');
39+
expect(result).toBe(dedent`
40+
LIMIT
41+
abs(-5) - 1, (2 + 3) * 5;
42+
`);
43+
});
44+
45+
// Regression test for #301
46+
it('formats LIMIT with comments', () => {
47+
const result = format('LIMIT --comment\n 5,--comment\n6;');
48+
expect(result).toBe(dedent`
49+
LIMIT
50+
--comment
51+
5, --comment
52+
6;
53+
`);
54+
});
55+
}

‎test/unit/Parser.test.ts

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -322,12 +322,17 @@ describe('Parser', () => {
322322
Object {
323323
"children": Array [
324324
Object {
325-
"countToken": Object {
326-
"text": "10",
327-
"type": "NUMBER",
328-
"value": "10",
329-
"whitespaceBefore": " ",
330-
},
325+
"count": Array [
326+
Object {
327+
"token": Object {
328+
"text": "10",
329+
"type": "NUMBER",
330+
"value": "10",
331+
"whitespaceBefore": " ",
332+
},
333+
"type": "token",
334+
},
335+
],
331336
"limitToken": Object {
332337
"text": "LIMIT",
333338
"type": "RESERVED_COMMAND",
@@ -350,24 +355,34 @@ describe('Parser', () => {
350355
Object {
351356
"children": Array [
352357
Object {
353-
"countToken": Object {
354-
"text": "10",
355-
"type": "NUMBER",
356-
"value": "10",
357-
"whitespaceBefore": " ",
358-
},
358+
"count": Array [
359+
Object {
360+
"token": Object {
361+
"text": "10",
362+
"type": "NUMBER",
363+
"value": "10",
364+
"whitespaceBefore": " ",
365+
},
366+
"type": "token",
367+
},
368+
],
359369
"limitToken": Object {
360370
"text": "LIMIT",
361371
"type": "RESERVED_COMMAND",
362372
"value": "LIMIT",
363373
"whitespaceBefore": "",
364374
},
365-
"offsetToken": Object {
366-
"text": "200",
367-
"type": "NUMBER",
368-
"value": "200",
369-
"whitespaceBefore": " ",
370-
},
375+
"offset": Array [
376+
Object {
377+
"token": Object {
378+
"text": "200",
379+
"type": "NUMBER",
380+
"value": "200",
381+
"whitespaceBefore": " ",
382+
},
383+
"type": "token",
384+
},
385+
],
371386
"type": "limit_clause",
372387
},
373388
],

0 commit comments

Comments
 (0)