Merge pull request #14131 from Budibase/fix/correcting-multi-relationships

Fixing issue with multiple relations to same table

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

@@ -22,6 +22,20 @@ export function isManyToMany(
   return !!(field as ManyToManyRelationshipFieldMetadata).through
 }
 
+function isCorrectRelationship(
+  relationship: RelationshipsJson,
+  table1: Table,
+  table2: Table,
+  row: Row
+): boolean {
+  const junctionTableId = generateJunctionTableID(table1._id!, table2._id!)
+  const possibleColumns = [
+    `${junctionTableId}.doc1.fieldName`,
+    `${junctionTableId}.doc2.fieldName`,
+  ]
+  return !!possibleColumns.find(col => row[col] === relationship.column)
+}
+
 /**
  * This iterates through the returned rows and works out what elements of the rows
  * actually match up to another row (based on primary keys) - this is pretty specific
@@ -64,7 +78,12 @@ export async function updateRelationshipColumns(
     if (!linked._id) {
       continue
     }
-    columns[relationship.column] = linked
+    if (
+      !opts?.sqs ||
+      isCorrectRelationship(relationship, table, linkedTable, row)
+    ) {
+      columns[relationship.column] = linked
+    }
   }
   for (let [column, related] of Object.entries(columns)) {
     if (!row._id) {

packages/server/src/api/routes/tests/search.spec.ts

@@ -2140,6 +2140,106 @@ describe.each([
       })
     })
 
+  isInternal &&
+    describe("relations to same table", () => {
+      let relatedTable: Table, relatedRows: Row[]
+
+      beforeAll(async () => {
+        relatedTable = await createTable(
+          {
+            name: { name: "name", type: FieldType.STRING },
+          },
+          "productCategory"
+        )
+        table = await createTable({
+          name: { name: "name", type: FieldType.STRING },
+          related1: {
+            type: FieldType.LINK,
+            name: "related1",
+            fieldName: "main1",
+            tableId: relatedTable._id!,
+            relationshipType: RelationshipType.MANY_TO_MANY,
+          },
+          related2: {
+            type: FieldType.LINK,
+            name: "related2",
+            fieldName: "main2",
+            tableId: relatedTable._id!,
+            relationshipType: RelationshipType.MANY_TO_MANY,
+          },
+        })
+        relatedRows = await Promise.all([
+          config.api.row.save(relatedTable._id!, { name: "foo" }),
+          config.api.row.save(relatedTable._id!, { name: "bar" }),
+          config.api.row.save(relatedTable._id!, { name: "baz" }),
+          config.api.row.save(relatedTable._id!, { name: "boo" }),
+        ])
+        await Promise.all([
+          config.api.row.save(table._id!, {
+            name: "test",
+            related1: [relatedRows[0]._id!],
+            related2: [relatedRows[1]._id!],
+          }),
+          config.api.row.save(table._id!, {
+            name: "test2",
+            related1: [relatedRows[2]._id!],
+            related2: [relatedRows[3]._id!],
+          }),
+        ])
+      })
+
+      it("should be able to relate to same table", async () => {
+        await expectSearch({
+          query: {},
+        }).toContainExactly([
+          {
+            name: "test",
+            related1: [{ _id: relatedRows[0]._id }],
+            related2: [{ _id: relatedRows[1]._id }],
+          },
+          {
+            name: "test2",
+            related1: [{ _id: relatedRows[2]._id }],
+            related2: [{ _id: relatedRows[3]._id }],
+          },
+        ])
+      })
+
+      isSqs &&
+        it("should be able to filter down to second row with equal", async () => {
+          await expectSearch({
+            query: {
+              equal: {
+                ["related1.name"]: "baz",
+              },
+            },
+          }).toContainExactly([
+            {
+              name: "test2",
+              related1: [{ _id: relatedRows[2]._id }],
+            },
+          ])
+        })
+
+      isSqs &&
+        it("should be able to filter down to first row with not equal", async () => {
+          await expectSearch({
+            query: {
+              notEqual: {
+                ["1:related2.name"]: "bar",
+                ["2:related2.name"]: "baz",
+                ["3:related2.name"]: "boo",
+              },
+            },
+          }).toContainExactly([
+            {
+              name: "test",
+              related1: [{ _id: relatedRows[0]._id }],
+            },
+          ])
+        })
+    })
+
   isInternal &&
     describe("no column error backwards compat", () => {
       beforeAll(async () => {

packages/server/src/sdk/app/rows/search/sqs.ts

@@ -5,6 +5,7 @@ import {
   Operation,
   QueryJson,
   RelationshipFieldMetadata,
+  RelationshipsJson,
   Row,
   RowSearchParams,
   SearchFilters,
@@ -30,7 +31,10 @@ import {
   SQLITE_DESIGN_DOC_ID,
   SQS_DATASOURCE_INTERNAL,
 } from "@budibase/backend-core"
-import { CONSTANT_INTERNAL_ROW_COLS } from "../../../../db/utils"
+import {
+  CONSTANT_INTERNAL_ROW_COLS,
+  generateJunctionTableID,
+} from "../../../../db/utils"
 import AliasTables from "../sqlAlias"
 import { outputProcessing } from "../../../../utilities/rowProcessor"
 import pick from "lodash/pick"
@@ -52,28 +56,35 @@ const USER_COLUMN_PREFIX_REGEX = new RegExp(
 function buildInternalFieldList(
   table: Table,
   tables: Table[],
-  opts: { relationships: boolean } = { relationships: true }
+  opts?: { relationships?: RelationshipsJson[] }
 ) {
   let fieldList: string[] = []
+  const addJunctionFields = (relatedTable: Table, fields: string[]) => {
+    fields.forEach(field => {
+      fieldList.push(
+        `${generateJunctionTableID(table._id!, relatedTable._id!)}.${field}`
+      )
+    })
+  }
   fieldList = fieldList.concat(
     CONSTANT_INTERNAL_ROW_COLS.map(col => `${table._id}.${col}`)
   )
   for (let col of Object.values(table.schema)) {
     const isRelationship = col.type === FieldType.LINK
-    if (!opts.relationships && isRelationship) {
+    if (!opts?.relationships && isRelationship) {
       continue
     }
     if (isRelationship) {
       const linkCol = col as RelationshipFieldMetadata
       const relatedTable = tables.find(table => table._id === linkCol.tableId)!
-      fieldList = fieldList.concat(
-        buildInternalFieldList(relatedTable, tables, { relationships: false })
-      )
+      // no relationships provided, don't go more than a layer deep
+      fieldList = fieldList.concat(buildInternalFieldList(relatedTable, tables))
+      addJunctionFields(relatedTable, ["doc1.fieldName", "doc2.fieldName"])
     } else {
       fieldList.push(`${table._id}.${mapToUserColumn(col.name)}`)
     }
   }
-  return fieldList
+  return [...new Set(fieldList)]
 }
 
 function cleanupFilters(
@@ -165,18 +176,27 @@ function reverseUserColumnMapping(rows: Row[]) {
   })
 }
 
-function runSqlQuery(json: QueryJson, tables: Table[]): Promise<Row[]>
 function runSqlQuery(
   json: QueryJson,
   tables: Table[],
+  relationships: RelationshipsJson[]
+): Promise<Row[]>
+function runSqlQuery(
+  json: QueryJson,
+  tables: Table[],
+  relationships: RelationshipsJson[],
   opts: { countTotalRows: true }
 ): Promise<number>
 async function runSqlQuery(
   json: QueryJson,
   tables: Table[],
+  relationships: RelationshipsJson[],
   opts?: { countTotalRows?: boolean }
 ) {
-  const alias = new AliasTables(tables.map(table => table.name))
+  const relationshipJunctionTableIds = relationships.map(rel => rel.through!)
+  const alias = new AliasTables(
+    tables.map(table => table.name).concat(relationshipJunctionTableIds)
+  )
   if (opts?.countTotalRows) {
     json.endpoint.operation = Operation.COUNT
   }
@@ -193,8 +213,13 @@ async function runSqlQuery(
     let bindings = query.bindings
 
     // quick hack for docIds
-    sql = sql.replace(/`doc1`.`rowId`/g, "`doc1.rowId`")
-    sql = sql.replace(/`doc2`.`rowId`/g, "`doc2.rowId`")
+
+    const fixJunctionDocs = (field: string) =>
+      ["doc1", "doc2"].forEach(doc => {
+        sql = sql.replaceAll(`\`${doc}\`.\`${field}\``, `\`${doc}.${field}\``)
+      })
+    fixJunctionDocs("rowId")
+    fixJunctionDocs("fieldName")
 
     if (Array.isArray(query)) {
       throw new Error("SQS cannot currently handle multiple queries")
@@ -260,7 +285,7 @@ export async function search(
       columnPrefix: USER_COLUMN_PREFIX,
     },
     resource: {
-      fields: buildInternalFieldList(table, allTables),
+      fields: buildInternalFieldList(table, allTables, { relationships }),
     },
     relationships,
   }
@@ -292,11 +317,11 @@ export async function search(
 
   try {
     const queries: Promise<Row[] | number>[] = []
-    queries.push(runSqlQuery(request, allTables))
+    queries.push(runSqlQuery(request, allTables, relationships))
     if (options.countRows) {
       // get the total count of rows
       queries.push(
-        runSqlQuery(request, allTables, {
+        runSqlQuery(request, allTables, relationships, {
           countTotalRows: true,
         })
       )

packages/server/src/sdk/app/rows/sqlAlias.ts

@@ -111,7 +111,8 @@ export default class AliasTables {
   aliasField(field: string) {
     const tableNames = this.tableNames
     if (field.includes(".")) {
-      const [tableName, column] = field.split(".")
+      const [tableName, ...rest] = field.split(".")
+      const column = rest.join(".")
       const foundTableName = tableNames.find(name => {
         const idx = tableName.indexOf(name)
         if (idx === -1 || idx > 1) {

packages/server/src/sdk/app/tables/internal/sqs.ts

@@ -176,9 +176,22 @@ export async function addTable(table: Table) {
 export async function removeTable(table: Table) {
   const db = context.getAppDB()
   try {
-    const definition = await db.get<SQLiteDefinition>(SQLITE_DESIGN_DOC_ID)
-    if (definition.sql?.tables?.[table._id!]) {
-      delete definition.sql.tables[table._id!]
+    const [tables, definition] = await Promise.all([
+      tablesSdk.getAllInternalTables(),
+      db.get<SQLiteDefinition>(SQLITE_DESIGN_DOC_ID),
+    ])
+    const tableIds = tables
+      .map(tbl => tbl._id!)
+      .filter(id => !id.includes(table._id!))
+    let cleanup = false
+    for (let tableKey of Object.keys(definition.sql?.tables || {})) {
+      // there are no tables matching anymore
+      if (!tableIds.find(id => tableKey.includes(id))) {
+        delete definition.sql.tables[tableKey]
+        cleanup = true
+      }
+    }
+    if (cleanup) {
       await db.put(definition)
       // make sure SQS is cleaned up, tables removed
       await db.sqlDiskCleanup()