Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type inference limitations #251

Open
timvandam opened this issue Aug 7, 2024 · 2 comments
Open

Type inference limitations #251

timvandam opened this issue Aug 7, 2024 · 2 comments

Comments

@timvandam
Copy link

timvandam commented Aug 7, 2024

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):

  • OS: [e.g. iOS] macos
  • PostgreSQL version [e.g. 13, 14] 15
  • Version [e.g. 22] latest

Additional context
Add any other context about the problem here.

@timvandam
Copy link
Author

timvandam commented Aug 7, 2024

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
Copy link
Author

timvandam commented Aug 8, 2024

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: brianc/node-postgres#3292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant