Skip to content

Commit b813a03

Browse files
gregnrgregnr
authored
Merge pull request #1001 from supabase/feat/parameter-binding
feat: optional parameter binding on query endpoint
2 parents d521264 + 455b744 commit b813a03

File tree

5 files changed

+113
-36
lines changed

5 files changed

+113
-36
lines changed

‎src/lib/PostgresMeta.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import { PostgresMetaResult, PoolConfig } from './types.js'
2424
export default class PostgresMeta {
2525
query: (
2626
sql: string,
27-
opts?: { statementQueryTimeout?: number; trackQueryInSentry?: boolean }
27+
opts?: { statementQueryTimeout?: number; trackQueryInSentry?: boolean; parameters?: unknown[] }
2828
) => Promise<PostgresMetaResult<any>>
2929
end: () => Promise<void>
3030
columnPrivileges: PostgresMetaColumnPrivileges

‎src/lib/db.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@ pg.types.setTypeParser(1017, (x) => x) // _point
2323

2424
// Ensure any query will have an appropriate error handler on the pool to prevent connections errors
2525
// to bubble up all the stack eventually killing the server
26-
const poolerQueryHandleError = (pgpool: pg.Pool, sql: string): Promise<pg.QueryResult<any>> => {
26+
const poolerQueryHandleError = (
27+
pgpool: pg.Pool,
28+
sql: string,
29+
parameters?: unknown[]
30+
): Promise<pg.QueryResult<any>> => {
2731
return Sentry.startSpan(
2832
{ op: 'db', name: 'poolerQuery' },
2933
() =>
@@ -44,7 +48,7 @@ const poolerQueryHandleError = (pgpool: pg.Pool, sql: string): Promise<pg.QueryR
4448
// such as parse or RESULT_SIZE_EXCEEDED errors instead, handle the error gracefully by bubbling in up to the caller
4549
pgpool.once('error', connectionErrorHandler)
4650
pgpool
47-
.query(sql)
51+
.query(sql, parameters)
4852
.then((results: pg.QueryResult<any>) => {
4953
if (!rejected) {
5054
return resolve(results)
@@ -64,7 +68,7 @@ const poolerQueryHandleError = (pgpool: pg.Pool, sql: string): Promise<pg.QueryR
6468
export const init: (config: PoolConfig) => {
6569
query: (
6670
sql: string,
67-
opts?: { statementQueryTimeout?: number; trackQueryInSentry?: boolean }
71+
opts?: { statementQueryTimeout?: number; trackQueryInSentry?: boolean; parameters?: unknown[] }
6872
) => Promise<PostgresMetaResult<any>>
6973
end: () => Promise<void>
7074
} = (config) => {
@@ -108,7 +112,7 @@ export const init: (config: PoolConfig) => {
108112
return {
109113
async query(
110114
sql,
111-
{ statementQueryTimeout, trackQueryInSentry } = { trackQueryInSentry: true }
115+
{ statementQueryTimeout, trackQueryInSentry, parameters } = { trackQueryInSentry: true }
112116
) {
113117
return Sentry.startSpan(
114118
// For metrics purposes, log the query that will be run if it's not an user provided query (with possibly sentitives infos)
@@ -131,15 +135,15 @@ export const init: (config: PoolConfig) => {
131135
try {
132136
if (!pool) {
133137
const pool = new pg.Pool(config)
134-
let res = await poolerQueryHandleError(pool, sqlWithStatementTimeout)
138+
let res = await poolerQueryHandleError(pool, sqlWithStatementTimeout, parameters)
135139
if (Array.isArray(res)) {
136140
res = res.reverse().find((x) => x.rows.length !== 0) ?? { rows: [] }
137141
}
138142
await pool.end()
139143
return { data: res.rows, error: null }
140144
}
141145

142-
let res = await poolerQueryHandleError(pool, sqlWithStatementTimeout)
146+
let res = await poolerQueryHandleError(pool, sqlWithStatementTimeout, parameters)
143147
if (Array.isArray(res)) {
144148
res = res.reverse().find((x) => x.rows.length !== 0) ?? { rows: [] }
145149
}

‎src/server/routes/query.ts‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,8 @@ const errorOnEmptyQuery = (request: FastifyRequest) => {
1616
export default async (fastify: FastifyInstance) => {
1717
fastify.post<{
1818
Headers: { pg: string; 'x-pg-application-name'?: string }
19-
Body: {
20-
query: string
21-
}
22-
Querystring: {
23-
statementTimeoutSecs?: number
24-
}
19+
Body: { query: string; parameters?: unknown[] }
20+
Querystring: { statementTimeoutSecs?: number }
2521
}>('/', async (request, reply) => {
2622
const statementTimeoutSecs = request.query.statementTimeoutSecs
2723
errorOnEmptyQuery(request)
@@ -30,6 +26,7 @@ export default async (fastify: FastifyInstance) => {
3026
const { data, error } = await pgMeta.query(request.body.query, {
3127
trackQueryInSentry: true,
3228
statementQueryTimeout: statementTimeoutSecs,
29+
parameters: request.body.parameters,
3330
})
3431
await pgMeta.end()
3532
if (error) {
@@ -43,9 +40,7 @@ export default async (fastify: FastifyInstance) => {
4340

4441
fastify.post<{
4542
Headers: { pg: string; 'x-pg-application-name'?: string }
46-
Body: {
47-
query: string
48-
}
43+
Body: { query: string }
4944
}>('/format', async (request, reply) => {
5045
errorOnEmptyQuery(request)
5146
const { data, error } = await Parser.Format(request.body.query)
@@ -61,9 +56,7 @@ export default async (fastify: FastifyInstance) => {
6156

6257
fastify.post<{
6358
Headers: { pg: string; 'x-pg-application-name'?: string }
64-
Body: {
65-
query: string
66-
}
59+
Body: { query: string }
6760
}>('/parse', async (request, reply) => {
6861
errorOnEmptyQuery(request)
6962
const { data, error } = Parser.Parse(request.body.query)
@@ -79,9 +72,7 @@ export default async (fastify: FastifyInstance) => {
7972

8073
fastify.post<{
8174
Headers: { pg: string; 'x-pg-application-name'?: string }
82-
Body: {
83-
ast: object
84-
}
75+
Body: { ast: object }
8576
}>('/deparse', async (request, reply) => {
8677
const { data, error } = Parser.Deparse(request.body.ast)
8778

‎test/server/query.ts‎

Lines changed: 95 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -547,9 +547,7 @@ test('return interval as string', async () => {
547547
const res = await app.inject({
548548
method: 'POST',
549549
path: '/query',
550-
payload: {
551-
query: `SELECT '1 day 1 hour 45 minutes'::interval`,
552-
},
550+
payload: { query: `SELECT '1 day 1 hour 45 minutes'::interval` },
553551
})
554552
expect(res.json()).toMatchInlineSnapshot(`
555553
[
@@ -703,9 +701,7 @@ test('error with internalQuery property', async () => {
703701
const res = await app.inject({
704702
method: 'POST',
705703
path: '/query',
706-
payload: {
707-
query: 'SELECT test_internal_query();',
708-
},
704+
payload: { query: 'SELECT test_internal_query();' },
709705
})
710706

711707
expect(res.json()).toMatchInlineSnapshot(`
@@ -737,19 +733,107 @@ test('custom application_name', async () => {
737733
const res = await app.inject({
738734
method: 'POST',
739735
path: '/query',
740-
headers: {
741-
'x-pg-application-name': 'test',
742-
},
736+
headers: { 'x-pg-application-name': 'test' },
737+
payload: { query: 'SHOW application_name;' },
738+
})
739+
740+
expect(res.json()).toMatchInlineSnapshot(`
741+
[
742+
{
743+
"application_name": "test",
744+
},
745+
]
746+
`)
747+
})
748+
749+
test('parameter binding with positional parameters', async () => {
750+
const res = await app.inject({
751+
method: 'POST',
752+
path: '/query',
743753
payload: {
744-
query: 'SHOW application_name;',
754+
query: 'SELECT * FROM users WHERE id = $1 AND status = $2',
755+
parameters: [1, 'ACTIVE'],
745756
},
746757
})
758+
expect(res.json()).toMatchInlineSnapshot(`
759+
[
760+
{
761+
"decimal": null,
762+
"id": 1,
763+
"name": "Joe Bloggs",
764+
"status": "ACTIVE",
765+
},
766+
]
767+
`)
768+
})
747769

770+
test('parameter binding with single parameter', async () => {
771+
const res = await app.inject({
772+
method: 'POST',
773+
path: '/query',
774+
payload: { query: 'SELECT name FROM users WHERE id = $1', parameters: [2] },
775+
})
748776
expect(res.json()).toMatchInlineSnapshot(`
749777
[
750778
{
751-
"application_name": "test",
779+
"name": "Jane Doe",
752780
},
753781
]
754782
`)
755783
})
784+
785+
test('parameter binding with no matches', async () => {
786+
const res = await app.inject({
787+
method: 'POST',
788+
path: '/query',
789+
payload: { query: 'SELECT * FROM users WHERE id = $1', parameters: [999] },
790+
})
791+
expect(res.json()).toMatchInlineSnapshot(`[]`)
792+
})
793+
794+
test('no parameters field', async () => {
795+
const res = await app.inject({
796+
method: 'POST',
797+
path: '/query',
798+
payload: { query: 'SELECT COUNT(*) as count FROM users' },
799+
})
800+
expect(res.json()).toMatchInlineSnapshot(`
801+
[
802+
{
803+
"count": 2,
804+
},
805+
]
806+
`)
807+
})
808+
809+
test('parameter binding with empty parameters array', async () => {
810+
const res = await app.inject({
811+
method: 'POST',
812+
path: '/query',
813+
payload: { query: 'SELECT COUNT(*) as count FROM users', parameters: [] },
814+
})
815+
expect(res.json()).toMatchInlineSnapshot(`
816+
[
817+
{
818+
"count": 2,
819+
},
820+
]
821+
`)
822+
})
823+
824+
test('parameter binding error - wrong parameter count', async () => {
825+
const res = await app.inject({
826+
method: 'POST',
827+
path: '/query',
828+
payload: {
829+
query: 'SELECT * FROM users WHERE id = $1 AND status = $2',
830+
parameters: [1], // Missing second parameter
831+
},
832+
})
833+
expect(res.statusCode).toBe(400)
834+
const json = res.json()
835+
expect(json.code).toBe('08P01')
836+
expect(json.message).toContain(
837+
'bind message supplies 1 parameters, but prepared statement "" requires 2'
838+
)
839+
})

‎vitest.config.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@ import { defineConfig } from 'vitest/config'
33

44
export default defineConfig({
55
test: {
6-
coverage: {
7-
reporter: ['lcov'],
8-
},
6+
coverage: { reporter: ['lcov'] },
97
maxConcurrency: 1,
108
// https://github.com/vitest-dev/vitest/issues/317#issuecomment-1542319622
119
pool: 'forks',

0 commit comments

Comments
 (0)