Only select required columns from sql databases

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

@@ -272,17 +272,6 @@ class InternalBuilder {
     return parts.join(".")
   }
 
-  private isFullSelectStatementRequired(): boolean {
-    for (let column of Object.values(this.table.schema)) {
-      if (this.SPECIAL_SELECT_CASES.POSTGRES_MONEY(column)) {
-        return true
-      } else if (this.SPECIAL_SELECT_CASES.MSSQL_DATES(column)) {
-        return true
-      }
-    }
-    return false
-  }
-
   private generateSelectStatement(): (string | Knex.Raw)[] | "*" {
     const { table, resource } = this.query
 
@@ -292,11 +281,9 @@ class InternalBuilder {
 
     const alias = this.getTableName(table)
     const schema = this.table.schema
-    if (!this.isFullSelectStatementRequired()) {
-      return [this.knex.raw("??", [`${alias}.*`])]
-    }
+
     // get just the fields for this table
-    return resource.fields
+    const tableFields = resource.fields
       .map(field => {
         const parts = field.split(/\./g)
         let table: string | undefined = undefined
@@ -311,34 +298,35 @@ class InternalBuilder {
         return { table, column, field }
       })
       .filter(({ table }) => !table || table === alias)
-      .map(({ table, column, field }) => {
-        const columnSchema = schema[column]
 
-        if (this.SPECIAL_SELECT_CASES.POSTGRES_MONEY(columnSchema)) {
-          return this.knex.raw(`??::money::numeric as ??`, [
-            this.rawQuotedIdentifier([table, column].join(".")),
-            this.knex.raw(this.quote(field)),
-          ])
-        }
+    return tableFields.map(({ table, column, field }) => {
+      const columnSchema = schema[column]
 
-        if (this.SPECIAL_SELECT_CASES.MSSQL_DATES(columnSchema)) {
-          // Time gets returned as timestamp from mssql, not matching the expected
-          // HH:mm format
+      if (this.SPECIAL_SELECT_CASES.POSTGRES_MONEY(columnSchema)) {
+        return this.knex.raw(`??::money::numeric as ??`, [
+          this.rawQuotedIdentifier([table, column].join(".")),
+          this.knex.raw(this.quote(field)),
+        ])
+      }
 
-          // TODO: figure out how to express this safely without string
-          // interpolation.
-          return this.knex.raw(`CONVERT(varchar, ??, 108) as ??`, [
-            this.rawQuotedIdentifier(field),
-            this.knex.raw(this.quote(field)),
-          ])
-        }
+      if (this.SPECIAL_SELECT_CASES.MSSQL_DATES(columnSchema)) {
+        // Time gets returned as timestamp from mssql, not matching the expected
+        // HH:mm format
 
-        if (table) {
-          return this.rawQuotedIdentifier(`${table}.${column}`)
-        } else {
-          return this.rawQuotedIdentifier(field)
-        }
-      })
+        // TODO: figure out how to express this safely without string
+        // interpolation.
+        return this.knex.raw(`CONVERT(varchar, ??, 108) as ??`, [
+          this.rawQuotedIdentifier(field),
+          this.knex.raw(this.quote(field)),
+        ])
+      }
+
+      if (table) {
+        return this.rawQuotedIdentifier(`${table}.${column}`)
+      } else {
+        return this.rawQuotedIdentifier(field)
+      }
+    })
   }
 
   // OracleDB can't use character-large-objects (CLOBs) in WHERE clauses,
@@ -1239,6 +1227,7 @@ class InternalBuilder {
       if (!toTable || !fromTable) {
         continue
       }
+
       const relatedTable = tables[toTable]
       if (!relatedTable) {
         throw new Error(`related table "${toTable}" not found in datasource`)
@@ -1267,6 +1256,10 @@ class InternalBuilder {
       const fieldList = relationshipFields.map(field =>
         this.buildJsonField(relatedTable, field)
       )
+      if (!fieldList.length) {
+        continue
+      }
+
       const fieldListFormatted = fieldList
         .map(f => {
           const separator = this.client === SqlClient.ORACLE ? " VALUE " : ","
@@ -1307,7 +1300,9 @@ class InternalBuilder {
       )
 
       const standardWrap = (select: Knex.Raw): Knex.QueryBuilder => {
-        subQuery = subQuery.select(`${toAlias}.*`).limit(getRelationshipLimit())
+        subQuery = subQuery
+          .select(relationshipFields)
+          .limit(getRelationshipLimit())
         // @ts-ignore - the from alias syntax isn't in Knex typing
         return knex.select(select).from({
           [toAlias]: subQuery,
@@ -1537,11 +1532,12 @@ class InternalBuilder {
       limits?: { base: number; query: number }
     } = {}
   ): Knex.QueryBuilder {
-    let { operation, filters, paginate, relationships, table } = this.query
+    const { operation, filters, paginate, relationships, table } = this.query
     const { limits } = opts
 
     // start building the query
     let query = this.qualifiedKnex()
+
     // handle pagination
     let foundOffset: number | null = null
     let foundLimit = limits?.query || limits?.base
@@ -1590,7 +1586,7 @@ class InternalBuilder {
       const mainTable = this.query.tableAliases?.[table.name] || table.name
       const cte = this.addSorting(
         this.knex
-          .with("paginated", query)
+          .with("paginated", query.clone().clearSelect().select("*"))
           .select(this.generateSelectStatement())
           .from({
             [mainTable]: "paginated",

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

@@ -45,6 +45,9 @@ export async function handleRequest<T extends Operation>(
 export async function patch(ctx: UserCtx<PatchRowRequest, PatchRowResponse>) {
   const source = await utils.getSource(ctx)
 
+  const { viewId, tableId } = utils.getSourceId(ctx)
+  const sourceId = viewId || tableId
+
   if (sdk.views.isView(source) && helpers.views.isCalculationView(source)) {
     ctx.throw(400, "Cannot update rows through a calculation view")
   }
@@ -86,7 +89,7 @@ export async function patch(ctx: UserCtx<PatchRowRequest, PatchRowResponse>) {
   // The id might have been changed, so the refetching would fail. Recalculating the id just in case
   const updatedId =
     generateIdForRow({ ...beforeRow, ...dataToUpdate }, table) || _id
-  const row = await sdk.rows.external.getRow(table._id!, updatedId, {
+  const row = await sdk.rows.external.getRow(sourceId, updatedId, {
     relationships: true,
   })
 

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

@@ -14,7 +14,8 @@ import {
 import { breakExternalTableId } from "../../../../integrations/utils"
 import { generateJunctionTableID } from "../../../../db/utils"
 import sdk from "../../../../sdk"
-import { helpers } from "@budibase/shared-core"
+import { helpers, PROTECTED_INTERNAL_COLUMNS } from "@budibase/shared-core"
+import { sql } from "@budibase/backend-core"
 
 type TableMap = Record<string, Table>
 
@@ -118,45 +119,131 @@ export async function buildSqlFieldList(
   opts?: { relationships: boolean }
 ) {
   const { relationships } = opts || {}
+
+  const nonMappedColumns = [FieldType.LINK, FieldType.FORMULA, FieldType.AI]
+
   function extractRealFields(table: Table, existing: string[] = []) {
     return Object.entries(table.schema)
       .filter(
         ([columnName, column]) =>
-          column.type !== FieldType.LINK &&
-          column.type !== FieldType.FORMULA &&
-          column.type !== FieldType.AI &&
-          !existing.find(
-            (field: string) => field === `${table.name}.${columnName}`
-          )
+          !nonMappedColumns.includes(column.type) &&
+          !existing.find((field: string) => field === columnName)
       )
-      .map(([columnName]) => `${table.name}.${columnName}`)
+      .map(([columnName]) => columnName)
   }
 
-  let fields: string[] = []
-  if (sdk.views.isView(source)) {
-    fields = Object.keys(helpers.views.basicFields(source))
-  } else {
-    fields = extractRealFields(source)
+  function getRequiredFields(table: Table, existing: string[] = []) {
+    const requiredFields: string[] = []
+    if (table.primary) {
+      requiredFields.push(...table.primary)
+    }
+    if (table.primaryDisplay) {
+      requiredFields.push(table.primaryDisplay)
+    }
+
+    if (!sql.utils.isExternalTable(table)) {
+      requiredFields.push(...PROTECTED_INTERNAL_COLUMNS)
+    }
+
+    return requiredFields.filter(
+      column =>
+        !existing.find((field: string) => field === column) &&
+        table.schema[column] &&
+        !nonMappedColumns.includes(table.schema[column].type)
+    )
   }
 
+  let fields: string[] = []
+
+  const isView = sdk.views.isView(source)
+
   let table: Table
-  if (sdk.views.isView(source)) {
+  if (isView) {
     table = await sdk.views.getTable(source.id)
+
+    fields = Object.keys(helpers.views.basicFields(source)).filter(
+      f => table.schema[f].type !== FieldType.LINK
+    )
   } else {
     table = source
+    fields = extractRealFields(source).filter(
+      f => table.schema[f].visible !== false
+    )
   }
 
-  for (let field of Object.values(table.schema)) {
+  const containsFormula = (isView ? fields : Object.keys(table.schema)).some(
+    f => table.schema[f]?.type === FieldType.FORMULA
+  )
+  // If are requesting for a formula field, we need to retrieve all fields
+  if (containsFormula) {
+    fields = extractRealFields(table)
+  }
+
+  if (!isView || !helpers.views.isCalculationView(source)) {
+    fields.push(
+      ...getRequiredFields(
+        {
+          ...table,
+          primaryDisplay: source.primaryDisplay || table.primaryDisplay,
+        },
+        fields
+      )
+    )
+  }
+
+  fields = fields.map(c => `${table.name}.${c}`)
+
+  for (const field of Object.values(table.schema)) {
     if (field.type !== FieldType.LINK || !relationships || !field.tableId) {
       continue
     }
+
+    if (
+      isView &&
+      (!source.schema?.[field.name] ||
+        !helpers.views.isVisible(source.schema[field.name])) &&
+      !containsFormula
+    ) {
+      continue
+    }
+
     const { tableName } = breakExternalTableId(field.tableId)
-    if (tables[tableName]) {
-      fields = fields.concat(extractRealFields(tables[tableName], fields))
+    const relatedTable = tables[tableName]
+    if (!relatedTable) {
+      continue
     }
+
+    const viewFields = new Set<string>()
+    if (containsFormula) {
+      extractRealFields(relatedTable).forEach(f => viewFields.add(f))
+    } else {
+      relatedTable.primary?.forEach(f => viewFields.add(f))
+      if (relatedTable.primaryDisplay) {
+        viewFields.add(relatedTable.primaryDisplay)
+      }
+
+      if (isView) {
+        Object.entries(source.schema?.[field.name]?.columns || {})
+          .filter(
+            ([columnName, columnConfig]) =>
+              relatedTable.schema[columnName] &&
+              helpers.views.isVisible(columnConfig) &&
+              ![FieldType.LINK, FieldType.FORMULA].includes(
+                relatedTable.schema[columnName].type
+              )
+          )
+          .forEach(([field]) => viewFields.add(field))
+      }
+    }
+
+    const fieldsToAdd = Array.from(viewFields)
+      .filter(f => !nonMappedColumns.includes(relatedTable.schema[f].type))
+      .map(f => `${relatedTable.name}.${f}`)
+      .filter(f => !fields.includes(f))
+    fields.push(...fieldsToAdd)
   }
 
-  return fields
+  return [...new Set(fields)]
 }
 
 export function isKnexEmptyReadResponse(resp: DatasourcePlusQueryResponse) {