Managing the scenario where columns can overlap in SQL relationships which most JSON based libraries cannot manage, instead of trying to manage this just don't return the overlapping columns which are not of interest.

packages/server/src/api/controllers/row/external.js

@@ -1,8 +1,5 @@
 const { makeExternalQuery } = require("./utils")
-const {
-  DataSourceOperation,
-  SortDirection,
-} = require("../../../constants")
+const { DataSourceOperation, SortDirection } = require("../../../constants")
 const { getAllExternalTables } = require("../table/utils")
 const {
   breakExternalTableId,
@@ -14,6 +11,7 @@ const {
   inputProcessing,
   outputProcessing,
   generateIdForRow,
+  buildFields,
 } = require("./externalUtils")
 const { processObjectSync } = require("@budibase/string-templates")
 
@@ -23,7 +21,7 @@ async function handleRequest(
   tableId,
   { id, row, filters, sort, paginate } = {}
 ) {
-  let {datasourceId, tableName} = breakExternalTableId(tableId)
+  let { datasourceId, tableName } = breakExternalTableId(tableId)
   const tables = await getAllExternalTables(appId, datasourceId)
   const table = tables[tableName]
   if (!table) {
@@ -32,7 +30,7 @@ async function handleRequest(
   // clean up row on ingress using schema
   filters = buildFilters(id, filters, table)
   const relationships = buildRelationships(table, tables)
-  const processed = inputProcessing(row, table)
+  const processed = inputProcessing(row, table, tables)
   row = processed.row
   if (
     operation === DataSourceOperation.DELETE &&
@@ -47,8 +45,8 @@ async function handleRequest(
       operation,
     },
     resource: {
-      // not specifying any fields means "*"
-      fields: [],
+      // have to specify the fields to avoid column overlap
+      fields: buildFields(table, tables),
     },
     filters,
     sort,
@@ -66,15 +64,17 @@ async function handleRequest(
   if (processed.manyRelationships) {
     const promises = []
     for (let toInsert of processed.manyRelationships) {
-      const {tableName} = breakExternalTableId(toInsert.tableId)
+      const { tableName } = breakExternalTableId(toInsert.tableId)
       delete toInsert.tableId
-      promises.push(makeExternalQuery(appId, {
+      promises.push(
+        makeExternalQuery(appId, {
           endpoint: {
             ...json.endpoint,
-          entityId: tableName,
+            entityId: processObjectSync(tableName, row),
           },
           body: toInsert,
-      }))
+        })
+      )
     }
     await Promise.all(promises)
   }

packages/server/src/api/controllers/row/externalUtils.js

@@ -10,7 +10,8 @@ exports.inputProcessing = (row, table, allTables) => {
   if (!row) {
     return { row, manyRelationships: [] }
   }
-  let newRow = {}, manyRelationships = []
+  let newRow = {},
+    manyRelationships = []
   for (let [key, field] of Object.entries(table.schema)) {
     // currently excludes empty strings
     if (!row[key]) {
@@ -21,18 +22,19 @@ exports.inputProcessing = (row, table, allTables) => {
       // we don't really support composite keys for relationships, this is why [0] is used
       newRow[key] = breakRowIdField(row[key][0])[0]
     } else if (isLink && field.through) {
-      const linkTable = allTables.find(table => table._id === field.tableId)
+      const { tableName: linkTableName } = breakExternalTableId(field.tableId)
       // table has to exist for many to many
-      if (!linkTable) {
+      if (!allTables[linkTableName]) {
         continue
       }
+      const linkTable = allTables[linkTableName]
       row[key].map(relationship => {
         // we don't really support composite keys for relationships, this is why [0] is used
         manyRelationships.push({
           tableId: field.through,
           [linkTable.primary]: breakRowIdField(relationship)[0],
           // leave the ID for enrichment later
-          [table.primary]: `{{ id }}`,
+          [table.primary]: `{{ _id }}`,
         })
       })
     } else {
@@ -120,6 +122,42 @@ exports.outputProcessing = (rows, table, relationships, allTables) => {
   return Object.values(finalRows)
 }
 
+/**
+ * This function is a bit crazy, but the exact purpose of it is to protect against the scenario in which
+ * you have column overlap in relationships, e.g. we join a few different tables and they all have the
+ * concept of an ID, but for some of them it will be null (if they say don't have a relationship).
+ * Creating the specific list of fields that we desire, and excluding the ones that are no use to us
+ * is more performant and has the added benefit of protecting against this scenario.
+ * @param {Object} table The table we are retrieving fields for.
+ * @param {Object[]} allTables All of the tables that exist in the external data source, this is
+ * needed to work out what is needed from other tables based on relationships.
+ * @return {string[]} A list of fields like ["products.productid"] which can be used for an SQL select.
+ */
+exports.buildFields = (table, allTables) => {
+  function extractNonLinkFieldNames(table, existing = []) {
+    return Object.entries(table.schema)
+      .filter(
+        column =>
+          column[1].type !== FieldTypes.LINK &&
+          !existing.find(field => field.includes(column[0]))
+      )
+      .map(column => `${table.name}.${column[0]}`)
+  }
+  let fields = extractNonLinkFieldNames(table)
+  for (let field of Object.values(table.schema)) {
+    if (field.type !== FieldTypes.LINK) {
+      continue
+    }
+    const { tableName: linkTableName } = breakExternalTableId(field.tableId)
+    const linkTable = allTables[linkTableName]
+    if (linkTable) {
+      const linkedFields = extractNonLinkFieldNames(linkTable, fields)
+      fields = fields.concat(linkedFields)
+    }
+  }
+  return fields
+}
+
 exports.buildFilters = (id, filters, table) => {
   const primary = table.primary
   // if passed in array need to copy for shifting etc
@@ -177,7 +215,9 @@ exports.buildRelationships = (table, allTables) => {
       column: fieldName,
     }
     if (field.through) {
-      const { tableName: throughTableName } = breakExternalTableId(field.through)
+      const { tableName: throughTableName } = breakExternalTableId(
+        field.through
+      )
       definition.through = throughTableName
       // don't support composite keys for relationships
       definition.from = table.primary[0]

packages/server/src/integrations/base/sql.ts

@@ -84,7 +84,7 @@ function addRelationships(
       toTable = relationship.tableName
     if (!relationship.through) {
       // @ts-ignore
-      query = query.innerJoin(
+      query = query.leftJoin(
         toTable,
         `${fromTable}.${from}`,
         `${relationship.tableName}.${to}`
@@ -93,8 +93,12 @@ function addRelationships(
       const throughTable = relationship.through
       query = query
         // @ts-ignore
-        .innerJoin(throughTable, `${fromTable}.${from}`, `${throughTable}.${from}`)
-        .innerJoin(toTable, `${toTable}.${to}`, `${throughTable}.${to}`)
+        .leftJoin(
+          throughTable,
+          `${fromTable}.${from}`,
+          `${throughTable}.${from}`
+        )
+        .leftJoin(toTable, `${toTable}.${to}`, `${throughTable}.${to}`)
     }
   }
   return query