Skip to content

Commit

Permalink
fix: materialized hints support for cte (#9605)
Browse files Browse the repository at this point in the history
Fix implementation of materialized hints in common table expressions
Previous behavior did not account for NOT MATERIALIZED hints, also
placed materialized hints in wrong place (before "AS")

Co-authored-by: Adrian Parry <adrian.parry@reign.cl>
  • Loading branch information
subparry and adrianparry committed Dec 29, 2022
1 parent 8668c29 commit 67973b4
Show file tree
Hide file tree
Showing 2 changed files with 149 additions and 5 deletions.
14 changes: 9 additions & 5 deletions src/query-builder/QueryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1117,17 +1117,21 @@ export abstract class QueryBuilder<Entity extends ObjectLiteral> {
cte.options.recursive && databaseRequireRecusiveHint
? "RECURSIVE"
: ""
const materializeClause =
cte.options.materialized &&
this.connection.driver.cteCapabilities.materializedHint
let materializeClause = ""
if (
this.connection.driver.cteCapabilities.materializedHint &&
cte.options.materialized !== undefined
) {
materializeClause = cte.options.materialized
? "MATERIALIZED"
: ""
: "NOT MATERIALIZED"
}

return [
recursiveClause,
cteHeader,
materializeClause,
"AS",
materializeClause,
`(${cteBodyExpression})`,
]
.filter(Boolean)
Expand Down
140 changes: 140 additions & 0 deletions test/functional/query-builder/cte/materialized-cte.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import "reflect-metadata"
import { expect } from "chai"
import {
createTestingConnections,
closeTestingConnections,
reloadTestingDatabases,
} from "../../../utils/test-utils"
import { Connection } from "../../../../src/connection/Connection"
import { Foo } from "./entity/foo"
import { filterByCteCapabilities } from "./helpers"
import { QueryBuilderCteOptions } from "../../../../src/query-builder/QueryBuilderCte"

describe("query builder > cte > materialized", () => {
let connections: Connection[]
before(
async () =>
(connections = await createTestingConnections({
entities: [__dirname + "/entity/*{.js,.ts}"],
schemaCreate: true,
dropSchema: true,
// enabledDrivers: [']
})),
)
beforeEach(() => reloadTestingDatabases(connections))
after(() => closeTestingConnections(connections))

it("should allow MATERIALIZED hint", () =>
Promise.all(
connections
.filter(filterByCteCapabilities("enabled"))
.filter(filterByCteCapabilities("materializedHint"))
.map(async (connection) => {
await connection
.getRepository(Foo)
.insert(
[1, 2, 3].map((i) => ({ id: i, bar: String(i) })),
)
const cteQuery = connection
.createQueryBuilder()
.select()
.addSelect(`foo.bar`, "bar")
.from(Foo, "foo")
.where(`foo.bar = :value`, { value: "2" })

const cteOptions: QueryBuilderCteOptions = {
columnNames: ["raz"],
materialized: true,
}

const cteSelection = "qaz.raz"

const qb = await connection
.createQueryBuilder()
.addCommonTableExpression(cteQuery, "qaz", cteOptions)
.from("qaz", "qaz")
.select([])
.addSelect(cteSelection, "raz")

expect(qb.getQuery()).to.contain(
`WITH "qaz"("raz") AS MATERIALIZED (`,
)
expect(await qb.getRawMany()).to.deep.equal([{ raz: "2" }])
}),
))

it("should allow NOT MATERIALIZED hint", () =>
Promise.all(
connections
.filter(filterByCteCapabilities("enabled"))
.filter(filterByCteCapabilities("materializedHint"))
.map(async (connection) => {
await connection
.getRepository(Foo)
.insert(
[1, 2, 3].map((i) => ({ id: i, bar: String(i) })),
)
const cteQuery = connection
.createQueryBuilder()
.select()
.addSelect(`foo.bar`, "bar")
.from(Foo, "foo")
.where(`foo.bar = :value`, { value: "2" })

const cteOptions: QueryBuilderCteOptions = {
columnNames: ["raz"],
materialized: false,
}

const cteSelection = "qaz.raz"

const qb = await connection
.createQueryBuilder()
.addCommonTableExpression(cteQuery, "qaz", cteOptions)
.from("qaz", "qaz")
.select([])
.addSelect(cteSelection, "raz")

expect(qb.getQuery()).to.contain(
`WITH "qaz"("raz") AS NOT MATERIALIZED (`,
)
expect(await qb.getRawMany()).to.deep.equal([{ raz: "2" }])
}),
))

it("should omit hint if materialized option is not set", () =>
Promise.all(
connections
.filter(filterByCteCapabilities("enabled"))
.filter(filterByCteCapabilities("materializedHint"))
.map(async (connection) => {
await connection
.getRepository(Foo)
.insert(
[1, 2, 3].map((i) => ({ id: i, bar: String(i) })),
)
const cteQuery = connection
.createQueryBuilder()
.select()
.addSelect(`foo.bar`, "bar")
.from(Foo, "foo")
.where(`foo.bar = :value`, { value: "2" })

const cteOptions: QueryBuilderCteOptions = {
columnNames: ["raz"],
}

const cteSelection = "qaz.raz"

const qb = await connection
.createQueryBuilder()
.addCommonTableExpression(cteQuery, "qaz", cteOptions)
.from("qaz", "qaz")
.select([])
.addSelect(cteSelection, "raz")

expect(qb.getQuery()).to.contain(`WITH "qaz"("raz") AS (`)
expect(await qb.getRawMany()).to.deep.equal([{ raz: "2" }])
}),
))
})

0 comments on commit 67973b4

Please sign in to comment.