Skip to content

Commit 56c5953

Browse files
committed
properly parse select subqueries
1 parent 620a4fb commit 56c5953

File tree

2 files changed

+56
-4
lines changed

2 files changed

+56
-4
lines changed

lib/db/query-parsers/sql.js

+20-4
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,29 @@
55

66
'use strict'
77

8-
const logger = require('../../logger').child({ component: 'sql_query_parser' })
8+
const defaultLogger = require('../../logger').child({ component: 'sql_query_parser' })
99
const stringify = require('json-stringify-safe')
1010

11+
/**
12+
* In a query like `select * from (select * from foo)`, extract the subquery
13+
* as the statement to retrieve the target identifier from.
14+
*
15+
* @type {RegExp}
16+
*/
17+
const selectSubquery = /from\s+?\((?<subquery>select.*?)\)(?:\s+?)?/gi
18+
1119
/**
1220
* Parses a SQL statement into the parts we want to report as metadata in
1321
* database transactions.
1422
*
1523
* @param {string} sql The statement to parse.
24+
* @param {object} [deps] A set of optional dependencies.
25+
* @param {object} [deps.logger] A logger instance.
1626
*
1727
* @returns {{query: string, collection: null|string, operation: string}} Parsed
1828
* metadata.
1929
*/
20-
module.exports = function parseSql(sql) {
30+
module.exports = function parseSql(sql, { logger = defaultLogger } = {}) {
2131
// Sometimes we get an object here from MySQL. We have been unable to
2232
// reproduce it, so we'll just log what that object is and return a statement
2333
// type of `other`.
@@ -27,7 +37,7 @@ module.exports = function parseSql(sql) {
2737
if (typeof sql !== 'string') {
2838
if (logger.traceEnabled()) {
2939
try {
30-
logger.trace('parseSQL got an a non-string sql that looks like: %s', stringify(sql))
40+
logger.trace('parseSQL got a non-string sql that looks like: %s', stringify(sql))
3141
} catch (err) {
3242
logger.debug(err, 'Unable to stringify SQL')
3343
}
@@ -166,10 +176,17 @@ function parseStatement(statement, kind = 'insert') {
166176
}
167177

168178
case 'select': {
179+
const subqueryMatch = selectSubquery.exec(statement)
180+
if (subqueryMatch !== null) {
181+
statement = subqueryMatch.groups.subquery
182+
}
183+
169184
if (/\bfrom\b/i.test(statement) === false) {
170185
// Statement does not specify a table. We don't need further processing.
186+
// E.g., we have a statement like `select 1 + 1 as added`.
171187
return { collection: 'unknown', table: 'unknown' }
172188
}
189+
173190
splitter = /\s*?\bfrom\b\s*?/i
174191
break
175192
}
@@ -240,7 +257,6 @@ function normalizeTableName(tableIdentifier) {
240257
if (parts.length === 1) {
241258
return parts[0]
242259
}
243-
return '(subquery)'
244260
}
245261

246262
const parenPos = tableIdentifier.indexOf('(')

test/unit/db/query-parsers/sql.test.js

+36
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,35 @@ test('database query parser', async (t) => {
164164
})
165165
})
166166

167+
test('logs correctly if input is incorrect', () => {
168+
let logs = []
169+
const logger = {
170+
traceEnabled() {
171+
return true
172+
},
173+
trace(msg, data) {
174+
logs.push([msg, data])
175+
},
176+
debug(error, msg) {
177+
logs.push([error, msg])
178+
}
179+
}
180+
181+
let result = parseSql({ an: 'invalid object' }, { logger })
182+
assert.deepStrictEqual(result, { operation: 'other', collection: null, query: '' })
183+
assert.deepStrictEqual(logs, [
184+
['parseSQL got a non-string sql that looks like: %s', `{"an":"invalid object"}`]
185+
])
186+
187+
logs = []
188+
logger.trace = () => {
189+
throw Error('boom')
190+
}
191+
result = parseSql({ an: 'invalid object' }, { logger })
192+
assert.deepStrictEqual(result, { operation: 'other', collection: null, query: '' })
193+
assert.equal(logs[0][1], 'Unable to stringify SQL')
194+
})
195+
167196
// test('reports correct info if inline comments present', () => {
168197
// const statement = `--insert into`
169198
// })
@@ -299,3 +328,10 @@ test('handles odd characters attached to table names', () => {
299328
found = parseSql(statement)
300329
match(found, expected)
301330
})
331+
332+
test('handles subqueries in place of table identifiers', () => {
333+
const expected = { operation: 'select', collection: 'foo', table: 'foo' }
334+
const statement = 'select * from (select foo from foo) where bar = "baz"'
335+
const found = parseSql(statement)
336+
match(found, expected)
337+
})

0 commit comments

Comments
 (0)