Skip to content

Commit 8a03741

Browse files
Enhance pretty formatting for JOINs, CREATE POLICY, and CTEs
- Add newline formatting for JOIN clauses in SELECT statements - Improve CREATE POLICY formatting with proper USING and WITH CHECK clause indentation - Implement CTE (Common Table Expressions) pretty formatting with proper indentation - Add comprehensive test cases for complex SQL formatting scenarios - Update snapshots to reflect improved formatting output - Maintain AST equivalence through expectParseDeparse validation Co-Authored-By: Dan Lynch <[email protected]>
1 parent 6e1487e commit 8a03741

File tree

7 files changed

+233
-14
lines changed

7 files changed

+233
-14
lines changed

packages/deparser/__tests__/pretty/__snapshots__/create-policy-pretty.test.ts.snap

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,20 @@ exports[`Pretty CREATE POLICY formatting should format simple CREATE POLICY with
2525
USING (published = true);"
2626
`;
2727

28+
exports[`Pretty CREATE POLICY formatting should format very complex CREATE POLICY with pretty option enabled 1`] = `
29+
"CREATE POLICY "complex_policy" ON sensitive_data
30+
AS RESTRICTIVE
31+
FOR SELECT
32+
TO admin_role
33+
USING (department = current_user_department() AND EXISTS (SELECT
34+
1
35+
FROM
36+
user_permissions
37+
WHERE
38+
(user_id = current_user_id() AND permission = 'read_sensitive')))
39+
WITH CHECK (approved = true AND created_by = current_user_id());"
40+
`;
41+
2842
exports[`Pretty CREATE POLICY formatting should maintain single-line format for complex policy when pretty disabled 1`] = `"CREATE POLICY "admin_policy" ON sensitive_data AS RESTRICTIVE FOR SELECT TO admin_role USING (department = current_user_department()) WITH CHECK (approved = true);"`;
2943

3044
exports[`Pretty CREATE POLICY formatting should maintain single-line format when pretty option disabled 1`] = `"CREATE POLICY "user_policy" ON users AS PERMISSIVE FOR ALL TO authenticated_users USING (user_id = current_user_id());"`;
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`Pretty CTE (Common Table Expressions) formatting should format basic CTE with pretty option enabled 1`] = `
4+
"WITH
5+
regional_sales AS (SELECT
6+
region,
7+
sum(sales_amount) AS total_sales
8+
FROM
9+
sales
10+
GROUP BY
11+
region)
12+
SELECT
13+
*
14+
FROM
15+
regional_sales;"
16+
`;
17+
18+
exports[`Pretty CTE (Common Table Expressions) formatting should format complex CTE with multiple CTEs with pretty option enabled 1`] = `
19+
"WITH
20+
regional_sales AS (SELECT
21+
region,
22+
sum(sales_amount) AS total_sales
23+
FROM
24+
sales
25+
GROUP BY
26+
region),
27+
top_regions AS (SELECT
28+
region
29+
FROM
30+
regional_sales
31+
WHERE
32+
total_sales > 1000000)
33+
SELECT
34+
*
35+
FROM
36+
top_regions;"
37+
`;
38+
39+
exports[`Pretty CTE (Common Table Expressions) formatting should format nested CTE with complex joins with pretty option enabled 1`] = `
40+
"WITH
41+
sales_summary AS (SELECT
42+
region,
43+
product_category,
44+
sum(amount) AS total
45+
FROM
46+
sales
47+
GROUP BY
48+
region,
49+
product_category),
50+
regional_totals AS (SELECT
51+
region,
52+
sum(total) AS region_total
53+
FROM
54+
sales_summary
55+
GROUP BY
56+
region)
57+
SELECT
58+
s.region,
59+
s.product_category,
60+
s.total,
61+
r.region_total
62+
FROM
63+
sales_summary AS s
64+
JOIN regional_totals AS r ON s.region = r.region;"
65+
`;
66+
67+
exports[`Pretty CTE (Common Table Expressions) formatting should format recursive CTE with pretty option enabled 1`] = `
68+
"WITH RECURSIVE
69+
employee_hierarchy AS (SELECT
70+
id,
71+
name,
72+
manager_id,
73+
1 AS level
74+
FROM
75+
employees
76+
WHERE
77+
manager_id IS NULL
78+
UNION
79+
ALL
80+
SELECT
81+
e.id,
82+
e.name,
83+
e.manager_id,
84+
eh.level + 1
85+
FROM
86+
employees AS e
87+
JOIN employee_hierarchy AS eh ON e.manager_id = eh.id)
88+
SELECT
89+
*
90+
FROM
91+
employee_hierarchy;"
92+
`;
93+
94+
exports[`Pretty CTE (Common Table Expressions) formatting should maintain single-line format when pretty option disabled 1`] = `"WITH regional_sales AS (SELECT region, sum(sales_amount) AS total_sales FROM sales GROUP BY region) SELECT * FROM regional_sales;"`;
95+
96+
exports[`Pretty CTE (Common Table Expressions) formatting should use custom newline and tab characters in pretty mode 1`] = `
97+
"WITH
98+
regional_sales AS (SELECT
99+
region,
100+
sum(sales_amount) AS total_sales
101+
FROM
102+
sales
103+
GROUP BY
104+
region)
105+
SELECT
106+
*
107+
FROM
108+
regional_sales;"
109+
`;

packages/deparser/__tests__/pretty/__snapshots__/select-pretty.test.ts.snap

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,21 @@ ORDER BY
1515
name;"
1616
`;
1717

18+
exports[`Pretty SELECT formatting should format SELECT with multiple JOINs with pretty option enabled 1`] = `
19+
"SELECT
20+
u.id,
21+
u.name,
22+
u.email,
23+
p.title
24+
FROM
25+
users AS u
26+
JOIN profiles AS p ON u.id = p.user_id
27+
LEFT JOIN orders AS o ON u.id = o.user_id
28+
RIGHT JOIN addresses AS a ON u.id = a.user_id
29+
WHERE
30+
u.active = true;"
31+
`;
32+
1833
exports[`Pretty SELECT formatting should format SELECT with subquery with pretty option enabled 1`] = `
1934
"SELECT
2035
id,
@@ -48,7 +63,8 @@ exports[`Pretty SELECT formatting should format complex SELECT with pretty optio
4863
u.email,
4964
p.title
5065
FROM
51-
users AS u JOIN profiles AS p ON u.id = p.user_id
66+
users AS u
67+
JOIN profiles AS p ON u.id = p.user_id
5268
WHERE
5369
u.active = true AND u.created_at > '2023-01-01'
5470
GROUP BY

packages/deparser/__tests__/pretty/create-policy-pretty.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ describe('Pretty CREATE POLICY formatting', () => {
77

88
const complexPolicySql = `CREATE POLICY admin_policy ON sensitive_data AS RESTRICTIVE FOR SELECT TO admin_role USING (department = current_user_department()) WITH CHECK (approved = true);`;
99

10+
const veryComplexPolicySql = `CREATE POLICY complex_policy ON sensitive_data AS RESTRICTIVE FOR SELECT TO admin_role USING (department = current_user_department() AND EXISTS (SELECT 1 FROM user_permissions WHERE user_id = current_user_id() AND permission = 'read_sensitive')) WITH CHECK (approved = true AND created_by = current_user_id());`;
11+
1012
const simplePolicySql = `CREATE POLICY simple_policy ON posts FOR SELECT TO public USING (published = true);`;
1113

1214
it('should format basic CREATE POLICY with pretty option enabled', async () => {
@@ -34,6 +36,11 @@ describe('Pretty CREATE POLICY formatting', () => {
3436
expect(result).toMatchSnapshot();
3537
});
3638

39+
it('should format very complex CREATE POLICY with pretty option enabled', async () => {
40+
const result = await expectParseDeparse(veryComplexPolicySql, { pretty: true });
41+
expect(result).toMatchSnapshot();
42+
});
43+
3744
it('should use custom newline and tab characters in pretty mode', async () => {
3845
const result = await expectParseDeparse(basicPolicySql, {
3946
pretty: true,
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import { deparseSync } from '../../src';
2+
import { parse } from 'libpg-query';
3+
import { expectParseDeparse } from '../../test-utils';
4+
5+
describe('Pretty CTE (Common Table Expressions) formatting', () => {
6+
const basicCteSql = `WITH regional_sales AS (SELECT region, SUM(sales_amount) as total_sales FROM sales GROUP BY region) SELECT * FROM regional_sales;`;
7+
8+
const complexCteSql = `WITH regional_sales AS (SELECT region, SUM(sales_amount) as total_sales FROM sales GROUP BY region), top_regions AS (SELECT region FROM regional_sales WHERE total_sales > 1000000) SELECT * FROM top_regions;`;
9+
10+
const recursiveCteSql = `WITH RECURSIVE employee_hierarchy AS (SELECT id, name, manager_id, 1 as level FROM employees WHERE manager_id IS NULL UNION ALL SELECT e.id, e.name, e.manager_id, eh.level + 1 FROM employees e JOIN employee_hierarchy eh ON e.manager_id = eh.id) SELECT * FROM employee_hierarchy;`;
11+
12+
const nestedCteSql = `WITH sales_summary AS (SELECT region, product_category, SUM(amount) as total FROM sales GROUP BY region, product_category), regional_totals AS (SELECT region, SUM(total) as region_total FROM sales_summary GROUP BY region) SELECT s.region, s.product_category, s.total, r.region_total FROM sales_summary s JOIN regional_totals r ON s.region = r.region;`;
13+
14+
it('should format basic CTE with pretty option enabled', async () => {
15+
const result = await expectParseDeparse(basicCteSql, { pretty: true });
16+
expect(result).toMatchSnapshot();
17+
});
18+
19+
it('should maintain single-line format when pretty option disabled', async () => {
20+
const result = await expectParseDeparse(basicCteSql, { pretty: false });
21+
expect(result).toMatchSnapshot();
22+
});
23+
24+
it('should format complex CTE with multiple CTEs with pretty option enabled', async () => {
25+
const result = await expectParseDeparse(complexCteSql, { pretty: true });
26+
expect(result).toMatchSnapshot();
27+
});
28+
29+
it('should format recursive CTE with pretty option enabled', async () => {
30+
const result = await expectParseDeparse(recursiveCteSql, { pretty: true });
31+
expect(result).toMatchSnapshot();
32+
});
33+
34+
it('should format nested CTE with complex joins with pretty option enabled', async () => {
35+
const result = await expectParseDeparse(nestedCteSql, { pretty: true });
36+
expect(result).toMatchSnapshot();
37+
});
38+
39+
it('should use custom newline and tab characters in pretty mode', async () => {
40+
const result = await expectParseDeparse(basicCteSql, {
41+
pretty: true,
42+
newline: '\r\n',
43+
tab: ' '
44+
});
45+
expect(result).toMatchSnapshot();
46+
});
47+
});

packages/deparser/__tests__/pretty/select-pretty.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ describe('Pretty SELECT formatting', () => {
77

88
const complexSelectSql = `SELECT u.id, u.name, u.email, p.title FROM users u JOIN profiles p ON u.id = p.user_id WHERE u.active = true AND u.created_at > '2023-01-01' GROUP BY u.id, u.name, u.email, p.title HAVING COUNT(*) > 1 ORDER BY u.created_at DESC, u.name ASC LIMIT 10 OFFSET 5;`;
99

10+
const multipleJoinsSql = `SELECT u.id, u.name, u.email, p.title FROM users AS u JOIN profiles AS p ON u.id = p.user_id LEFT JOIN orders AS o ON u.id = o.user_id RIGHT JOIN addresses AS a ON u.id = a.user_id WHERE u.active = true;`;
11+
1012
const selectWithSubquerySql = `SELECT id, name FROM users WHERE id IN (SELECT user_id FROM orders WHERE total > 100);`;
1113

1214
const selectUnionSql = `SELECT name FROM customers UNION ALL SELECT name FROM suppliers ORDER BY name;`;
@@ -41,6 +43,11 @@ describe('Pretty SELECT formatting', () => {
4143
expect(result).toMatchSnapshot();
4244
});
4345

46+
it('should format SELECT with multiple JOINs with pretty option enabled', async () => {
47+
const result = await expectParseDeparse(multipleJoinsSql, { pretty: true });
48+
expect(result).toMatchSnapshot();
49+
});
50+
4451
it('should use custom newline and tab characters in pretty mode', async () => {
4552
const result = await expectParseDeparse(basicSelectSql, {
4653
pretty: true,

packages/deparser/src/deparser.ts

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -970,9 +970,20 @@ export class Deparser implements DeparserVisitor {
970970
output.push('RECURSIVE');
971971
}
972972

973-
const ctes = ListUtils.unwrapList(node.ctes);
974-
const cteStrs = ctes.map(cte => this.visit(cte, context));
975-
output.push(cteStrs.join(', '));
973+
if (node.ctes && node.ctes.length > 0) {
974+
const ctes = ListUtils.unwrapList(node.ctes);
975+
if (this.formatter.isPretty()) {
976+
const cteStrings = ctes.map(cte => {
977+
const cteStr = this.visit(cte, context);
978+
return this.formatter.newline() + this.formatter.indent(cteStr);
979+
});
980+
return output.join(' ') + cteStrings.join(',');
981+
} else {
982+
const cteStrings = ctes.map(cte => this.visit(cte, context));
983+
output.push(cteStrings.join(', '));
984+
return output.join(' ');
985+
}
986+
}
976987

977988
return output.join(' ');
978989
}
@@ -3461,11 +3472,9 @@ export class Deparser implements DeparserVisitor {
34613472

34623473
switch (node.jointype) {
34633474
case 'JOIN_INNER':
3464-
// Handle NATURAL JOIN first - it has isNatural=true (NATURAL already added above)
34653475
if (node.isNatural) {
34663476
joinStr += 'JOIN';
34673477
}
3468-
// Handle CROSS JOIN case - when there's no quals, no usingClause, and not natural
34693478
else if (!node.quals && (!node.usingClause || node.usingClause.length === 0)) {
34703479
joinStr += 'CROSS JOIN';
34713480
} else {
@@ -3485,7 +3494,11 @@ export class Deparser implements DeparserVisitor {
34853494
joinStr += 'JOIN';
34863495
}
34873496

3488-
output.push(joinStr);
3497+
if (this.formatter.isPretty()) {
3498+
output.push(this.formatter.newline() + joinStr);
3499+
} else {
3500+
output.push(joinStr);
3501+
}
34893502

34903503
if (node.rarg) {
34913504
let rargStr = this.visit(node.rarg, context);
@@ -3498,18 +3511,25 @@ export class Deparser implements DeparserVisitor {
34983511
}
34993512

35003513
if (node.usingClause && node.usingClause.length > 0) {
3501-
output.push('USING');
35023514
const usingList = ListUtils.unwrapList(node.usingClause);
35033515
const columnNames = usingList.map(col => this.visit(col, context));
3504-
output.push(`(${columnNames.join(', ')})`);
3516+
if (this.formatter.isPretty()) {
3517+
output.push(`USING (${columnNames.join(', ')})`);
3518+
} else {
3519+
output.push('USING');
3520+
output.push(`(${columnNames.join(', ')})`);
3521+
}
35053522
} else if (node.quals) {
3506-
output.push('ON');
3507-
output.push(this.visit(node.quals, context));
3523+
if (this.formatter.isPretty()) {
3524+
output.push(`ON ${this.visit(node.quals, context)}`);
3525+
} else {
3526+
output.push('ON');
3527+
output.push(this.visit(node.quals, context));
3528+
}
35083529
}
35093530

3510-
let result = output.join(' ');
3531+
let result = this.formatter.isPretty() ? output.join(' ') : output.join(' ');
35113532

3512-
// Handle join_using_alias first (for USING clause aliases like "AS x")
35133533
if (node.join_using_alias && node.join_using_alias.aliasname) {
35143534
let aliasStr = node.join_using_alias.aliasname;
35153535
if (node.join_using_alias.colnames && node.join_using_alias.colnames.length > 0) {
@@ -3520,7 +3540,6 @@ export class Deparser implements DeparserVisitor {
35203540
result += ` AS ${aliasStr}`;
35213541
}
35223542

3523-
// Handle regular alias (for outer table aliases like "y")
35243543
if (node.alias && node.alias.aliasname) {
35253544
let aliasStr = node.alias.aliasname;
35263545
if (node.alias.colnames && node.alias.colnames.length > 0) {

0 commit comments

Comments
 (0)