Feature Request: Format "ON" to separate line

[rigogsilva]

Describe the Feature Is there a region so not add OR to the logicalOperatorNewline? Or maybe is there a different way to control that?

Why do you want this feature? Normally in sql the ON is not on the same line as the join itself. For example:

SELECT
  *
FROM
  foo
  JOIN foo 
    ON foo.account_id = bar.account_id
    AND foo.something = bar.something
  AND a = b

This looks a bit weird:

SELECT
  *
FROM
  foo
  JOIN foo ON foo.account_id = bar.account_id
    AND foo.something = bar.something
  AND a = b

And it is nice to look at the ON and AND clause without having to move your eye to the right. Helps readability.

[nene]

With the latest version (7.0.2) I actually get this result:

SELECT
  *
FROM
  foo
  JOIN foo ON foo.account_id = bar.account_id
  AND foo.something = bar.something
  AND a = b

But I agree that the current formatting of ON expression leaves it looking quite confusing. Ideally both of these AND-operator lines should be indented. I would consider both of these to be acceptable:

FROM
  foo
  JOIN foo ON foo.account_id = bar.account_id
    AND foo.something = bar.something
    AND a = b
-- or
FROM
  foo
  JOIN foo ON
    foo.account_id = bar.account_id
    AND foo.something = bar.something
    AND a = b

Though in my experience joining over multiple fields is not something you do often. You can get a much better result from the formatter if you use parenthesis:

SELECT
  *
FROM
  foo
  JOIN foo ON (
    foo.account_id = bar.account_id
    AND foo.something = bar.something
    AND a = b
  )

With the current architecture this is pretty difficult to fix. More likely after a larger rewrite of the parser that's currently in progress.

[rigogsilva]

Yeah I was thinking something like this also:

FROM
  foo
  JOIN foo 
    ON foo.account_id = bar.account_id
    AND foo.something = bar.something
    AND a = b

Which is closer to what I have seen in sql formatting.

Not a problem. Thanks for considering. I was able to handle it with post processing witch is not ideal but works:

  const onFormattedQuery = (
    sqlFormatterQuery.toString().split("\n")
    .map((line: string) => {
      if (line.includes(" ON ")) {
        const beginSpacesCount = line.search(/\S/);
        const newOnLone = `\n${"".padStart(2 + beginSpacesCount, " ")}ON `;
        return line.replace(" ON ", newOnLone);
      } else {
        return line;
      }
    })
)

[nene]

Glad you found a solution :)

[nene]

Relates to #267

[andrewtavis]

I was able to handle it with post processing witch is not ideal but works

@rigogsilva, would you be able to explain how you did the post processing you mentioned here? The onFormattedQuery snippet you posted makes sense, but then I'm not sure exactly where this should go and how to call it in accordance with sql-formatter. Any input you could provide would also be much appreciated, @nene 😊

[nene]

That snippet is supposed to simply be applied to sql-formatter output, like so:

import {format} from "sql-formatter";

const result = postProcessOnKeywords(format("SELECT *"));

function postProcessOnKeywords(sql) {
  return (
    sql.toString().split("\n")
    .map((line: string) => {
      if (line.includes(" ON ")) {
        const beginSpacesCount = line.search(/\S/);
        const newOnLone = `\n${"".padStart(2 + beginSpacesCount, " ")}ON `;
        return line.replace(" ON ", newOnLone);
      } else {
        return line;
      }
    })
  );
}

[andrewtavis]

Thanks for your reply, @nene!

And is there a way to then get this into a VS Code task or something that could be triggered automatically on save via scripts/settings? Asking as sql-formatter is very close to what my team and I need, but getting ON on the next line and adding some spacing that we can write our own scripts for would be needed for a rollout. Just have never written something that would automate some code changes, so am a bit a loss :)

[nene]

I guess the simplest way to integrate it into VSCode is to fork the VSCode extension and add this code.

Though, given the repeated interest here, I might also finally implement an option for this formatting style in sql-formatter.

[andrewtavis]

Thanks for a further reply! It'd be great if this were an option in here. As stated, the big thing on my end for potentially adopting sql-formatter team wide is ON on the next line and a line space before statements.

My TypeScript is a bit novice, but I'd be happy to give this a shot if you'd be willing to provide some directions on this 😊

[nene]

Thanks for interest in helping. Though there might be some quick'n'dirty fixes (like the one above) to patch this up, I want to do this properly, which means properly parsing a FROM-clause with all the joins. This is to ensure we only format the ON keyword inside join syntax and nowhere else (like in ON DELETE CASCADE, which will trip up the above hack).

However, I'm not yet fully sure how I would go about implementing it, so I can't really provide any real guidance.

I am actively considering it though. So hopefully it'll happen rather sooner than later.

[andrewtavis]

Thanks for your interest in implementing! I'll go ahead and read up a bit on the project further and we can discuss/ideate this going forward :) At the very least I can be here to cheer you on when the work begins 😅😊

[StephenDriffill]

Look forward to hearing if this feature will be added - it's the only thing holding our team back from using the vscode extension which otherwise is great - much faster and easier to integrate that the alternative we are considering (sqlfluff) despite that library being more configurable 👍

[nene]

This feature will likely never be implemented in SQL Formatter because there are fundamental problems with the architecture of this library.

I can instead suggest using prettier-plugin-sql-cst which already does format the code in the style you're suggesting.