Unexpected error on sql helper function

[Eprince-hub]

Describe the bug
SafeQL throws an error when using the sql() helper in Postgres. This is a feature that is supported in postgres.js but it seems like SafeQL doesn't allow using this helper in writing queries. The queries below would throw this error Invalid Query: the type "Helper<number[], []>" is not supported".

export async function query(a: number[]) {
  return await sql`
    SELECT
      *
    FROM
      try_safe_ql
    WHERE
      id IN ${sql(a)}
  `;
}

To Reproduce
Steps to reproduce the behavior:

1. Setup SafeQL
2. Use this code in .ts file

export async function query(a: number[]) {
  return await sql`
    SELECT
      *
    FROM
      try_safe_ql
    WHERE
      id IN ${sql(a)}
  `;
}

Expected behavior
Usage of sql() helper should not throw errors

Screenshots

Desktop (please complete the following information):

• OS: MAC OS
• PostgreSQL version 14
• Version [e.g. 22]

Additional context
Add any other context about the problem here.

[Newbie012]

Thanks for the detailed report.

This issue is similar to the other issue you've opened (#99) so I'll answer for both of them:

SafeQL tries to get the type for each "expression" in the SQL tag. It doesn't know that sql should be dealt with differently.

for example:

function sum(a, b) {
  return a + b;
}

const q = (x, y) => sql`select from tbl where col = ${sum(x, y)}`;

SafeQL takes the expression sum(x, y), get the return type (which is number) and converts it to pg type (int):

select from tbl where col = $1::int

SafeQL should start supporting query builders, but it shouldn't be specific to Postgres.js. Libraries such as Slonik should also be taken into account.

[esdee]

I think that the main advantage of supporting this is to be able to DRY up some code around column selection.

const cols = ['id', 'name', ...]; // e.g. users table
sql `select ${sql(cols)} from users`;

This is mentioned in the docs for Postgres.js, dynamic columns

[Newbie012]

Correct. Ideally, I'm not against using sql fragments in code.

I think I have no other option than implementing library-specific behavior { sqlFragmentSyntax: "postgres" | "slonik" | "..." }` to prevent incompatibilities between libraries.

[Newbie012]

I tweeted about my progress on this matter. Hopefully I'll push a V1 for this.

[Eprince-hub]

@Newbie012
Thanks a lot for creating such a useful plugin. I have found it to be very helpful in my work, and I appreciate the effort you put into developing and maintaining it. I see that you started the implementation of the SQL fragment support here #113
I just wanted to know how it is progressing and when we are expected to have this feature working in SafeQL

Thanks 👍

[Newbie012]

Hi @Eprince-hub

Thanks for the support 🙂
I think I need to rewrite my implementation for SQL fragments so it can be more robust and easier to maintain. SafeQL doesn't have a community so it's a bit hard for me to prioritize features. I hope I'll continue working on fragment support in the next week. Although, fragments have their own limitations, so be advised.

[Eprince-hub]

Hi @Newbie012,
I hope you're doing well. I wanted to show you an "upsert" query (INSERT...ON CONFLICT UPDATE) like the example I have below. I know you are working on this issue, but I just want to show you more examples of the SQL fragments usage

export async function upsertQuery(inputs: Input[]) {
  return await sql<Input[]>`
    INSERT INTO try_safe_ql
      ${sql(inputs)}
    ON CONFLICT
      (id)
    DO UPDATE SET
      ${Object.keys(inputs[0]!).map(
        (field, count) =>
          sql`${count > 0 ? sql`,` : sql``} ${sql(field)} = excluded.${sql(
            field,
          )}`,
      )}
    RETURNING
      *
  `;
}

[Eprince-hub]

Workaround 1

You can disable the ESLint check for SafeQL for the lines showing this error.

export async function query(a: number[]) {
  return await sql`
    SELECT
      *
    FROM
      try_safe_ql
    WHERE
      id IN ${
        // eslint-disable-next-line @ts-safeql/check-sql -- Postgres.js sql() helper function currently not supported by SafeQL https://github.com/ts-safeql/safeql/issues/101#issuecomment-1623687201
        sql(a)
      }
  `;
}

[Eprince-hub]

Workaround 2

If disabling the ESLint checking is not what you want to do, then you can do something like this.

// Workaround for the Postgres.js sql() helper function that is causing a SafeQL linting error
// https://github.com/ts-safeql/safeql/issues/101#issuecomment-1623699171
export async function query(a: number[]) {
  return await sql`
    SELECT
      *
    FROM
      try_safe_ql
    WHERE
      id IN (SELECT unnest(${a}::int[]));
 `;
}

[louneskmt]

Unfortunately, the workaround 1 suppress the error for the whole sql statement and not only the fragment when using a conditional:

export function query(id: number) {
  return sql`
    SELECT *
    FROM try_safe_ql
    WHERE
      test = 'test'
      ${id ? sql`AND id = ${id}` : sql``}
  `;
}

[Newbie012]

@Eprince-hub another workaround would be to simply use = ANY instead of IN:

export async function query(a: number[]) {
  return await sql`
    SELECT
      *
    FROM
      try_safe_ql
    WHERE
      id = ANY(${a})
  `;
}

@louneskmt While SafeQL won't support conditional queries due to performance reasons, You could write your query with CASE WHEN expression instead (which personally I think it looks nicer):

export function query(id: number) {
  return sql`
    SELECT *
    FROM try_safe_ql
    WHERE
      test = 'test'
      AND CASE WHEN ${id !== ...} THEN id = ${id} END
  `;
}

[louneskmt]

Nice, I'll try that. Thanks 👍

[gajus]

Joining the party late. Does this currently work with Slonik helpers?

[Newbie012]

I have a private branch that allows 3rd party providers (such as DrizzleORM, kysely, postgres-js and slonik) to "translate" their expressions to sql syntax. I started with postgres-js, but it's way too dynamic.

I need to rewrite the branch so it could be easily maintained by others, but that will require some time that I don't have these days. I will get back to it eventually.

[gajus]

Looking forward to contributing Slonik adapter.

[mustakimkr]

It's really important feacher. I hope that you will give an update soon.
Thank you.