Skip to content

Commit

Permalink
Merge pull request #15169 from Budibase/BUDI-8885/only-select-require…
Browse files Browse the repository at this point in the history 
…d-columns-from-sql-databases

Only select required columns from sql databases
  • Loading branch information
@adrinr
adrinr authored Jan 13, 2025
2 parents 0468dac + 593c74e commit 05b2c07
Show file tree
Hide file tree
Showing 9 changed files with 1,486 additions and 84 deletions.
78 changes: 36 additions & 42 deletions packages/backend-core/src/sql/sql.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -311,34 +298,33 @@ 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)
}
})
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,
Expand Down Expand Up @@ -1291,6 +1277,7 @@ class InternalBuilder {
if (!toTable || !fromTable) {
continue
}

const relatedTable = tables[toTable]
if (!relatedTable) {
throw new Error(`related table "${toTable}" not found in datasource`)
Expand Down Expand Up @@ -1319,6 +1306,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 " : ","
Expand Down Expand Up @@ -1359,7 +1350,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,
Expand Down Expand Up @@ -1589,11 +1582,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
Expand Down Expand Up @@ -1642,7 +1636,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",
Expand Down
5 changes: 4 additions & 1 deletion packages/server/src/api/controllers/row/external.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down Expand Up @@ -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,
})

Expand Down
123 changes: 105 additions & 18 deletions packages/server/src/api/controllers/row/utils/sqlUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 05b2c07

Please sign in to comment.