Skip to content

fix(client): parse Presto JSON response with custom reviver and BigInts #25

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

Merged
merged 8 commits into from
Dec 20, 2024
2 changes: 1 addition & 1 deletion .nvmrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
18.17.1
22.12.0
9 changes: 8 additions & 1 deletion apps/nest-server/src/app/app.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,14 @@ export class AppService {
const results = await client.query(
`select returnflag, linestatus, sum(quantity) as sum_qty, sum(extendedprice) as sum_base_price, sum(extendedprice * (1 - discount)) as sum_disc_price, sum(extendedprice * (1 - discount) * (1 + tax)) as sum_charge, avg(quantity) as avg_qty, avg(extendedprice) as avg_price, avg(discount) as avg_disc, count(*) as count_order from lineitem where shipdate <= date '1998-12-01' group by returnflag, linestatus order by returnflag, linestatus`,
)
return { columns: results.columns, rows: results.data }
return {
columns: results.columns,
rows: JSON.stringify(results.data, (key, value) => {
if (typeof value != 'bigint') return value

return value.toString()
}),
}
} catch (error) {
return (error as PrestoError).message
}
Expand Down
50 changes: 46 additions & 4 deletions presto-client/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,18 @@ try {

### Additional notes

Additional notes
Additional notes on the `query` method:

- The `query` method is asynchronous and will return a promise that resolves to a PrestoQuery object.
- The `query` method will automatically retry the query if it fails due to a transient error.
- The `query` method will cancel the query if the client is destroyed.
- It's asynchronous and will return a promise that resolves to a PrestoQuery object.
- It will automatically retry the query if it fails due to a transient error.
- It will cancel the query if the client is destroyed.
- \*It parses big numbers with the BigInt JavaScript primitive. If your Presto response includes a number bigger than `Number.MAX_SAFE_INTEGER`, it will be parsed into a bigint, so you may need to consider that when handling the response, or serializing it.

\* Only if the current JavaScript environment supports the reviver with context in the JSON.parse callback. Check compatibility here:

- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse#browser_compatibility

Otherwise, bigger numbers will lose precision due to the default JavaScript JSON parsing.

## Get Query metadata information

Expand Down Expand Up @@ -240,3 +247,38 @@ const client = new PrestoClient({
user: 'root',
})
```

## Troubleshooting

### Do not know how to serialize a BigInt

Example error message:

```
Do not know how to serialize a BigInt
TypeError: Do not know how to serialize a BigInt
at JSON.stringify (<anonymous>)
```

Make sure to write a custom replacer and handle the serialization of BigInts if your Presto query returns a number bigger than `Number.MAX_SAFE_INTEGER`.
Example JSON.stringify replacer:

```javascript
const results = await client.query(`SELECT 1234567890123456623`)
return {
columns: results.columns,
rows: JSON.stringify(results.data, (key, value) => {
if (typeof value !== 'bigint') return value

return value.toString()
}),
}
```

### Numbers lose precision

Known issue:
If working with numbers bigger than `Number.MAX_SAFE_INTEGER`, and your environment does not support the `JSON.parse` with the context in the reviver (Node.js > 21.0.0, and certain browser versions), the default JSON.parse will make the number lose precision.
Check compatibility here:

- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse#browser_compatibility
10 changes: 9 additions & 1 deletion presto-client/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
QueryInfo,
Table,
} from './types'
import { parseWithBigInts } from './utils'

export class PrestoClient {
private baseUrl: string
Expand Down Expand Up @@ -270,7 +271,7 @@ export class PrestoClient {
throw new Error(`Query failed: ${JSON.stringify(await response.text())}`)
}

const prestoResponse = (await response.json()) as PrestoResponse
const prestoResponse = (await this.prestoConversionToJSON({ response })) as PrestoResponse
if (!prestoResponse) {
throw new Error(`Query failed with an empty response from the server.`)
}
Expand Down Expand Up @@ -334,6 +335,13 @@ export class PrestoClient {
method,
})
}

private async prestoConversionToJSON({ response }: { response: Response }): Promise<unknown> {
const text = await response.text()
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore JSON.parse with a 3 argument reviver is a stage 3 proposal with some support, allow it here.
return JSON.parse(text, parseWithBigInts)
}
}

export default PrestoClient
38 changes: 38 additions & 0 deletions presto-client/src/utils.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { parseWithBigInts } from './utils'

describe('parse big ints', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

it('can be plugged into JSON.parse', () => {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const primitiveNumber = JSON.parse('123', parseWithBigInts)
expect(primitiveNumber).toBe(123)

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const parsedObject = JSON.parse('{"key": "value"}', parseWithBigInts)
expect(parsedObject).toHaveProperty('key')
expect(parsedObject.key).toBe('value')
})

it('parses when the JSON.parse reviver context is available', () => {
const parsedValue = parseWithBigInts('key', Number.MAX_SAFE_INTEGER, {
source: Number.MIN_SAFE_INTEGER.toString(),
})
expect(parsedValue).toBe(Number.MAX_SAFE_INTEGER)
})

it('parses when the JSON.parse reviver context is not available', () => {
const parsedValue = parseWithBigInts('key', Number.MAX_SAFE_INTEGER, undefined)
expect(parsedValue).toBe(Number.MAX_SAFE_INTEGER)
})

it('parses big integers when JSON.parse reviver context is available', () => {
const largeNumberAsBigInt = BigInt(Number.MAX_SAFE_INTEGER) + 2n
const largeNumberAsNumber = Number.MAX_SAFE_INTEGER + 2
const parsedValue = parseWithBigInts('key', largeNumberAsNumber, {
source: largeNumberAsBigInt.toString(),
})
expect(typeof parsedValue).toEqual('bigint')
expect(parsedValue).toEqual(largeNumberAsBigInt)
})
})
27 changes: 27 additions & 0 deletions presto-client/src/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* Parses a JSON including bigger numbers into BigInts
* This function checks if JSON.parse reviver callback has a context parameter
* and falls back onto the default parsing if not.
* See also:
* - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse#browser_compatibility
* - https://github.com/tc39/proposal-json-parse-with-source
* @param _ Key
* @param value Parsed value
* @param context Context with source text
* @returns Parsed object with BigInts where required
*/
export function parseWithBigInts(_: string, value: unknown, context: { source: string } | undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤯

if (!context?.source) return value // Context source is not available, fallback to default parse

// Ignore non-numbers
if (typeof value != 'number') return value

// If not an integer, use the value
// TODO: Check if Presto can return floats that could also lose precision
if (!Number.isInteger(value)) return value

// If number is a safe integer, we can use it
if (Number.isSafeInteger(value)) return value

return BigInt(context.source)
}
Loading