ts-safeql / safeql

Validate and auto-generate TypeScript types from raw SQL queries in PostgreSQL.
https://safeql.dev
MIT License
1.35k stars 22 forks source link

Type inference limitations #251

Open timvandam opened 3 months ago

timvandam commented 3 months ago

Describe the bug A clear and concise description of what the bug is.

Not sure if a bug or limitation, but types are not inferred accurately. I have two ways of providing a number[] to UNNEST, but one of them forces me to cast to int[] while the other does not

To Reproduce Steps to reproduce the behavior:

Example setup:

export async function upsertWorkOrderItems(
  items: {
    workOrderId: number;
    uuid: string;
    shopifyOrderLineItemId: ID | null;
    data: WorkOrderItemData;
  }[],
) {
  if (!isNonEmptyArray(items)) {
    return;
  }

  const { shopifyOrderLineItemId, workOrderId, uuid, data } = nest(items);

  await sql`
    INSERT INTO "WorkOrderItem" ("workOrderId", uuid, "shopifyOrderLineItemId", data)
    SELECT *
    FROM UNNEST(${workOrderId} :: int[],
                ${uuid} :: uuid[],
                ${shopifyOrderLineItemId} :: text[],
                ${data.map(data => JSON.stringify(data))} :: jsonb[]);`;
}

export type Nest<T extends object> = { [K in keyof T]: T[K][] };

/**
 * The opposite of postgres' UNNEST.
 * Takes an array of objects and returns an array of properties.
 */
export function nest<T extends object>(items: NonEmptyArray<T>): Nest<T> {
  const [item] = items;
  const keys = Object.keys(item) as (keyof T)[];
  return Object.fromEntries(keys.map(key => [key, items.map(item => item[key])])) as Nest<T>;
}

Expected behavior A clear and concise description of what you expected to happen.

I would expect to not be forced to provide type hints like :: int[]. If I remove this cast I get errors. (ESLint: Invalid Query: function pg_catalog.unnest(text) does not exist(@ts-safeql/check-sql)) However, if I replace ${workOrderId} with ${items.map(item => item.workOrderId)} I am able to omit the :: int[]. The types of both is number[], so I would not expect this

Screenshots If applicable, add screenshots to help explain your problem.

N/A

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

timvandam commented 3 months ago

Another issue is branded types. I understand limitations in this area, but working around this is difficult. In the code above, shopifyOrderLineItemId has type (ID | null)[], where ID is branded. Using this branded type does not work, but surprisingly ${shopifyOrderLineItemId as (string | null)[]} also does not work. This only works if I first assign it to a variable:

const asd = shopifyOrderLineItemId as (string | null)[]

... ${asd}, ...

So the issue here is not really related to branded types, but to type casts inside the substitution template. This is probably similar to #234, as it only correctly considers identifiers (ie the variable) and not expressions (ie the inline type case)

timvandam commented 3 months ago

Getting some issues running the actual queries now. If I do not include type annotations manually, UNNEST will fail with

14:57:12 │ web-backend  │ error: function pg_catalog.unnest(unknown) is not unique
...
14:57:12 │ web-backend  │   hint: 'Could not choose a best candidate function. You might need to add explicit type casts.',

This even happens for a query that takes two string[] inside of unnest, so a pretty straightforward situation. This could be a postgres/pg limitation too, though, but not sure

Update: I'm blaming pg on this. It does not make the same assumptions about array types as safeql. Safeql assumes that arrays will be serialized to typed arrays, but pg uses array literal syntax which does not allow postgres to do any type inference, making casting required. Opened an issue for this: https://github.com/brianc/node-postgres/issues/3292