yujunjun/knex
1
0
Fork 0
mirror of https://github.com/knex/knex.git synced 2025-06-26 22:00:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(oracledb): withRecursive: omit invalid RECURSIVE keyword, include column list [#4514] (#4652)

Browse Source
This commit is contained in:
Jeremy W. Sherman 2021-09-03 15:27:32 -04:00 committed by GitHub
parent ce35bd6b64
commit a9338208f1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 197 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
lib/dialects/oracledb/query/oracledb-querycompiler.js
View File

@ -201,6 +201,29 @@ class Oracledb_Compiler extends Oracle_Compiler {
return sql;
}
with() {
// WITH RECURSIVE is a syntax error in Oracle SQL.
// So mark all statements as non-recursive, generate the SQL, then restore.
// This approach ensures any changes in base class with() get propagated here.
const undoList = [];
if (this.grouped.with) {
for (const stmt of this.grouped.with) {
if (stmt.recursive) {
undoList.push(stmt);
stmt.recursive = false;
}
}
}
const result = super.with();
// Restore the recursive markings, in case this same query gets cloned and passed to other drivers.
for (const stmt of undoList) {
stmt.recursive = true;
}
return result;
}
_addReturningToSqlAndConvert(sql, outBinding, tableName, returning) {
const self = this;
const res = {

73
lib/query/querybuilder.js
View File

@ -114,17 +114,22 @@ class Builder extends EventEmitter {
// With
// ------
with(alias, statement) {
validateWithArgs(alias, statement, 'with');
return this.withWrapped(alias, statement);
with(alias, statementOrColumnList, nothingOrStatement) {
validateWithArgs(alias, statementOrColumnList, nothingOrStatement, 'with');
return this.withWrapped(alias, statementOrColumnList, nothingOrStatement);
}
// Helper for compiling any advanced `with` queries.
withWrapped(alias, query) {
withWrapped(alias, statementOrColumnList, nothingOrStatement) {
const [query, columnList] =
typeof nothingOrStatement === 'undefined'
? [statementOrColumnList, undefined]
: [nothingOrStatement, statementOrColumnList];
this._statements.push({
grouping: 'with',
type: 'withWrapped',
alias: alias,
columnList,
value: query,
});
return this;
@ -133,14 +138,23 @@ class Builder extends EventEmitter {
// With Recursive
// ------
withRecursive(alias, statement) {
validateWithArgs(alias, statement, 'withRecursive');
return this.withRecursiveWrapped(alias, statement);
withRecursive(alias, statementOrColumnList, nothingOrStatement) {
validateWithArgs(
alias,
statementOrColumnList,
nothingOrStatement,
'withRecursive'
);
return this.withRecursiveWrapped(
alias,
statementOrColumnList,
nothingOrStatement
);
}
// Helper for compiling any advanced `withRecursive` queries.
withRecursiveWrapped(alias, query) {
this.withWrapped(alias, query);
withRecursiveWrapped(alias, statementOrColumnList, nothingOrStatement) {
this.withWrapped(alias, statementOrColumnList, nothingOrStatement);
this._statements[this._statements.length - 1].recursive = true;
return this;
}
@ -1392,19 +1406,46 @@ class Builder extends EventEmitter {
}
}
const validateWithArgs = function (alias, statement, method) {
const isValidStatementArg = (statement) =>
typeof statement === 'function' ||
statement instanceof Builder ||
(statement && statement.isRawInstance);
const validateWithArgs = function (
alias,
statementOrColumnList,
nothingOrStatement,
method
) {
const [query, columnList] =
typeof nothingOrStatement === 'undefined'
? [statementOrColumnList, undefined]
: [nothingOrStatement, statementOrColumnList];
if (typeof alias !== 'string') {
throw new Error(`${method}() first argument must be a string`);
}
if (
typeof statement === 'function' ||
statement instanceof Builder ||
(statement && statement.isRawInstance)
) {
if (isValidStatementArg(query) && typeof columnList === 'undefined') {
// Validated as two-arg variant (alias, statement).
return;
}
// Attempt to interpret as three-arg variant (alias, columnList, statement).
const isNonEmptyNameList =
Array.isArray(columnList) &&
columnList.length > 0 &&
columnList.every((it) => typeof it === 'string');
if (!isNonEmptyNameList) {
throw new Error(
`${method}() second argument must be a statement or non-empty column name list.`
);
}
if (isValidStatementArg(query)) {
return;
}
throw new Error(
`${method}() second argument must be a function / QueryBuilder or a raw`
`${method}() third argument must be a function / QueryBuilder or a raw when its second argument is a column name list`
);
};

11
lib/query/querycompiler.js
View File

@ -1087,6 +1087,16 @@ class QueryCompiler {
this.client,
this.bindingsHolder
);
const columnList = statement.columnList
? '(' +
columnize_(
statement.columnList,
this.builder,
this.client,
this.bindingsHolder
) +
')'
: '';
return (
(val &&
columnize_(
@ -1095,6 +1105,7 @@ class QueryCompiler {
this.client,
this.bindingsHolder
) +
columnList +
' as (' +
val +
')') ||

3
package.json
View File

@ -26,6 +26,7 @@
"test:mssql": "cross-env DB=mssql npm run test:db",
"test:mysql": "cross-env DB=mysql npm run test:db",
"test:mysql2": "cross-env DB=mysql2 npm run test:db",
"test:oracledb": "cross-env DB=oracledb npm run test:db",
"test:sqlite": "cross-env DB=sqlite3 npm run test:db",
"test:postgres": "cross-env DB=postgres npm run test:db",
"test:tape": "node test/tape/index.js | tap-spec",
@ -37,6 +38,8 @@
"db:stop:postgres": "docker-compose -f scripts/docker-compose.yml down",
"db:start:mysql": "docker-compose -f scripts/docker-compose.yml up --build -d mysql && docker-compose -f scripts/docker-compose.yml up waitmysql",
"db:stop:mysql": "docker-compose -f scripts/docker-compose.yml down",
"db:start:oracle": "docker-compose -f scripts/docker-compose.yml up --build -d oracledbxe && docker-compose -f scripts/docker-compose.yml up waitoracledbxe",
"db:stop:oracle": "docker-compose -f scripts/docker-compose.yml down",
"stress:init": "docker-compose -f scripts/stress-test/docker-compose.yml up --no-start && docker-compose -f scripts/stress-test/docker-compose.yml start",
"stress:test": "node scripts/stress-test/knex-stress-test.js | grep -A 5 -B 60 -- '- STATS '",
"stress:destroy": "docker-compose -f scripts/stress-test/docker-compose.yml stop"

33
test-tsd/types.test-d.ts
View File

@ -115,3 +115,36 @@ expectAssignable<QueryBuilder>(
.merge(['active', 'id'])
.debug(true)
);
knexInstance.withWrapped('qb', knexInstance.select('column').from('table'))
knexInstance.withWrapped('callback', (qb) => qb.select('column').from('table'))
knexInstance.withWrapped('columnList+qb', ['columnName'], (qb) => qb.select('column').from('table'))
knexInstance.withWrapped('columnList+callback', ['columnName'], (qb) => qb.select('column').from('table'))
// FIXME: The withRaw function does not exist any more. with handles raw directly now.
knexInstance.withRaw('raw', knexInstance.raw('raw'))
knexInstance.withRaw('sql', 'just sql')
knexInstance.withRaw('sql+bindingsObj', 'sql with named bindings', {x: 1})
knexInstance.withRaw('sql+bindingsArr', 'sql with positional bindings', [1])
knexInstance.withRaw('columnList+raw', ['columnName'], knexInstance.raw('raw'))
knexInstance.withRaw('columnList+sql', ['columnName'], 'just sql')
knexInstance.withRaw('columnList+sql+bindingsObj', ['columnName'], 'sql with named bindings', {x: 1})
knexInstance.withRaw('columnList+sql+bindingsArr', ['columnName'], 'sql with positional bindings', [1])
// the With type is used both for with and withRecursive. With extends both withWrapped and withRaw, so should support all the same variants:
// those inherited from WithWrapped
knexInstance.with('qb', knexInstance.select('column').from('table'))
knexInstance.with('callback', (qb) => qb.select('column').from('table'))
knexInstance.with('columnList+qb', ['columnName'], (qb) => qb.select('column').from('table'))
knexInstance.with('columnList+callback', ['columnName'], (qb) => qb.select('column').from('table'))
// those inherited from withRaw
knexInstance.with('raw', knexInstance.raw('raw'))
knexInstance.with('sql', 'just sql')
knexInstance.with('sql+bindingsObj', 'sql with named bindings', {x: 1})
knexInstance.with('sql+bindingsArr', 'sql with positional bindings', [1])
knexInstance.with('columnList+raw', ['columnName'], knexInstance.raw('raw'))
knexInstance.with('columnList+sql', ['columnName'], 'just sql')
knexInstance.with('columnList+sql+bindingsObj', ['columnName'], 'sql with named bindings', {x: 1})
knexInstance.with('columnList+sql+bindingsArr', ['columnName'], 'sql with positional bindings', [1])

64
test/integration/query/selects.js
View File

@ -1215,42 +1215,52 @@ module.exports = function (knex) {
});
describe('recursive CTE support', function () {
before(async function() {
await knex.schema.dropTableIfExists('rcte')
before(async function () {
await knex.schema.dropTableIfExists('rcte');
await knex.schema.createTable('rcte', (table) => {
table.string('name')
table.string('parentName').nullable()
})
table.string('name');
table.string('parentName').nullable();
});
// We will check later that this name was found by chaining up parentId using an rCTE.
await knex('rcte').insert({name: 'parent'})
let parentName = 'parent'
await knex('rcte').insert({ name: 'parent' });
let parentName = 'parent';
for (const name of ['child', 'grandchild']) {
await knex('rcte').insert({name, parentName})
parentName = name
await knex('rcte').insert({ name, parentName });
parentName = name;
}
// We will check later that this name is not returned.
await knex('rcte').insert({name: 'nope'})
})
await knex('rcte').insert({ name: 'nope' });
});
it('supports recursive CTEs', async function () {
// FIXME: Oracle requires to omit RECURSIVE. [#4514]
if (isOracle(knex)) {
return this.skip()
}
const results = await knex
.withRecursive('family', ['name', 'parentName'], (qb) => {
qb.select('name', 'parentName')
.from('rcte')
.where({ name: 'grandchild' })
.unionAll((qb) =>
qb
.select('rcte.name', 'rcte.parentName')
.from('rcte')
.join(
'family',
knex.ref('family.parentName'),
knex.ref('rcte.name')
)
);
})
.select('name')
.from('family');
const names = results.map(({ name }) => name);
const results = await knex.withRecursive('family', (qb) => {
qb.select('name', 'parentName').from('rcte').where({name: 'grandchild'}).unionAll(
(qb) => qb.select('rcte.name', 'rcte.parentName').from('rcte').join('family', knex.ref('family.parentName'), knex.ref('rcte.name'))
)
}).select('name').from('family')
const names = results.map(({name}) => name)
expect(names).to.have.length('parent child grandchild'.split(' ').length)
expect(names).to.contain('parent')
expect(names).not.to.contain('nope')
})
})
expect(names).to.have.length(
'parent child grandchild'.split(' ').length
);
expect(names).to.contain('parent');
expect(names).not.to.contain('nope');
});
});
it('supports the <> operator', function () {
return knex('accounts').where('id', '<>', 2).select('email', 'logins');

23
test/unit/query/builder.js
View File

@ -9401,7 +9401,28 @@ describe('QueryBuilder', () => {
// FIXME: oracledb does not allow the RECURSIVE keyword, but does require a list of column aliases for a recursive query. [#4514]
// https://github.com/knex/knex/issues/4514#issuecomment-903727391
oracledb:
'with recursive "firstWithClause" as (with recursive "firstWithSubClause" as ((select "foo" from "users") "foz") select * from "firstWithSubClause"), "secondWithClause" as (with recursive "secondWithSubClause" as ((select "bar" from "users") "baz") select * from "secondWithSubClause") select * from "secondWithClause"',
'with "firstWithClause" as (with "firstWithSubClause" as ((select "foo" from "users") "foz") select * from "firstWithSubClause"), "secondWithClause" as (with "secondWithSubClause" as ((select "bar" from "users") "baz") select * from "secondWithSubClause") select * from "secondWithClause"',
}
);
});
it('Oracle: withRecursive with column list', function () {
testsql(
qb()
.withRecursive('hasColumns', ['id', 'nickname'], function () {
this.select('id', 'nickname')
.from('users')
.unionAll(function () {
this.select('id', 'firstname')
.from('users')
.join('hasColumns', 'hasColumns.nickname', 'users.firstname');
});
})
.select('name')
.from('hasColumns'),
{
oracledb:
'with "hasColumns"("id", "nickname") as (select "id", "nickname" from "users" union all select "id", "firstname" from "users" inner join "hasColumns" on "hasColumns"."nickname" = "users"."firstname") select "name" from "hasColumns"',
}
);
});

11
types/index.d.ts vendored
View File

@ -1275,6 +1275,11 @@ export declare namespace Knex {
TRecord,
TResult
>;
(alias: string, columnList: string[], raw: Raw | QueryBuilder): QueryBuilder<TRecord, TResult>;
(alias: string, columnList: string[], sql: string, bindings?: readonly Value[] | Object): QueryBuilder<
TRecord,
TResult
>;
}
interface WithSchema<TRecord = any, TResult = unknown[]> {
@ -1287,6 +1292,12 @@ export declare namespace Knex {
alias: string,
callback: (queryBuilder: QueryBuilder) => any
): QueryBuilder<TRecord, TResult>;
(alias: string, columnList: string[], queryBuilder: QueryBuilder): QueryBuilder<TRecord, TResult>;
(
alias: string,
columnList: string[],
callback: (queryBuilder: QueryBuilder) => any
): QueryBuilder<TRecord, TResult>;
}
interface Where<TRecord = any, TResult = unknown>