Merge pull request #14557 from Budibase/fix/sql-use-ctes

SQL - use CTEs to avoid pagination issues with large tables

hosting/docker-compose.dev.yaml

@@ -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}

package.json

@@ -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",

packages/backend-core/src/sql/sql.ts

@@ -862,16 +862,7 @@ class InternalBuilder {
     return withSchema
   }
 
-  addJsonRelationships(
+  private buildJsonField(field: string): string {
-    query: Knex.QueryBuilder,
-    fromTable: string,
-    relationships: RelationshipsJson[]
-  ): Knex.QueryBuilder {
-    const sqlClient = this.client
-    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) {
@@ -882,16 +873,19 @@ class InternalBuilder {
       unaliased = parts.join(".")
       tableField = this.quote(unaliased)
     }
-      let separator = ","
+    const separator = this.client === SqlClient.ORACLE ? " VALUE " : ","
-      switch (sqlClient) {
-        case SqlClient.ORACLE:
-          separator = " VALUE "
-          break
-        case SqlClient.MS_SQL:
-          separator = ":"
-      }
     return `'${unaliased}'${separator}${tableField}`
   }
+
+  addJsonRelationships(
+    query: Knex.QueryBuilder,
+    fromTable: string,
+    relationships: RelationshipsJson[]
+  ): Knex.QueryBuilder {
+    const sqlClient = this.client
+    const knex = this.knex
+    const { resource, tableAliases: aliases, endpoint } = this.query
+    const fields = resource?.fields || []
     for (let relationship of relationships) {
       const {
         tableName: toTable,
@@ -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
@@ -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) {
@@ -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 query.insert(parsedBody).onConflict(primary).merge()
-      return ret
     } else if (
       this.client === SqlClient.MS_SQL ||
       this.client === SqlClient.ORACLE
@@ -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 {

packages/server/src/api/controllers/row/utils/basic.ts

@@ -145,12 +145,10 @@ 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]!
-        const sortField =
-          relatedTable.primaryDisplay || relatedTable.primary![0]!
       thisRow[col] = (thisRow[col] as Row[])
         .map(relatedRow =>
           basicProcessing({
@@ -175,6 +173,5 @@ export function basicProcessing({
         })
     }
   }
-  }
   return fixJsonTypes(thisRow, table)
 }

packages/server/src/api/routes/tests/queries/generic-sql.spec.ts

@@ -832,12 +832,13 @@ describe.each(
         },
       })
       expect(res).toHaveLength(1)
-      expect(res[0]).toEqual(
+      expect(res[0]).toEqual({
-        expect.objectContaining({
         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

packages/server/src/integrations/tests/sql.spec.ts

@@ -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],
+      bindings: [limit, relationshipLimit],
-      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`,
+      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],
+      bindings: [limit, relationshipLimit],
-      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`,
+      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`,
     })
   })
 
@@ -179,8 +179,8 @@ describe("SQL query builder", () => {
       generateManyRelationshipJson({ schema: "production" })
     )
     expect(query).toEqual({
-      bindings: [relationshipLimit, limit],
+      bindings: [limit, relationshipLimit],
-      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`,
+      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`,
     })
   })
 

packages/server/src/integrations/tests/sqlAlias.spec.ts

@@ -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")`
@@ -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 exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" 
-                 where "c"."productid" = "a"."productid" and COALESCE("b"."taskname" = $2, FALSE)`
+                 and COALESCE("b"."taskname" = $1, FALSE)`
           )
         ),
       })
@@ -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")) 
+            `with "paginated" as (select "a".* from "products" as "a" order by "a"."productname" asc nulls first, "a"."productid" asc limit $1) 
-                 from (select "b".* from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" 
+                 select "a".*, (select json_agg(json_build_object('executorid',"b"."executorid",'taskname',"b"."taskname",'taskid',"b"."taskid",'completed',"b"."completed",'qaid',"b"."qaid")) 
-                 where "c"."productid" = "a"."productid" order by "b"."taskid" asc limit $1`
+                 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`
           )
         ),
       })
@@ -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" 
+               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`
-               from "tasks" as "a" where "a"."taskid" in ($2, $3) order by "a"."taskid" asc limit $4`
         ),
       })
     })
@@ -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)`
           )
         ),
       })