adelsz / pgtyped

pgTyped - Typesafe SQL in TypeScript
https://pgtyped.dev
MIT License
2.91k stars 94 forks source link

Alternative variable syntax that's compatible with SQL formatters #459

Open bradleyayers opened 2 years ago

bradleyayers commented 2 years ago

I'd like to explore the idea of changing PgTyped to use syntax that is valid SQL, so that it can be formatted by SQL formatters that are standard compliant. At the moment I'm finding most SQL formatters get tripped up by the :foo and particularly the :foo! syntax.

Are there any ideas for alternative variable placeholder syntax that would stay SQL compliant? The best idea I've had is to use quoted identifier syntax (e.g. ":id!"), but unfortunately this is still a syntax error for spread scenarios.

FWIW it looks like https://sql-formatter-org.github.io/sql-formatter/ is one of the best out there.

adelsz commented 2 years ago

Would it make more sense to provide our own formatting as part of pgTyped? I did some experiments with alternative parameter styles like @paramName but couldn't find any which would play nice with sql formatters.

bradleyayers commented 2 years ago

Would it make more sense to provide our own formatting as part of pgTyped? I did some experiments with alternative parameter styles like @paramName but couldn't find any which would play nice with sql formatters.

As in include a SQL formatter in pgTyped? My initial reaction is no, as I'm skeptical that there's enough time from contributors (including yourself) to support that extra surface area, and competing with existing formatters seems like a battle that's inevitable to lose.

I'm still somewhat optimistic that there's an alternative syntax we could use that would be SQL compatible as well being something pgTyped could do variable substitution on, I don't feel confident we've explored the full landscape of possibilities.

I'd like to put together a full list of syntax ideas that have been considered, I think that would help me be confident whether there is/isn't a viable option.

adelsz commented 2 years ago

As in include a SQL formatter in pgTyped? My initial reaction is no, as I'm skeptical that there's enough time from contributors (including yourself) to support that extra surface area, and competing with existing formatters seems like a battle that's inevitable to lose.

Writing our own formatter from scratch wouldn't make sense, but we can fork sql-formatter or some other project and extend it to support pgTyped syntax. I expect the diff to upstream to be quite minimal.

I'm still somewhat optimistic that there's an alternative syntax we could use that would be SQL compatible as well being something pgTyped could do variable substitution on

I am not that optimistic once we require syntax conciseness and simplicity. With that said I am open to suggestions here, maybe there are some good variants I am missing.

jayp commented 1 year ago

I just build a tiny wrapper around sql-formatter npm package to solve this.

import * as fg from "fast-glob";
import * as fs from "fs";
import { format } from "sql-formatter";

/**
 * We need to create our own sql formatter as prettier-plugin-sql is busted.
 * Won't support passing of paramTypes to underlying sql-formatter.
 */
// find files recursively if not provided as argument
const files =
  process.argv.length > 2 ? process.argv.slice(2) : fg.sync("**/*.sql");
for (const f of files) {
  console.log("formatting: ", f);
  const original = fs.readFileSync(f, "utf-8");
  // NOTE(jayp): rewrite sql to support pgtyped's non-nullable params, e.g., :user_id!
  const nonNullableParamsReplaced = original.replace(/(:\w+)!/g, "$1__");
  const formatted =
    format(nonNullableParamsReplaced, {
      language: "postgresql",
      keywordCase: "lower",
      paramTypes: { named: [":"] },
    }) + "\n"; // add EOL
  // NOTE(jayp): There is a bug in sql-formatter where it adds a space
  // if the file starts with a SQL comment, as in:
  // "-- migrate:up" becomes " -- migrate:up" without this fix.
  const formattedWithoutLeadingSpace = formatted.startsWith(" --")
    ? formatted.substring(1)
    : formatted;
  const nonNullableParamsInsertedBack = formattedWithoutLeadingSpace.replace(
    /(:\w+)__/g,
    "$1!"
  );
  if (nonNullableParamsInsertedBack !== original) {
    fs.writeFileSync(f, nonNullableParamsInsertedBack);
  }
}

I then used lint-staged npm package to format my sql files prior to each commit (and also do code autogen at this time):

"lint-staged": {
    "*.sql": "ts-node buildutils/sql_formatter.ts",
    "src/**/*.sql": "yarn pgtyped -c pgtyped.json"
}
bradleyayers commented 1 year ago

Pretty neat! Thanks for sharing. I would absolutely love this as a VS code formatter, as we use "format on save" rather than lint staged hooks. Any thoughts on integrating it in that way?

cabello commented 1 month ago

I used to have the same issue, I use the following VSCode extension: https://marketplace.visualstudio.com/items?itemName=bradymholt.pgformatter Which uses https://github.com/darold/pgFormatter

When formatting variables I would end up with an unwanted space:

where
  people.id = $ personId

pgFormatter has a param called placeholder to allow for variables like that and this is the way I ended up configuring:

placeholder=\$[\S]+

Explanation: escape the $ sign, one or more of anything that is not space.

Now when I format my queries it doesn't touch these variables:

where
  people.id = $personId