pgtyped icon indicating copy to clipboard operation
pgtyped copied to clipboard

Alternative variable syntax that's compatible with SQL formatters

Open bradleyayers opened this issue 3 years ago • 6 comments

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.

bradleyayers avatar Jul 26 '22 21:07 bradleyayers

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.

adelsz avatar Aug 03 '22 21:08 adelsz

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.

bradleyayers avatar Aug 03 '22 21:08 bradleyayers

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.

adelsz avatar Aug 04 '22 00:08 adelsz

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"
}

jayp avatar Oct 02 '22 05:10 jayp

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?

bradleyayers avatar Oct 02 '22 08:10 bradleyayers