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

MySQL: args for offset, limit seem to be always number regardless of bigNumberStrings option value #24

Open
yshrsmz opened this issue Apr 12, 2024 · 0 comments

Comments

@yshrsmz
Copy link
Contributor

yshrsmz commented Apr 12, 2024

also posted to node-mysql2: sidorares/node-mysql2#2551

As stated in the title(and the liked issue), node-mysql2 seems to always use number for limit and offset(and possibly some other cases).

And that does not work well with our current implementation.

-- name: FindManyByAccountId :many
SELECT
  account_transaction_id,
  executed_at,
  account_id,
  point_saas_account_transaction_category_id,
  tenant_account_transaction_category_id,
  history_type,
  amount,
  expired_at,
  balance,
  title,
  content,
  sorted_id,
  created_at
FROM
  account_transactions
WHERE
  account_id = ?
ORDER BY
  executed_at DESC,
  sorted_id DESC
LIMIT
  ?
OFFSET
  ?;

sqlc-gen-typescript currently generates following code for the above query.

export interface FindManyByAccountIdArgs {
  accountId: number
  limit: string
  offset: string
}

export async function findManyByAccountId(
  client: Client,
  args: FindManyByAccountIdArgs,
): Promise<FindManyByAccountIdRow[]> {
  const [rows] = await client.query<RowDataPacket[]>({
    sql: findManyByAccountIdQuery,
    values: [args.accountId, args.limit, args.offset],
    rowsAsArray: true,
  })
  return rows.map((row) => {
    return {
      accountTransactionId: row[0],
      executedAt: row[1],
      accountId: row[2],
      pointSaasAccountTransactionCategoryId: row[3],
      tenantAccountTransactionCategoryId: row[4],
      historyType: row[5],
      amount: row[6],
      expiredAt: row[7],
      balance: row[8],
      title: row[9],
      content: row[10],
      sortedId: row[11],
      createdAt: row[12],
    }
  })
}

and this code results in the following node-mysql2 error

 { 
  code: 'ER_PARSE_ERROR', 
  errno: 1064, 
  sql: '-- name: FindManyByAccountId :many\nSELECT\n  account_transaction_id,\n  account_id,\n  point_saas_account_transaction_category_id,\n  tenant_account_transaction_category_id,\n  history_type,\n  amount,\n  expired_at,\n  balance,\n  title,\n  content,\n  created_at\nFROM\n  account_transactions\nWHERE\n  account_id = 1\nORDER BY\n  created_at DESC\nLIMIT\n  \'0\', \'20\'',
  sqlState: '42000', 
  sqlMessage: 'You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near \'\'20\', \'0\'\' at line 21'
}

So I think node-mysql2 does not respect supportBigNumbers and bigNumberStrings options here.

What I came up with is these two solution:

  1. Somehow get which argument is used for which column/condition, and decide TypeScript type
  2. Loosen the BigNumber type to always string | number and let users chose the type
    • if both supportBigNumbers and bigNumberStrings is false/unspecified, use number in that case.
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