Apply various formatting fixes by joelmukuthu · Pull Request #80 · nene/prettier-plugin-sql-cst

joelmukuthu · 2026-08-10T14:28:52Z

Fixes:

Both proposals in Possible bug [postgres]: line break between function parethesis when indented #74:
- Break long binary expressions into multiple lines
- Prevent line break between empty parens, except when there are comments
Discussion [postgres]: How to format COMMENT statements spanning multiple lines #76
- Also applies a similar fix to CREATE INDEX statements
Discussion [postgres]: multi-line WHEN/THEN clauses in CASE statements #78 - with no line-break between multiple WHEN clauses

joelmukuthu · 2026-08-10T14:30:13Z

+      !expr.distinctKw &&
+      !expr.nullHandlingKw &&
+      !expr.orderBy &&
+      !expr.limit &&
+      !expr.having


Is there a way to avoid handling each case individually here?

At the moment I think not much can be done here. This needs some changes on the parser side to make it easier to handle within the prettier plugin.

nene

Thanks for the pull request. I've been busy with other stuff. Finally got to reviewing this.

In general it looks good:

Break long WHEN/THEN clauses into separate lines
- Needs some more thought on how to actually indent these.
Break long COMMENT ON clauses into separate lines
- Doesn't really match with the indentation suggested in #76
Break long CREATE INDEX clauses into separate lines ✅
Break long binary expressions into separate lines ✅
Avoid line-break between empty parenthesis
- This is the trickiest change. The general logic looks sound. I do have some recommendations though.

nene · 2026-08-20T10:37:33Z

+          SELECT
+            CASE
+              WHEN column_name = 1
+              THEN result_name
+              WHEN column_name = 2
+              THEN other_result
+              ELSE foo


I find this not so easy to read, because WHEN and THEN blocks are indented the same amount. IMHO it's sort of like formatting if-else in some other language like so:

if x > 10 return 15 else if x < 10 return 20

It also doesn't match with how the procedural version of CASE expression gets formatted:

CASE WHEN column_name = 1 THEN SELECT \good' WHEN columne_name = 2 THEN SELECT 'bad' ELSE SELECT 'other' END CASE

See case.test.ts

I would go with similar indentation for the long WHEN..THEN blocks in general:

CASE WHEN column_name = 1 THEN result_name WHEN columne_name = 2 THEN other_result ELSE foo END CASE

Makes sense. So for multi-line WHEN/THEN clauses, will it be okay to format as follows?

CASE WHEN column_name = 1 AND (column_name = 2 OR column_name = 3) AND column_name = 4 THEN result_name AND (result_name OR result_name) WHEN columne_name = 5 THEN other_result ELSE foo END

I think it would be better to treat the WHEN .. THEN pair as ( .. ) pair in C-style if statement, which in Prettier would get formatted like:

if ( column_name === 1 && (column_name === 2 OR column_name === 3) && column_name === 4 ) { return result_name } else if (columne_name === 5) { return other_result; }

So in SQL this would be:

CASE WHEN column_name = 1 AND (column_name = 2 OR column_name = 3) AND column_name = 4 THEN result_name WHEN columne_name = 5 THEN other_result ELSE foo END

This has the nice property, that the body of THEN is always indented the same amount. When the code between WHEN..THEN needs to wrap to multiple lines, the code coming after THEN won't need to be reformatted in any way.

nene · 2026-08-20T10:50:41Z

+      COMMENT ON
+      CONSTRAINT constraint_name ON DOMAIN domain_name
+      IS 'This is a really nice comment here.'


This indentation doesn't quite match with what was proposed in #76. According to that issue, it really should be:

COMMENT ON CONSTRAINT constraint_name ON DOMAIN domain_name IS 'This is a really nice comment here.'

Though I admit that the version you implemented might better align with the current overall indentation style that the formatter produces. But on another hand I'm not really so happy with that overall style. I'd like to move it more towards a bit more indentation. For example I'm not too happy with the indentation of ALTER TABLE statements:

-- current ALTER TABLE foo ALTER COLUMN bar SET DEFAULT 'hello'; -- I'd rather have it like: ALTER TABLE foo ALTER COLUMN bar SET DEFAULT 'hello';

Anyway... this COMMENT ON formatting is a minor thing. I don't mind if you leave it like this or change it.

Yeah, my implementation was trying to align with the current indentation for multiline statetements, but I also agree (and prefer) the hanging indentation style. Though most SQL code samples I see online don't using hanging indentation, so this might be a candidate for a config -- I think I also saw something alluding to that in this repo's STYLE GUIDE.

Would the hanging indentation also extend to DML statements? In my experience it works well for INSERT, UPDATE and DELETE but is a little tricky for SELECT statements where the return-list either has to be at the same indentation as the other clauses or at an extra indentation level.

SELECT col -- or one level deeper FROM .. WHERE ..;

Wrt this PR, can we ship the current non-hanging style for COMMENT ON and CREATE INDEX so that hanging indentation can be worked on in a separate PR?

Yep, as I said, no need to do anything about this in this PR.

In general the indentation of SQL is a tricky beast. It's hard to come up with a very good overall rule. Like yeah, that style for SELECT would be horrible.

I guess the main question to ask is does the current style cause confusion when reading it. With ALTER TABLE I see the problem that this single statement:

ALTER TABLE foo ALTER COLUMN bar SET DEFAULT 'hello';

Is at first glance very similar to 3 separate statements (ignore that this is completely broken code):

ALTER TABLE foo; ALTER COLUMN bar; SET DEFAULT 'hello';

...because each line starts with keywords that can be used to start a separate statement. This is not a case with SELECT where WHERE, FROM, etc aren't keywords that also start SQL statements.

I guess the main question to ask is does the current style cause confusion when reading it.

Great point. Perhaps taking it case by case is a good approach for this that could also reduce adoption friction. I'm happy to help out with it, just let me know how I can help 😊

nene · 2026-08-20T11:03:54Z

+const hasComments = (node: Node): boolean =>
+  Boolean((node as Node & { comments?: unknown[] }).comments?.length);


A better way to implement this, would be:

const hasComments = (node: Node): boolean => (node.leading?.length ?? 0) > 0 || (node.trailing?.length ?? 0) > 0;

The comments field is added to nodes by the Prettier engine. But the leading & trailing fields come directly from our parser.

Given that in here we really are interested from where the comments were in the original source code, we're better off using the leading & trailing. Plus we don't need the type-casts. Additionally it should be more resilient against changes in Prettier.

nene · 2026-08-20T11:13:06Z


 const isCompactOp = (op: string) => op === "->" || op === "->>";
+
+const isEmptyParenContent = (expr: Node): boolean => {


I would turn this function into:

const isEmptyParenExpr = (expr: ParenExpr): boolean => {

Then all that logic for determining whether it's an empty parenthesis would live in one place, and also the name of the function would IMHO be easier to understand.

joelmukuthu · 2026-08-20T14:42:43Z

Thanks for the review. Everything you write makes sense! I'm going to be AFK for the next 2 weeks so I'll respond/make updates after :)

joelmukuthu · 2026-09-03T11:23:54Z

@nene this is ready for re-review :)

joelmukuthu · 2026-09-03T11:26:35Z

+    it(`breaks long WHEN/THEN into separate lines`, async () => {
+      await test(
+        dedent`
+          SELECT
+            CASE
+              WHEN column_name = 1 THEN
+                result_name
+            END
+        `,
+        { printWidth: 40 },
+      );
+    });
+
+    it(`breaks multiple long WHEN/THEN clauses without blank lines between them`, async () => {
+      await test(
+        dedent`
+          SELECT
+            CASE
+              WHEN column_name = 1 THEN
+                result_name
+              WHEN column_name = 2 THEN
+                other_result
+              ELSE
+                foo
+            END
+        `,
+        { printWidth: 40 },
+      );
+    });


I think these tests have become obsolete now that we always break. Best to remove

joelmukuthu commented Aug 10, 2026

View reviewed changes

joelmukuthu force-pushed the fix/formatting-fixes branch 3 times, most recently from d26d012 to 120b664 Compare August 11, 2026 06:45

nene requested changes Aug 20, 2026

View reviewed changes

joelmukuthu force-pushed the fix/formatting-fixes branch from 120b664 to 893a539 Compare September 3, 2026 10:52

joelmukuthu added 5 commits September 3, 2026 13:17

Match WHEN/THEN/ELSE clause formatting with procedural style

f333c58

Break long COMMENT ON clauses into separate lines

d830774

Break long CREATE INDEX clauses into separate lines

b1a2322

Break long binary expressions into separate lines

c9069a4

Avoid line-break between empty parenthesis

868b3f9

joelmukuthu force-pushed the fix/formatting-fixes branch from 893a539 to 868b3f9 Compare September 3, 2026 11:18

joelmukuthu commented Sep 3, 2026

View reviewed changes

		const hasComments = (node: Node): boolean =>
		Boolean((node as Node & { comments?: unknown[] }).comments?.length);


		const isCompactOp = (op: string) => op === "->" \|\| op === "->>";

		const isEmptyParenContent = (expr: Node): boolean => {

Conversation

joelmukuthu commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nene left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nene Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joelmukuthu commented Aug 20, 2026

Uh oh!

joelmukuthu commented Sep 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

joelmukuthu commented Aug 10, 2026 •

edited

Loading

nene Aug 31, 2026 •

edited

Loading