Merge pull request #4297 from Budibase/fix/4255

Some small SQL core fixes
2022-02-03 20:29:38 +00:00 · 2022-02-03 20:29:38 +00:00 · 6f79bc53e0
parent f2a025013e 0b68db7516
commit 6f79bc53e0
5 changed files with 133 additions and 87 deletions
--- a/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/PlusConfigForm.svelte
+++ b/packages/builder/src/components/backend/DatasourceNavigator/TableIntegrationMenu/PlusConfigForm.svelte
@ -188,18 +188,17 @@
 {:else}
  <Body size="S"><i>No tables found.</i></Body>
 {/if}
-{#if plusTables?.length !== 0 && integration.relationships}
-  <Divider size="S" />
-  <div class="query-header">
+<Divider size="S" />
+<div class="query-header">
  <Heading size="S">Relationships</Heading>
-    <Button primary on:click={openRelationshipModal}>
+  <Button primary on:click={() => openRelationshipModal()}>
    Define relationship
  </Button>
-  </div>
-  <Body>
+</div>
+<Body>
  Tell budibase how your tables are related to get even more smart features.
-  </Body>
-  {#if relationshipInfo && relationshipInfo.length > 0}
+</Body>
+{#if relationshipInfo && relationshipInfo.length > 0}
  <Table
    on:click={({ detail }) => openRelationshipModal(detail.from, detail.to)}
    schema={relationshipSchema}
@ -208,9 +207,8 @@
    allowEditRows={false}
    allowSelectRows={false}
  />
-  {:else}
+{:else}
  <Body size="S"><i>No relationships configured.</i></Body>
-  {/if}
 {/if}

 <style>
--- a/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte
+++ b/packages/builder/src/components/backend/Datasources/CreateEditRelationship.svelte
@ -22,6 +22,10 @@

  let originalFromName = fromRelationship.name,
    originalToName = toRelationship.name
+  let fromTable, toTable, through, linkTable, tableOptions
+  let isManyToMany, isManyToOne, relationshipTypes
+  let errors, valid
+  let currentTables = {}

  if (fromRelationship && !fromRelationship.relationshipType) {
    fromRelationship.relationshipType = RelationshipTypes.MANY_TO_ONE
@ -41,61 +45,52 @@

  const touched = writable({})

-  function checkForErrors(
-    fromTable,
-    toTable,
-    throughTable,
-    fromRelate,
-    toRelate
-  ) {
+  function checkForErrors(fromRelate, toRelate) {
    const isMany =
      fromRelate.relationshipType === RelationshipTypes.MANY_TO_MANY
    const tableNotSet = "Please specify a table"
-    const errors = {}
+    const errObj = {}
    if ($touched.from && !fromTable) {
-      errors.from = tableNotSet
+      errObj.from = tableNotSet
    }
    if ($touched.to && !toTable) {
-      errors.to = tableNotSet
+      errObj.to = tableNotSet
    }
    if ($touched.through && isMany && !fromRelate.through) {
-      errors.through = tableNotSet
+      errObj.through = tableNotSet
    }
    if ($touched.foreign && !isMany && !fromRelate.fieldName) {
-      errors.foreign = "Please pick the foreign key"
+      errObj.foreign = "Please pick the foreign key"
    }
    const colNotSet = "Please specify a column name"
    if ($touched.fromCol && !fromRelate.name) {
-      errors.fromCol = colNotSet
+      errObj.fromCol = colNotSet
    }
    if ($touched.toCol && !toRelate.name) {
-      errors.toCol = colNotSet
+      errObj.toCol = colNotSet
    }
    if ($touched.primary && !fromPrimary) {
-      errors.primary = "Please pick the primary key"
+      errObj.primary = "Please pick the primary key"
    }
    // currently don't support relationships back onto the table itself, needs to relate out
    const tableError = "From/to/through tables must be different"
-    if (fromTable && (fromTable === toTable || fromTable === throughTable)) {
-      errors.from = tableError
+    if (fromTable && (fromTable === toTable || fromTable === through)) {
+      errObj.from = tableError
    }
-    if (toTable && (toTable === fromTable || toTable === throughTable)) {
-      errors.to = tableError
+    if (toTable && (toTable === fromTable || toTable === through)) {
+      errObj.to = tableError
    }
-    if (
-      throughTable &&
-      (throughTable === fromTable || throughTable === toTable)
-    ) {
-      errors.through = tableError
+    if (through && (through === fromTable || through === toTable)) {
+      errObj.through = tableError
    }
    const colError = "Column name cannot be an existing column"
    if (inSchema(fromTable, fromRelate.name, originalFromName)) {
-      errors.fromCol = colError
+      errObj.fromCol = colError
    }
    if (inSchema(toTable, toRelate.name, originalToName)) {
-      errors.toCol = colError
+      errObj.toCol = colError
    }
-    return errors
+    errors = errObj
  }

  let fromPrimary
@ -115,13 +110,7 @@
  $: fromTable = plusTables.find(table => table._id === toRelationship?.tableId)
  $: toTable = plusTables.find(table => table._id === fromRelationship?.tableId)
  $: through = plusTables.find(table => table._id === fromRelationship?.through)
-  $: errors = checkForErrors(
-    fromTable,
-    toTable,
-    through,
-    fromRelationship,
-    toRelationship
-  )
+  $: checkForErrors(fromRelationship, toRelationship)
  $: valid =
    Object.keys(errors).length === 0 && Object.keys($touched).length !== 0
  $: linkTable = through || toTable
@ -239,19 +228,19 @@
  }

  function tableChanged(fromTbl, toTbl) {
+    if (
+      (currentTables?.from?._id === fromTbl?._id &&
+        currentTables?.to?._id === toTbl?._id) ||
+      originalFromName ||
+      originalToName
+    ) {
+      return
+    }
    fromRelationship.name = toTbl?.name || ""
    errors.fromCol = ""
    toRelationship.name = fromTbl?.name || ""
    errors.toCol = ""
-    if (toTbl || fromTbl) {
-      checkForErrors(
-        fromTable,
-        toTable,
-        through,
-        fromRelationship,
-        toRelationship
-      )
-    }
+    currentTables = { from: fromTbl, to: toTbl }
  }
 </script>

--- a/packages/server/scripts/integrations/postgres/init.sql
+++ b/packages/server/scripts/integrations/postgres/init.sql
@ -9,11 +9,15 @@ CREATE TABLE Persons (
 );
 CREATE TABLE Tasks (
    TaskID SERIAL PRIMARY KEY,
-    PersonID INT,
+    ExecutorID INT,
+    QaID INT,
    Completed BOOLEAN,
    TaskName varchar(255),
-    CONSTRAINT fkPersons
-        FOREIGN KEY(PersonID)
+    CONSTRAINT fkexecutor
+        FOREIGN KEY(ExecutorID)
+            REFERENCES Persons(PersonID),
+    CONSTRAINT fkqa
+        FOREIGN KEY(QaID)
 	    REFERENCES Persons(PersonID)
 );
 CREATE TABLE Products (
@ -32,8 +36,9 @@ CREATE TABLE Products_Tasks (
    PRIMARY KEY (ProductID, TaskID)
 );
 INSERT INTO Persons (FirstName, LastName, Address, City) VALUES ('Mike', 'Hughes', '123 Fake Street', 'Belfast');
-INSERT INTO Tasks (PersonID, TaskName, Completed) VALUES (1, 'assembling', TRUE);
-INSERT INTO Tasks (PersonID, TaskName, Completed) VALUES (1, 'processing', FALSE);
+INSERT INTO Persons (FirstName, LastName, Address, City) Values ('John', 'Smith', '64 Updown Road', 'Dublin');
+INSERT INTO Tasks (ExecutorID, QaID, TaskName, Completed) VALUES (1, 2, 'assembling', TRUE);
+INSERT INTO Tasks (ExecutorID, QaID, TaskName, Completed) VALUES (2, 1, 'processing', FALSE);
 INSERT INTO Products (ProductName) VALUES ('Computers');
 INSERT INTO Products (ProductName) VALUES ('Laptops');
 INSERT INTO Products (ProductName) VALUES ('Chairs');
--- a/packages/server/src/api/controllers/row/ExternalRequest.ts
+++ b/packages/server/src/api/controllers/row/ExternalRequest.ts
@ -184,7 +184,7 @@ module External {
    thisRow._id = generateIdForRow(row, table)
    thisRow.tableId = table._id
    thisRow._rev = "rev"
-    return thisRow
+    return processFormulas(table, thisRow)
  }

  function fixArrayTypes(row: Row, table: Table) {
@ -327,8 +327,12 @@ module External {
     * 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
     * to SQL and the way that SQL relationships are returned based on joins.
+     * This is complicated, but the idea is that when a SQL query returns all the relations
+     * will be separate rows, with all of the data in each row. We have to decipher what comes
+     * from where (which tables) and how to convert that into budibase columns.
     */
    updateRelationshipColumns(
+      table: Table,
      row: Row,
      rows: { [key: string]: Row },
      relationships: RelationshipsJson[]
@ -339,6 +343,13 @@ module External {
        if (!linkedTable) {
          continue
        }
+        const fromColumn = `${table.name}.${relationship.from}`
+        const toColumn = `${linkedTable.name}.${relationship.to}`
+        // this is important when working with multiple relationships
+        // between the same tables, don't want to overlap/multiply the relations
+        if (!relationship.through && row[fromColumn] !== row[toColumn]) {
+          continue
+        }
        let linked = basicProcessing(row, linkedTable)
        if (!linked._id) {
          continue
@ -386,6 +397,7 @@ module External {
        // this is a relationship of some sort
        if (finalRows[rowId]) {
          finalRows = this.updateRelationshipColumns(
+            table,
            row,
            finalRows,
            relationships
@ -399,6 +411,7 @@ module External {
        finalRows[thisRow._id] = thisRow
        // do this at end once its been added to the final rows
        finalRows = this.updateRelationshipColumns(
+          table,
          row,
          finalRows,
          relationships
--- a/packages/server/src/integrations/base/sql.ts
+++ b/packages/server/src/integrations/base/sql.ts
@ -191,29 +191,70 @@ class InternalBuilder {
    if (!relationships) {
      return query
    }
+    const tableSets: Record<string, [any]> = {}
+    // 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)
+      if (!throughTable) {
+        // @ts-ignore
+        query = query.join(
+          toTable,
+          function () {
            for (let relationship of relationships) {
              const from = relationship.from,
-        to = relationship.to,
-        toTable = relationship.tableName
-      if (!relationship.through) {
+                to = relationship.to
              // @ts-ignore
-        query = query.leftJoin(
-          toTable,
-          `${fromTable}.${from}`,
-          `${toTable}.${to}`
+              this.orOn(`${fromTable}.${from}`, "=", `${toTable}.${to}`)
+            }
+          },
+          "left"
        )
      } else {
-        const throughTable = relationship.through
-        const fromPrimary = relationship.fromPrimary
-        const toPrimary = relationship.toPrimary
        query = query
          // @ts-ignore
-          .leftJoin(
+          .join(
            throughTable,
+            function () {
+              for (let relationship of relationships) {
+                const fromPrimary = relationship.fromPrimary
+                const from = relationship.from
+                // @ts-ignore
+                this.orOn(
                  `${fromTable}.${fromPrimary}`,
+                  "=",
                  `${throughTable}.${from}`
                )
-          .leftJoin(toTable, `${toTable}.${toPrimary}`, `${throughTable}.${to}`)
+              }
+            },
+            "left"
+          )
+          .join(
+            toTable,
+            function () {
+              for (let relationship of relationships) {
+                const toPrimary = relationship.toPrimary
+                const to = relationship.to
+                // @ts-ignore
+                this.orOn(`${toTable}.${toPrimary}`, `${throughTable}.${to}`)
+              }
+            },
+            "left"
+          )
      }
    }
    return query.limit(BASE_LIMIT)