Skip to content
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

SQL - use CTEs to avoid pagination issues with large tables #14557

Merged
merged 12 commits into from
Sep 11, 2024
Merged
2 changes: 1 addition & 1 deletion hosting/docker-compose.dev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ services:
couchdb-service:
container_name: budi-couchdb3-dev
restart: on-failure
image: budibase/couchdb:v3.3.3
image: budibase/couchdb:v3.3.3-sqs-v2.1.1
environment:
- COUCHDB_PASSWORD=${COUCH_DB_PASSWORD}
- COUCHDB_USER=${COUCH_DB_USER}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
"build:docker:single": "./scripts/build-single-image.sh",
"build:docker:single:sqs": "./scripts/build-single-image-sqs.sh",
"build:docker:dependencies": "docker build -f hosting/dependencies/Dockerfile -t budibase/dependencies:latest ./hosting",
"publish:docker:couch": "docker buildx build --platform linux/arm64,linux/amd64 -f hosting/couchdb/Dockerfile -t budibase/couchdb:latest -t budibase/couchdb:v3.3.3 --push ./hosting/couchdb",
"publish:docker:couch": "docker buildx build --platform linux/arm64,linux/amd64 -f hosting/couchdb/Dockerfile -t budibase/couchdb:latest -t budibase/couchdb:v3.3.3 -t budibase/couchdb:v3.3.3-sqs-v2.1.1 --push ./hosting/couchdb",
"publish:docker:dependencies": "docker buildx build --platform linux/arm64,linux/amd64 -f hosting/dependencies/Dockerfile -t budibase/dependencies:latest -t budibase/dependencies:v3.2.1 --push ./hosting",
"release:helm": "node scripts/releaseHelmChart",
"env:multi:enable": "lerna run --stream env:multi:enable",
Expand Down
105 changes: 39 additions & 66 deletions packages/backend-core/src/sql/sql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,21 @@ class InternalBuilder {
return withSchema
}

private buildJsonField(field: string): string {
const parts = field.split(".")
let tableField: string, unaliased: string
if (parts.length > 1) {
const alias = parts.shift()!
unaliased = parts.join(".")
tableField = `${this.quote(alias)}.${this.quote(unaliased)}`
} else {
unaliased = parts.join(".")
PClmnt marked this conversation as resolved.
Show resolved Hide resolved
tableField = this.quote(unaliased)
}
const separator = this.client === SqlClient.ORACLE ? " VALUE " : ","
return `'${unaliased}'${separator}${tableField}`
}

addJsonRelationships(
query: Knex.QueryBuilder,
fromTable: string,
Expand All @@ -871,27 +886,6 @@ class InternalBuilder {
const knex = this.knex
const { resource, tableAliases: aliases, endpoint } = this.query
const fields = resource?.fields || []
const jsonField = (field: string) => {
const parts = field.split(".")
let tableField: string, unaliased: string
if (parts.length > 1) {
const alias = parts.shift()!
unaliased = parts.join(".")
tableField = `${this.quote(alias)}.${this.quote(unaliased)}`
} else {
unaliased = parts.join(".")
tableField = this.quote(unaliased)
}
let separator = ","
switch (sqlClient) {
case SqlClient.ORACLE:
separator = " VALUE "
break
case SqlClient.MS_SQL:
separator = ":"
}
return `'${unaliased}'${separator}${tableField}`
}
for (let relationship of relationships) {
const {
tableName: toTable,
Expand Down Expand Up @@ -921,7 +915,7 @@ class InternalBuilder {
)
}
const fieldList: string = relationshipFields
.map(field => jsonField(field))
.map(field => this.buildJsonField(field))
.join(",")
// SQL Server uses TOP - which performs a little differently to the normal LIMIT syntax
// it reduces the result set rather than limiting how much data it filters over
Expand Down Expand Up @@ -1073,43 +1067,6 @@ class InternalBuilder {
return query
}

addRelationships(
query: Knex.QueryBuilder,
fromTable: string,
relationships: RelationshipsJson[]
): Knex.QueryBuilder {
const tableSets: Record<string, [RelationshipsJson]> = {}
// aggregate into table sets (all the same to tables)
for (let relationship of relationships) {
const keyObj: { toTable: string; throughTable: string | undefined } = {
toTable: relationship.tableName,
throughTable: undefined,
}
if (relationship.through) {
keyObj.throughTable = relationship.through
}
const key = JSON.stringify(keyObj)
if (tableSets[key]) {
tableSets[key].push(relationship)
} else {
tableSets[key] = [relationship]
}
}
for (let [key, relationships] of Object.entries(tableSets)) {
const { toTable, throughTable } = JSON.parse(key)
query = this.addJoin(
query,
{
from: fromTable,
to: toTable,
through: throughTable,
},
relationships
)
}
return query
}

qualifiedKnex(opts?: { alias?: string | boolean }): Knex.QueryBuilder {
let alias = this.query.tableAliases?.[this.query.endpoint.entityId]
if (opts?.alias === false) {
Expand Down Expand Up @@ -1193,8 +1150,7 @@ class InternalBuilder {
if (!primary) {
throw new Error("Primary key is required for upsert")
}
const ret = query.insert(parsedBody).onConflict(primary).merge()
return ret
return query.insert(parsedBody).onConflict(primary).merge()
} else if (
this.client === SqlClient.MS_SQL ||
this.client === SqlClient.ORACLE
Expand Down Expand Up @@ -1253,12 +1209,29 @@ class InternalBuilder {
if (!counting) {
query = this.addSorting(query)
}
// handle joins
if (relationships) {
query = this.addJsonRelationships(query, tableName, relationships)
}

return this.addFilters(query, filters, { relationship: true })
query = this.addFilters(query, filters, { relationship: true })

// handle relationships with a CTE for all others
if (relationships?.length) {
const mainTable =
this.query.tableAliases?.[this.query.endpoint.entityId] ||
this.query.endpoint.entityId
const cte = this.addSorting(
this.knex
.with("paginated", query)
.select(this.generateSelectStatement())
.from({
[mainTable]: "paginated",
})
)
// add JSON aggregations attached to the CTE
return this.addJsonRelationships(cte, tableName, relationships)
}
// no relationships found - return query
else {
return query
}
}

update(opts: QueryOptions): Knex.QueryBuilder {
Expand Down
49 changes: 23 additions & 26 deletions packages/server/src/api/controllers/row/utils/basic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,35 +145,32 @@ export function basicProcessing({
: typeof value === "string"
? JSON.parse(value)
: undefined
if (array) {
if (array && Array.isArray(array)) {
thisRow[col] = array
// make sure all of them have an _id
if (Array.isArray(thisRow[col])) {
const sortField =
relatedTable.primaryDisplay || relatedTable.primary![0]!
thisRow[col] = (thisRow[col] as Row[])
.map(relatedRow =>
basicProcessing({
row: relatedRow,
table: relatedTable,
tables,
isLinked: false,
sqs,
})
)
.sort((a, b) => {
const aField = a?.[sortField],
bField = b?.[sortField]
if (!aField) {
return 1
} else if (!bField) {
return -1
}
return aField.localeCompare
? aField.localeCompare(bField)
: aField - bField
const sortField = relatedTable.primaryDisplay || relatedTable.primary![0]!
thisRow[col] = (thisRow[col] as Row[])
.map(relatedRow =>
basicProcessing({
row: relatedRow,
table: relatedTable,
tables,
isLinked: false,
sqs,
})
}
)
.sort((a, b) => {
const aField = a?.[sortField],
bField = b?.[sortField]
if (!aField) {
return 1
} else if (!bField) {
return -1
}
return aField.localeCompare
? aField.localeCompare(bField)
: aField - bField
})
}
}
return fixJsonTypes(thisRow, table)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -832,12 +832,13 @@ describe.each(
},
})
expect(res).toHaveLength(1)
expect(res[0]).toEqual(
expect.objectContaining({
id: 2,
name: "two",
})
)
expect(res[0]).toEqual({
id: 2,
name: "two",
// the use of table.* introduces the possibility of nulls being returned
birthday: null,
number: null,
})
})

// this parameter really only impacts SQL queries
Expand Down
12 changes: 6 additions & 6 deletions packages/server/src/integrations/tests/sql.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,16 +161,16 @@ describe("SQL query builder", () => {
it("should add the schema to the LEFT JOIN", () => {
const query = sql._query(generateRelationshipJson({ schema: "production" }))
expect(query).toEqual({
bindings: [relationshipLimit, limit],
sql: `select "brands".*, (select json_agg(json_build_object('product_id',"products"."product_id",'product_name',"products"."product_name",'brand_id',"products"."brand_id")) from (select "products".* from "production"."products" as "products" where "products"."brand_id" = "brands"."brand_id" order by "products"."brand_id" asc limit $1) as "products") as "products" from "production"."brands" order by "test"."id" asc limit $2`,
bindings: [limit, relationshipLimit],
sql: `with "paginated" as (select "brands".* from "production"."brands" order by "test"."id" asc limit $1) select "brands".*, (select json_agg(json_build_object('product_id',"products"."product_id",'product_name',"products"."product_name",'brand_id',"products"."brand_id")) from (select "products".* from "production"."products" as "products" where "products"."brand_id" = "brands"."brand_id" order by "products"."brand_id" asc limit $2) as "products") as "products" from "paginated" as "brands" order by "test"."id" asc`,
})
})

it("should handle if the schema is not present when doing a LEFT JOIN", () => {
const query = sql._query(generateRelationshipJson())
expect(query).toEqual({
bindings: [relationshipLimit, limit],
sql: `select "brands".*, (select json_agg(json_build_object('product_id',"products"."product_id",'product_name',"products"."product_name",'brand_id',"products"."brand_id")) from (select "products".* from "products" as "products" where "products"."brand_id" = "brands"."brand_id" order by "products"."brand_id" asc limit $1) as "products") as "products" from "brands" order by "test"."id" asc limit $2`,
bindings: [limit, relationshipLimit],
sql: `with "paginated" as (select "brands".* from "brands" order by "test"."id" asc limit $1) select "brands".*, (select json_agg(json_build_object('product_id',"products"."product_id",'product_name',"products"."product_name",'brand_id',"products"."brand_id")) from (select "products".* from "products" as "products" where "products"."brand_id" = "brands"."brand_id" order by "products"."brand_id" asc limit $2) as "products") as "products" from "paginated" as "brands" order by "test"."id" asc`,
})
})

Expand All @@ -179,8 +179,8 @@ describe("SQL query builder", () => {
generateManyRelationshipJson({ schema: "production" })
)
expect(query).toEqual({
bindings: [relationshipLimit, limit],
sql: `select "stores".*, (select json_agg(json_build_object('product_id',"products"."product_id",'product_name',"products"."product_name")) from (select "products".* from "production"."products" as "products" inner join "production"."stocks" as "stocks" on "products"."product_id" = "stocks"."product_id" where "stocks"."store_id" = "stores"."store_id" order by "products"."product_id" asc limit $1) as "products") as "products" from "production"."stores" order by "test"."id" asc limit $2`,
bindings: [limit, relationshipLimit],
sql: `with "paginated" as (select "stores".* from "production"."stores" order by "test"."id" asc limit $1) select "stores".*, (select json_agg(json_build_object('product_id',"products"."product_id",'product_name',"products"."product_name")) from (select "products".* from "production"."products" as "products" inner join "production"."stocks" as "stocks" on "products"."product_id" = "stocks"."product_id" where "stocks"."store_id" = "stores"."store_id" order by "products"."product_id" asc limit $2) as "products") as "products" from "paginated" as "stores" order by "test"."id" asc`,
})
})

Expand Down
33 changes: 17 additions & 16 deletions packages/server/src/integrations/tests/sqlAlias.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ describe("Captures of real examples", () => {
queryJson
)
expect(query).toEqual({
bindings: [relationshipLimit, relationshipLimit, primaryLimit],
bindings: [primaryLimit, relationshipLimit, relationshipLimit],
sql: expect.stringContaining(
multiline(
`select json_agg(json_build_object('executorid',"b"."executorid",'taskname',"b"."taskname",'taskid',"b"."taskid",'completed',"b"."completed",'qaid',"b"."qaid",'executorid',"b"."executorid",'taskname',"b"."taskname",'taskid',"b"."taskid",'completed',"b"."completed",'qaid',"b"."qaid")`
Expand All @@ -75,11 +75,11 @@ describe("Captures of real examples", () => {
queryJson
)
expect(query).toEqual({
bindings: [relationshipLimit, "assembling", primaryLimit],
bindings: ["assembling", primaryLimit, relationshipLimit],
sql: expect.stringContaining(
multiline(
`where exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid"
where "c"."productid" = "a"."productid" and COALESCE("b"."taskname" = $2, FALSE)`
`where exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid"
and COALESCE("b"."taskname" = $1, FALSE)`
)
),
})
Expand All @@ -91,12 +91,13 @@ describe("Captures of real examples", () => {
queryJson
)
expect(query).toEqual({
bindings: [relationshipLimit, primaryLimit],
bindings: [primaryLimit, relationshipLimit],
sql: expect.stringContaining(
multiline(
`select json_agg(json_build_object('executorid',"b"."executorid",'taskname',"b"."taskname",'taskid',"b"."taskid",'completed',"b"."completed",'qaid',"b"."qaid"))
from (select "b".* from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid"
where "c"."productid" = "a"."productid" order by "b"."taskid" asc limit $1`
`with "paginated" as (select "a".* from "products" as "a" order by "a"."productname" asc nulls first, "a"."productid" asc limit $1)
select "a".*, (select json_agg(json_build_object('executorid',"b"."executorid",'taskname',"b"."taskname",'taskid',"b"."taskid",'completed',"b"."completed",'qaid',"b"."qaid"))
from (select "b".* from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" order by "b"."taskid" asc limit $2) as "b") as "tasks"
from "paginated" as "a" order by "a"."productname" asc nulls first, "a"."productid" asc`
)
),
})
Expand All @@ -109,12 +110,12 @@ describe("Captures of real examples", () => {
queryJson
)
expect(query).toEqual({
bindings: [relationshipLimit, ...filters, relationshipLimit],
bindings: [...filters, relationshipLimit, relationshipLimit],
sql: multiline(
`select "a".*, (select json_agg(json_build_object('productname',"b"."productname",'productid',"b"."productid"))
`with "paginated" as (select "a".* from "tasks" as "a" where "a"."taskid" in ($1, $2) order by "a"."taskid" asc limit $3)
select "a".*, (select json_agg(json_build_object('productname',"b"."productname",'productid',"b"."productid"))
from (select "b".* from "products" as "b" inner join "products_tasks" as "c" on "b"."productid" = "c"."productid"
where "c"."taskid" = "a"."taskid" order by "b"."productid" asc limit $1) as "b") as "products"
from "tasks" as "a" where "a"."taskid" in ($2, $3) order by "a"."taskid" asc limit $4`
where "c"."taskid" = "a"."taskid" order by "b"."productid" asc limit $4) as "b") as "products" from "paginated" as "a" order by "a"."taskid" asc`
),
})
})
Expand All @@ -132,18 +133,18 @@ describe("Captures of real examples", () => {

expect(query).toEqual({
bindings: [
relationshipLimit,
relationshipLimit,
relationshipLimit,
rangeValue.low,
rangeValue.high,
equalValue,
notEqualsValue,
primaryLimit,
relationshipLimit,
relationshipLimit,
relationshipLimit,
],
sql: expect.stringContaining(
multiline(
`where exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and "c"."year" between $4 and $5)`
`where exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and "c"."year" between $1 and $2)`
)
),
})
Expand Down
Loading