From 544302a12992b22eed209626d01fe12567f006e3 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 2 Dec 2024 12:42:48 +0100 Subject: [PATCH 01/51] Request only needed fields --- packages/backend-core/src/sql/sql.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 5f462ee1446..a4828a9f4ae 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -1537,11 +1537,16 @@ class InternalBuilder { limits?: { base: number; query: number } } = {} ): Knex.QueryBuilder { - let { operation, filters, paginate, relationships, table } = this.query + const { operation, filters, paginate, relationships, table, resource } = + this.query const { limits } = opts // start building the query let query = this.qualifiedKnex() + if (resource?.fields) { + query = query.columns(resource?.fields) + } + // handle pagination let foundOffset: number | null = null let foundLimit = limits?.query || limits?.base From 0e0b6471d64e93f2559ab51374c734f5d103eed9 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 11 Dec 2024 10:22:16 +0100 Subject: [PATCH 02/51] Generate proper select --- packages/backend-core/src/sql/sql.ts | 5 +---- packages/server/src/api/controllers/row/utils/sqlUtils.ts | 1 + 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index a4828a9f4ae..76d554d2119 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -292,7 +292,7 @@ class InternalBuilder { const alias = this.getTableName(table) const schema = this.table.schema - if (!this.isFullSelectStatementRequired()) { + if (this.isFullSelectStatementRequired()) { return [this.knex.raw("??", [`${alias}.*`])] } // get just the fields for this table @@ -1543,9 +1543,6 @@ class InternalBuilder { // start building the query let query = this.qualifiedKnex() - if (resource?.fields) { - query = query.columns(resource?.fields) - } // handle pagination let foundOffset: number | null = null diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index af696c0758e..9729018bf74 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -142,6 +142,7 @@ export async function buildSqlFieldList( let table: Table if (sdk.views.isView(source)) { table = await sdk.views.getTable(source.id) + fields = fields.filter(f => table.schema[f].type !== FieldType.LINK) } else { table = source } From 191b63270cabd2a76eb85c401f6308821d02de47 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 11 Dec 2024 13:30:08 +0100 Subject: [PATCH 03/51] Dry --- packages/server/src/api/controllers/row/utils/sqlUtils.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 9729018bf74..19f251bef38 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -133,14 +133,17 @@ export async function buildSqlFieldList( } let fields: string[] = [] - if (sdk.views.isView(source)) { + + const isView = sdk.views.isView(source) + + if (isView) { fields = Object.keys(helpers.views.basicFields(source)) } else { fields = extractRealFields(source) } let table: Table - if (sdk.views.isView(source)) { + if (isView) { table = await sdk.views.getTable(source.id) fields = fields.filter(f => table.schema[f].type !== FieldType.LINK) } else { From a415095c470f2d8a59575c4b6a38c0e0ecc67a5b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 11 Dec 2024 14:09:11 +0100 Subject: [PATCH 04/51] Lint --- packages/backend-core/src/sql/sql.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 76d554d2119..758dfa7fec7 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -1537,8 +1537,7 @@ class InternalBuilder { limits?: { base: number; query: number } } = {} ): Knex.QueryBuilder { - const { operation, filters, paginate, relationships, table, resource } = - this.query + const { operation, filters, paginate, relationships, table } = this.query const { limits } = opts // start building the query From 74e1bbc7c66faa507edac9e56eb1214aa0b93384 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 12 Dec 2024 13:57:44 +0100 Subject: [PATCH 05/51] Require only visible fields on views --- .../server/src/api/controllers/row/utils/sqlUtils.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 19f251bef38..b67ef6be92d 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -150,10 +150,19 @@ export async function buildSqlFieldList( table = source } - for (let field of Object.values(table.schema)) { + 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]) + ) { + continue + } + const { tableName } = breakExternalTableId(field.tableId) if (tables[tableName]) { fields = fields.concat(extractRealFields(tables[tableName], fields)) From e3c9156aef80c593b35936f853fba064936e8b04 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 12 Dec 2024 13:58:37 +0100 Subject: [PATCH 06/51] Trim selected fields --- packages/backend-core/src/sql/sql.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 758dfa7fec7..4691cd71ba9 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -1591,7 +1591,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", From 00557e5f7ea87f44dfe0cc6ca7202a3af28653e9 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 12 Dec 2024 14:01:56 +0100 Subject: [PATCH 07/51] Don't include unnecessary joins --- packages/backend-core/src/sql/sql.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 4691cd71ba9..0fe7e0ca576 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -1239,6 +1239,12 @@ class InternalBuilder { if (!toTable || !fromTable) { continue } + + // Don't include if not required + if (relationship.from && !fields.find(f => f === relationship.from)) { + continue + } + const relatedTable = tables[toTable] if (!relatedTable) { throw new Error(`related table "${toTable}" not found in datasource`) From 910faa6f33720cc3b396449cf8e036a6f77534ca Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 12 Dec 2024 17:10:01 +0100 Subject: [PATCH 08/51] Fix getting relations --- packages/backend-core/src/sql/sql.ts | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 0fe7e0ca576..24f30d57faf 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -1240,11 +1240,6 @@ class InternalBuilder { continue } - // Don't include if not required - if (relationship.from && !fields.find(f => f === relationship.from)) { - continue - } - const relatedTable = tables[toTable] if (!relatedTable) { throw new Error(`related table "${toTable}" not found in datasource`) @@ -1594,17 +1589,8 @@ class InternalBuilder { // handle relationships with a CTE for all others if (relationships?.length && aggregations.length === 0) { - const mainTable = this.query.tableAliases?.[table.name] || table.name - const cte = this.addSorting( - this.knex - .with("paginated", query.clone().clearSelect().select("*")) - .select(this.generateSelectStatement()) - .from({ - [mainTable]: "paginated", - }) - ) // add JSON aggregations attached to the CTE - return this.addJsonRelationships(cte, table.name, relationships) + return this.addJsonRelationships(query, table.name, relationships) } return query From be0074105cc8126c2029acc76b03bad8846b998e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 12 Dec 2024 17:25:07 +0100 Subject: [PATCH 09/51] Request relation only when required --- packages/backend-core/src/sql/sql.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 24f30d57faf..c38c0bd2c81 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -1268,6 +1268,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 " : "," From 112b7c70e23e6d2c2b3f0d1f655ff1d622391ddf Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 13 Dec 2024 11:21:24 +0100 Subject: [PATCH 10/51] Fix sqs views --- .../server/src/sdk/app/rows/search/internal/sqs.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index aa799390b83..6eb9bd913b1 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -234,6 +234,17 @@ async function runSqlQuery( json.operation = Operation.COUNT } const processSQLQuery = async (json: EnrichedQueryJson) => { + const fields = json.resource?.fields + if (fields) { + const tableId = json.tableAliases?.[json.table._id!] ?? json.table._id! + for (const key of ["_id", "_rev", "tableId"]) { + const field = `${tableId}.${key}` + if (fields.includes(field)) { + continue + } + fields.push(field) + } + } const query = builder._query(json, { disableReturning: true, }) From bbb60afdc2d01c5c0f9e6e1bc4196139a351645b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 13 Dec 2024 13:30:39 +0100 Subject: [PATCH 11/51] Relations, select only required fields --- .../src/api/controllers/row/utils/sqlUtils.ts | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index b67ef6be92d..51fba4bb79d 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -164,8 +164,24 @@ export async function buildSqlFieldList( } const { tableName } = breakExternalTableId(field.tableId) - if (tables[tableName]) { - fields = fields.concat(extractRealFields(tables[tableName], fields)) + const relatedTable = tables[tableName] + if (relatedTable) { + const viewFields = new Set() + relatedTable.primary?.forEach(f => viewFields.add(f)) + if (relatedTable.primaryDisplay) { + viewFields.add(relatedTable.primaryDisplay) + } + + if (isView) { + Object.entries(source.schema?.[field.name].columns || {}) + .filter(([_, column]) => helpers.views.isVisible(column)) + .forEach(([field]) => viewFields.add(field)) + } + + const fieldsToAdd = Array.from(viewFields) + .map(f => `${relatedTable.name}.${f}`) + .filter(f => !fields.includes(f)) + fields.push(...fieldsToAdd) } } From 80dcc519235d419ed3224ac729c747beebd86517 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 09:55:18 +0100 Subject: [PATCH 12/51] Fix sql alias tests --- .../src/integrations/tests/sqlAlias.spec.ts | 35 ++++++++----------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index 7c6f583762a..f7dab0ff46e 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -79,7 +79,7 @@ describe("Captures of real examples", () => { queryJson ) expect(query).toEqual({ - bindings: [primaryLimit, relationshipLimit, relationshipLimit], + bindings: [relationshipLimit, relationshipLimit, primaryLimit], sql: expect.stringContaining( multiline( `select json_agg(json_build_object('executorid',"b"."executorid",'executorid',"b"."executorid",'qaid',"b"."qaid",'qaid',"b"."qaid",'taskid',"b"."taskid",'taskid',"b"."taskid",'completed',"b"."completed",'completed',"b"."completed",'taskname',"b"."taskname",'taskname',"b"."taskname"` @@ -94,10 +94,10 @@ describe("Captures of real examples", () => { queryJson ) expect(query).toEqual({ - bindings: ["assembling", primaryLimit, relationshipLimit], + bindings: [relationshipLimit, "assembling", primaryLimit], sql: expect.stringContaining( multiline( - `where (exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" and (COALESCE("b"."taskname" = $1, FALSE)))` + `where (exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" and (COALESCE("b"."taskname" = $2, FALSE)))` ) ), }) @@ -109,13 +109,13 @@ describe("Captures of real examples", () => { queryJson ) expect(query).toEqual({ - bindings: [primaryLimit, relationshipLimit], + bindings: [relationshipLimit, primaryLimit], sql: expect.stringContaining( multiline( - `with "paginated" as (select "a".* from "products" as "a" order by "a"."productname" asc nulls first, "a"."productid" asc limit $1) - select "a".*, (select json_agg(json_build_object('executorid',"b"."executorid",'qaid',"b"."qaid",'taskid',"b"."taskid",'completed',"b"."completed",'taskname',"b"."taskname")) - from (select "b".* from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" order by "b"."taskid" asc limit $2) as "b") as "tasks" - from "paginated" as "a" order by "a"."productname" asc nulls first, "a"."productid" asc` + `select "a"."productname", + "a"."productid", + (select json_agg(json_build_object('executorid',"b"."executorid",'qaid',"b"."qaid",'taskid',"b"."taskid",'completed',"b"."completed",'taskname',"b"."taskname")) from (select "b".* from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" order by "b"."taskid" asc limit $1) as "b") as "tasks" + from "products" as "a" order by "a"."productname" asc nulls first, "a"."productid" asc limit $2` ) ), }) @@ -128,13 +128,8 @@ describe("Captures of real examples", () => { queryJson ) expect(query).toEqual({ - bindings: [...filters, relationshipLimit, relationshipLimit], - sql: multiline( - `with "paginated" as (select "a".* from "tasks" as "a" where "a"."taskid" in ($1, $2) order by "a"."taskid" asc limit $3) - select "a".*, (select json_agg(json_build_object('productid',"b"."productid",'productname',"b"."productname")) - from (select "b".* from "products" as "b" inner join "products_tasks" as "c" on "b"."productid" = "c"."productid" - where "c"."taskid" = "a"."taskid" order by "b"."productid" asc limit $4) as "b") as "products" from "paginated" as "a" order by "a"."taskid" asc` - ), + bindings: [relationshipLimit, ...filters, relationshipLimit], + sql: `select "a"."executorid", "a"."taskname", "a"."taskid", "a"."completed", "a"."qaid", (select json_agg(json_build_object('productid',"b"."productid",'productname',"b"."productname")) from (select "b".* from "products" as "b" inner join "products_tasks" as "c" on "b"."productid" = "c"."productid" where "c"."taskid" = "a"."taskid" order by "b"."productid" asc limit $1) as "b") as "products" from "tasks" as "a" where "a"."taskid" in ($2, $3) order by "a"."taskid" asc limit $4`, }) }) @@ -151,6 +146,9 @@ describe("Captures of real examples", () => { expect(query).toEqual({ bindings: [ + relationshipLimit, + relationshipLimit, + relationshipLimit, rangeValue.low, rangeValue.high, rangeValue.low, @@ -158,13 +156,10 @@ describe("Captures of real examples", () => { equalValue, notEqualsValue, primaryLimit, - relationshipLimit, - relationshipLimit, - relationshipLimit, ], sql: expect.stringContaining( multiline( - `where (exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2))) and (exists (select 1 from "persons" as "c" where "c"."personid" = "a"."qaid" and ("c"."year" between $3 and $4))) and (exists (select 1 from "products" as "b" inner join "products_tasks" as "d" on "b"."productid" = "d"."productid" where "d"."taskid" = "a"."taskid" and (COALESCE("b"."productname" = $5, FALSE))))` + `where (exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $4 and $5))) and (exists (select 1 from "persons" as "c" where "c"."personid" = "a"."qaid" and ("c"."year" between $6 and $7))) and (exists (select 1 from "products" as "b" inner join "products_tasks" as "d" on "b"."productid" = "d"."productid" where "d"."taskid" = "a"."taskid" and (COALESCE("b"."productname" = $8, FALSE))))` ) ), }) @@ -209,7 +204,7 @@ describe("Captures of real examples", () => { bindings: ["ddd", ""], sql: multiline(`delete from "compositetable" as "a" where COALESCE("a"."keypartone" = $1, FALSE) and COALESCE("a"."keyparttwo" = $2, FALSE) - returning "a".*`), + returning "a"."keyparttwo", "a"."keypartone", "a"."name"`), }) }) }) From f92fcea42abab49afc413aba4f51cb3b4c650b14 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 10:41:37 +0100 Subject: [PATCH 13/51] Include * when having formulas --- packages/backend-core/src/sql/sql.ts | 72 +++++++++++-------- .../src/api/controllers/row/utils/sqlUtils.ts | 2 +- 2 files changed, 43 insertions(+), 31 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index c38c0bd2c81..337c65ff0fe 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -272,12 +272,17 @@ class InternalBuilder { return parts.join(".") } - private isFullSelectStatementRequired(): boolean { - for (let column of Object.values(this.table.schema)) { + private isFullSelectStatementRequired(includedFields: string[]): boolean { + for (const 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 + } else if ( + column.type === FieldType.FORMULA && + includedFields.includes(column.name) + ) { + return true } } return false @@ -292,11 +297,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 +314,43 @@ 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)), - ]) - } + if ( + this.isFullSelectStatementRequired( + tableFields.map(({ column }) => column) + ) + ) { + return [this.knex.raw("??", [`${alias}.*`])] + } - if (this.SPECIAL_SELECT_CASES.MSSQL_DATES(columnSchema)) { - // Time gets returned as timestamp from mssql, not matching the expected - // HH:mm format + return tableFields.map(({ table, column, field }) => { + const columnSchema = schema[column] - // 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.POSTGRES_MONEY(columnSchema)) { + return this.knex.raw(`??::money::numeric as ??`, [ + this.rawQuotedIdentifier([table, column].join(".")), + this.knex.raw(this.quote(field)), + ]) + } - if (table) { - return this.rawQuotedIdentifier(`${table}.${column}`) - } else { - return this.rawQuotedIdentifier(field) - } - }) + if (this.SPECIAL_SELECT_CASES.MSSQL_DATES(columnSchema)) { + // Time gets returned as timestamp from mssql, not matching the expected + // HH:mm format + + // 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, diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 51fba4bb79d..2d5fc35761a 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -173,7 +173,7 @@ export async function buildSqlFieldList( } if (isView) { - Object.entries(source.schema?.[field.name].columns || {}) + Object.entries(source.schema?.[field.name]?.columns || {}) .filter(([_, column]) => helpers.views.isVisible(column)) .forEach(([field]) => viewFields.add(field)) } From f8ece826488927c314fec3245a4d4224cd66c7c3 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 10:53:45 +0100 Subject: [PATCH 14/51] Fix formulas on internal --- packages/backend-core/src/sql/sql.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 337c65ff0fe..ba3076e553d 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -315,11 +315,10 @@ class InternalBuilder { }) .filter(({ table }) => !table || table === alias) - if ( - this.isFullSelectStatementRequired( - tableFields.map(({ column }) => column) - ) - ) { + const requestedTableColumns = tableFields.map(({ column }) => + column.replace(new RegExp(`^${this.query.meta?.columnPrefix}`), "") + ) + if (this.isFullSelectStatementRequired(requestedTableColumns)) { return [this.knex.raw("??", [`${alias}.*`])] } From e6a27ad4d50ebc4776091a0020c4afbc3a44ff17 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 12:25:29 +0100 Subject: [PATCH 15/51] Ensure required fields are included --- .../src/api/controllers/row/utils/sqlUtils.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 2d5fc35761a..0c3d2b8607a 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -15,6 +15,7 @@ import { breakExternalTableId } from "../../../../integrations/utils" import { generateJunctionTableID } from "../../../../db/utils" import sdk from "../../../../sdk" import { helpers } from "@budibase/shared-core" +import { sql } from "@budibase/backend-core" type TableMap = Record @@ -132,6 +133,27 @@ export async function buildSqlFieldList( .map(([columnName]) => `${table.name}.${columnName}`) } + 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(...["_id", "_rev", "_tableId"]) + } + + return requiredFields + .filter( + column => + !existing.find((field: string) => field === `${table.name}.${column}`) + ) + .map(column => `${table.name}.${column}`) + } + let fields: string[] = [] const isView = sdk.views.isView(source) @@ -150,6 +172,16 @@ export async function buildSqlFieldList( table = source } + fields.push( + ...getRequiredFields( + { + ...table, + primaryDisplay: source.primaryDisplay || table.primaryDisplay, + }, + fields + ) + ) + for (const field of Object.values(table.schema)) { if (field.type !== FieldType.LINK || !relationships || !field.tableId) { continue From 2d771c96dd81f663ea6360cb6e3997138f03e045 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 12:53:40 +0100 Subject: [PATCH 16/51] Fix wrong relationship select --- .../server/src/api/controllers/row/utils/sqlUtils.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 0c3d2b8607a..80c5f95b05d 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -206,7 +206,14 @@ export async function buildSqlFieldList( if (isView) { Object.entries(source.schema?.[field.name]?.columns || {}) - .filter(([_, column]) => helpers.views.isVisible(column)) + .filter( + ([columnName, columnConfig]) => + relatedTable.schema[columnName] && + helpers.views.isVisible(columnConfig) && + ![FieldType.LINK, FieldType.FORMULA].includes( + relatedTable.schema[columnName].type + ) + ) .forEach(([field]) => viewFields.add(field)) } From 14b5a4264700e66bf2a7aef0f130bc41d034e885 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 13:57:38 +0100 Subject: [PATCH 17/51] Fix patch issues --- packages/server/src/api/controllers/row/external.ts | 7 +++++-- packages/server/src/api/controllers/row/utils/utils.ts | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/controllers/row/external.ts b/packages/server/src/api/controllers/row/external.ts index 082d07283b5..6dbd4dbb815 100644 --- a/packages/server/src/api/controllers/row/external.ts +++ b/packages/server/src/api/controllers/row/external.ts @@ -45,6 +45,9 @@ export async function handleRequest( export async function patch(ctx: UserCtx) { 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") } @@ -66,7 +69,7 @@ export async function patch(ctx: UserCtx) { throw { validation: validateResult.errors } } - const beforeRow = await sdk.rows.external.getRow(table._id!, _id, { + const beforeRow = await sdk.rows.external.getRow(sourceId, _id, { relationships: true, }) @@ -78,7 +81,7 @@ export async function patch(ctx: UserCtx) { // 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, }) diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index baa811fe909..8701ba91978 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -66,7 +66,7 @@ export function getSourceId(ctx: Ctx): { tableId: string; viewId?: string } { if (docIds.isViewId(sourceId)) { return { tableId: utils.extractViewInfoFromID(sourceId).tableId, - viewId: sourceId, + viewId: encodeURI(sourceId), } } return { tableId: sql.utils.encodeTableId(ctx.params.sourceId) } From d60cc7aaf4c4c47ff80d2caecac0837ae1c9c8fd Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 15:51:17 +0100 Subject: [PATCH 18/51] Fix encoding --- packages/server/src/api/controllers/row/utils/utils.ts | 2 +- packages/server/src/db/utils.ts | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index 8701ba91978..021a6317ea6 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -66,7 +66,7 @@ export function getSourceId(ctx: Ctx): { tableId: string; viewId?: string } { if (docIds.isViewId(sourceId)) { return { tableId: utils.extractViewInfoFromID(sourceId).tableId, - viewId: encodeURI(sourceId), + viewId: sql.utils.encodeTableId(sourceId), } } return { tableId: sql.utils.encodeTableId(ctx.params.sourceId) } diff --git a/packages/server/src/db/utils.ts b/packages/server/src/db/utils.ts index 70c69b3c60f..6c1065e847a 100644 --- a/packages/server/src/db/utils.ts +++ b/packages/server/src/db/utils.ts @@ -1,10 +1,4 @@ -import { - context, - db as dbCore, - docIds, - utils, - sql, -} from "@budibase/backend-core" +import { context, db as dbCore, docIds, utils } from "@budibase/backend-core" import { DatabaseQueryOpts, Datasource, @@ -334,7 +328,7 @@ export function extractViewInfoFromID(viewId: string) { const regex = new RegExp(`^(?.+)${SEPARATOR}([^${SEPARATOR}]+)$`) const res = regex.exec(viewId) return { - tableId: sql.utils.encodeTableId(res!.groups!["tableId"]), + tableId: res!.groups!["tableId"], } } From 0ef4a154ef9b98b41eed2fcfdf424ff6ba3a4724 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 16:02:10 +0100 Subject: [PATCH 19/51] Prevent repeated fields on select --- .../src/api/controllers/row/utils/sqlUtils.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 80c5f95b05d..b6215cc5b61 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -158,12 +158,6 @@ export async function buildSqlFieldList( const isView = sdk.views.isView(source) - if (isView) { - fields = Object.keys(helpers.views.basicFields(source)) - } else { - fields = extractRealFields(source) - } - let table: Table if (isView) { table = await sdk.views.getTable(source.id) @@ -172,6 +166,14 @@ export async function buildSqlFieldList( table = source } + if (isView) { + fields = Object.keys(helpers.views.basicFields(source)).map( + c => `${table.name}.${c}` + ) + } else { + fields = extractRealFields(source) + } + fields.push( ...getRequiredFields( { From fc22db35ac5a5d341f617446789fdc6d06cb6655 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 16:16:22 +0100 Subject: [PATCH 20/51] Add back cte --- packages/backend-core/src/sql/sql.ts | 11 ++++++++++- .../server/src/api/controllers/row/utils/sqlUtils.ts | 7 +++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index ba3076e553d..1a7e3beba4b 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -1604,8 +1604,17 @@ class InternalBuilder { // handle relationships with a CTE for all others if (relationships?.length && aggregations.length === 0) { + const mainTable = this.query.tableAliases?.[table.name] || table.name + const cte = this.addSorting( + this.knex + .with("paginated", query.clone().clearSelect().select("*")) + .select(this.generateSelectStatement()) + .from({ + [mainTable]: "paginated", + }) + ) // add JSON aggregations attached to the CTE - return this.addJsonRelationships(query, table.name, relationships) + return this.addJsonRelationships(cte, table.name, relationships) } return query diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index b6215cc5b61..9f5e14d732a 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -161,15 +161,14 @@ export async function buildSqlFieldList( let table: Table if (isView) { table = await sdk.views.getTable(source.id) - fields = fields.filter(f => table.schema[f].type !== FieldType.LINK) } else { table = source } if (isView) { - fields = Object.keys(helpers.views.basicFields(source)).map( - c => `${table.name}.${c}` - ) + fields = Object.keys(helpers.views.basicFields(source)) + .filter(f => table.schema[f].type !== FieldType.LINK) + .map(c => `${table.name}.${c}`) } else { fields = extractRealFields(source) } From aa288966d89e40ca85a8286f39a8f8eb13cec607 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 16:23:05 +0100 Subject: [PATCH 21/51] Fix tests back --- .../src/integrations/tests/sqlAlias.spec.ts | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index f7dab0ff46e..1630e9b2042 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -79,7 +79,7 @@ describe("Captures of real examples", () => { queryJson ) expect(query).toEqual({ - bindings: [relationshipLimit, relationshipLimit, primaryLimit], + bindings: [primaryLimit, relationshipLimit, relationshipLimit], sql: expect.stringContaining( multiline( `select json_agg(json_build_object('executorid',"b"."executorid",'executorid',"b"."executorid",'qaid',"b"."qaid",'qaid',"b"."qaid",'taskid',"b"."taskid",'taskid',"b"."taskid",'completed',"b"."completed",'completed',"b"."completed",'taskname',"b"."taskname",'taskname',"b"."taskname"` @@ -94,10 +94,10 @@ describe("Captures of real examples", () => { queryJson ) expect(query).toEqual({ - bindings: [relationshipLimit, "assembling", primaryLimit], + bindings: ["assembling", primaryLimit, relationshipLimit], sql: expect.stringContaining( multiline( - `where (exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" and (COALESCE("b"."taskname" = $2, FALSE)))` + `where (exists (select 1 from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" and (COALESCE("b"."taskname" = $1, FALSE)))` ) ), }) @@ -109,13 +109,13 @@ describe("Captures of real examples", () => { queryJson ) expect(query).toEqual({ - bindings: [relationshipLimit, primaryLimit], + bindings: [primaryLimit, relationshipLimit], sql: expect.stringContaining( multiline( - `select "a"."productname", - "a"."productid", - (select json_agg(json_build_object('executorid',"b"."executorid",'qaid',"b"."qaid",'taskid',"b"."taskid",'completed',"b"."completed",'taskname',"b"."taskname")) from (select "b".* from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" order by "b"."taskid" asc limit $1) as "b") as "tasks" - from "products" as "a" order by "a"."productname" asc nulls first, "a"."productid" asc limit $2` + `with "paginated" as (select * from "products" as "a" order by "a"."productname" asc nulls first, "a"."productid" asc limit $1) + select "a"."productname", "a"."productid", (select json_agg(json_build_object('executorid',"b"."executorid",'qaid',"b"."qaid",'taskid',"b"."taskid",'completed',"b"."completed",'taskname',"b"."taskname")) + from (select "b".* from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" order by "b"."taskid" asc limit $2) as "b") as "tasks" + from "paginated" as "a" order by "a"."productname" asc nulls first, "a"."productid" asc` ) ), }) @@ -128,8 +128,13 @@ describe("Captures of real examples", () => { queryJson ) expect(query).toEqual({ - bindings: [relationshipLimit, ...filters, relationshipLimit], - sql: `select "a"."executorid", "a"."taskname", "a"."taskid", "a"."completed", "a"."qaid", (select json_agg(json_build_object('productid',"b"."productid",'productname',"b"."productname")) from (select "b".* from "products" as "b" inner join "products_tasks" as "c" on "b"."productid" = "c"."productid" where "c"."taskid" = "a"."taskid" order by "b"."productid" asc limit $1) as "b") as "products" from "tasks" as "a" where "a"."taskid" in ($2, $3) order by "a"."taskid" asc limit $4`, + bindings: [...filters, relationshipLimit, relationshipLimit], + sql: multiline( + `with "paginated" as (select * from "tasks" as "a" where "a"."taskid" in ($1, $2) order by "a"."taskid" asc limit $3) + select "a"."executorid", "a"."taskname", "a"."taskid", "a"."completed", "a"."qaid", (select json_agg(json_build_object('productid',"b"."productid",'productname',"b"."productname")) + from (select "b".* from "products" as "b" inner join "products_tasks" as "c" on "b"."productid" = "c"."productid" + where "c"."taskid" = "a"."taskid" order by "b"."productid" asc limit $4) as "b") as "products" from "paginated" as "a" order by "a"."taskid" asc` + ), }) }) @@ -146,9 +151,6 @@ describe("Captures of real examples", () => { expect(query).toEqual({ bindings: [ - relationshipLimit, - relationshipLimit, - relationshipLimit, rangeValue.low, rangeValue.high, rangeValue.low, @@ -156,10 +158,13 @@ describe("Captures of real examples", () => { equalValue, notEqualsValue, primaryLimit, + relationshipLimit, + relationshipLimit, + relationshipLimit, ], sql: expect.stringContaining( multiline( - `where (exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $4 and $5))) and (exists (select 1 from "persons" as "c" where "c"."personid" = "a"."qaid" and ("c"."year" between $6 and $7))) and (exists (select 1 from "products" as "b" inner join "products_tasks" as "d" on "b"."productid" = "d"."productid" where "d"."taskid" = "a"."taskid" and (COALESCE("b"."productname" = $8, FALSE))))` + `where (exists (select 1 from "persons" as "c" where "c"."personid" = "a"."executorid" and ("c"."year" between $1 and $2))) and (exists (select 1 from "persons" as "c" where "c"."personid" = "a"."qaid" and ("c"."year" between $3 and $4))) and (exists (select 1 from "products" as "b" inner join "products_tasks" as "d" on "b"."productid" = "d"."productid" where "d"."taskid" = "a"."taskid" and (COALESCE("b"."productname" = $5, FALSE))))` ) ), }) From eb7fcd0219d33eeb9fc07696e5ece0b519e189b2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 16:33:43 +0100 Subject: [PATCH 22/51] Don't select * on relationships --- packages/backend-core/src/sql/sql.ts | 4 +++- packages/server/src/integrations/tests/sqlAlias.spec.ts | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 1a7e3beba4b..394ab92fd32 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -1323,7 +1323,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, diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index 1630e9b2042..704c003994f 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -114,7 +114,7 @@ describe("Captures of real examples", () => { multiline( `with "paginated" as (select * from "products" as "a" order by "a"."productname" asc nulls first, "a"."productid" asc limit $1) select "a"."productname", "a"."productid", (select json_agg(json_build_object('executorid',"b"."executorid",'qaid',"b"."qaid",'taskid',"b"."taskid",'completed',"b"."completed",'taskname',"b"."taskname")) - from (select "b".* from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" order by "b"."taskid" asc limit $2) as "b") as "tasks" + from (select "b"."executorid", "b"."qaid", "b"."taskid", "b"."completed", "b"."taskname" from "tasks" as "b" inner join "products_tasks" as "c" on "b"."taskid" = "c"."taskid" where "c"."productid" = "a"."productid" order by "b"."taskid" asc limit $2) as "b") as "tasks" from "paginated" as "a" order by "a"."productname" asc nulls first, "a"."productid" asc` ) ), @@ -132,7 +132,7 @@ describe("Captures of real examples", () => { sql: multiline( `with "paginated" as (select * from "tasks" as "a" where "a"."taskid" in ($1, $2) order by "a"."taskid" asc limit $3) select "a"."executorid", "a"."taskname", "a"."taskid", "a"."completed", "a"."qaid", (select json_agg(json_build_object('productid',"b"."productid",'productname',"b"."productname")) - from (select "b".* from "products" as "b" inner join "products_tasks" as "c" on "b"."productid" = "c"."productid" + from (select "b"."productid", "b"."productname" from "products" as "b" inner join "products_tasks" as "c" on "b"."productid" = "c"."productid" where "c"."taskid" = "a"."taskid" order by "b"."productid" asc limit $4) as "b") as "products" from "paginated" as "a" order by "a"."taskid" asc` ), }) From df62845cf9c492aec568e6903ceb0882aec83517 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 16:55:38 +0100 Subject: [PATCH 23/51] Fix encodings --- packages/backend-core/src/sql/utils.ts | 4 ++++ packages/server/src/api/controllers/row/utils/utils.ts | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/backend-core/src/sql/utils.ts b/packages/backend-core/src/sql/utils.ts index 14127a189f0..16b352995b1 100644 --- a/packages/backend-core/src/sql/utils.ts +++ b/packages/backend-core/src/sql/utils.ts @@ -70,6 +70,10 @@ export function encodeTableId(tableId: string) { } } +export function encodeViewId(viewId: string) { + return encodeURIComponent(viewId) +} + export function breakExternalTableId(tableId: string) { const parts = tableId.split(DOUBLE_SEPARATOR) let datasourceId = parts.shift() diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index 021a6317ea6..86986b64e8d 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -66,7 +66,7 @@ export function getSourceId(ctx: Ctx): { tableId: string; viewId?: string } { if (docIds.isViewId(sourceId)) { return { tableId: utils.extractViewInfoFromID(sourceId).tableId, - viewId: sql.utils.encodeTableId(sourceId), + viewId: sql.utils.encodeViewId(sourceId), } } return { tableId: sql.utils.encodeTableId(ctx.params.sourceId) } From 740069ea78a7866ca1373f0b423526d24792d232 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 17:01:02 +0100 Subject: [PATCH 24/51] Use table for get before row --- packages/server/src/api/controllers/row/external.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/external.ts b/packages/server/src/api/controllers/row/external.ts index 6dbd4dbb815..0405203f2fc 100644 --- a/packages/server/src/api/controllers/row/external.ts +++ b/packages/server/src/api/controllers/row/external.ts @@ -69,7 +69,7 @@ export async function patch(ctx: UserCtx) { throw { validation: validateResult.errors } } - const beforeRow = await sdk.rows.external.getRow(sourceId, _id, { + const beforeRow = await sdk.rows.external.getRow(table._id!, _id, { relationships: true, }) From 7cd412bfdcd15e6815c99f2a5b5e6a93ee1caf1c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 17:46:37 +0100 Subject: [PATCH 25/51] Add basic sql alias test --- .../src/integrations/tests/sqlAlias.spec.ts | 9 ++ .../tests/sqlQueryJson/basicFetch.json | 137 ++++++++++++++++++ 2 files changed, 146 insertions(+) create mode 100644 packages/server/src/integrations/tests/sqlQueryJson/basicFetch.json diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index 704c003994f..4d39fa73382 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -73,6 +73,15 @@ describe("Captures of real examples", () => { }) describe("read", () => { + it("should retrieve only requested fields", () => { + const queryJson = getJson("basicFetch.json") + let query = new Sql(SqlClient.POSTGRES)._query(queryJson) + expect(query).toEqual({ + bindings: [primaryLimit], + sql: `select "a"."year", "a"."firstname", "a"."personid", "a"."age", "a"."type", "a"."lastname" from "persons" as "a" order by "a"."firstname" asc nulls first, "a"."personid" asc limit $1`, + }) + }) + it("should handle basic retrieval with relationships", () => { const queryJson = getJson("basicFetchWithRelationships.json") let query = new Sql(SqlClient.POSTGRES, relationshipLimit)._query( diff --git a/packages/server/src/integrations/tests/sqlQueryJson/basicFetch.json b/packages/server/src/integrations/tests/sqlQueryJson/basicFetch.json new file mode 100644 index 00000000000..9d9026c9221 --- /dev/null +++ b/packages/server/src/integrations/tests/sqlQueryJson/basicFetch.json @@ -0,0 +1,137 @@ +{ + "operation": "READ", + "resource": { + "fields": [ + "a.year", + "a.firstname", + "a.personid", + "a.age", + "a.type", + "a.lastname" + ] + }, + "filters": {}, + "sort": { + "firstname": { + "direction": "ascending" + } + }, + "paginate": { + "limit": 100, + "page": 1 + }, + "relationships": [], + "extra": { + "idFilter": {} + }, + "table": { + "type": "table", + "_id": "datasource_plus_8066e56456784eb2a00129d31be5c3e7__persons", + "primary": ["personid"], + "name": "persons", + "schema": { + "year": { + "type": "number", + "externalType": "integer", + "autocolumn": false, + "name": "year", + "constraints": { + "presence": false + } + }, + "firstname": { + "type": "string", + "externalType": "character varying", + "autocolumn": false, + "name": "firstname", + "constraints": { + "presence": false + } + }, + "personid": { + "type": "number", + "externalType": "integer", + "autocolumn": true, + "name": "personid", + "constraints": { + "presence": false + } + }, + "address": { + "type": "string", + "externalType": "character varying", + "autocolumn": false, + "name": "address", + "constraints": { + "presence": false + } + }, + "age": { + "type": "number", + "externalType": "integer", + "autocolumn": false, + "name": "age", + "constraints": { + "presence": false + } + }, + "type": { + "type": "options", + "externalType": "USER-DEFINED", + "autocolumn": false, + "name": "type", + "constraints": { + "presence": false, + "inclusion": ["support", "designer", "programmer", "qa"] + } + }, + "city": { + "type": "string", + "externalType": "character varying", + "autocolumn": false, + "name": "city", + "constraints": { + "presence": false + } + }, + "lastname": { + "type": "string", + "externalType": "character varying", + "autocolumn": false, + "name": "lastname", + "constraints": { + "presence": false + } + }, + "QA": { + "tableId": "datasource_plus_8066e56456784eb2a00129d31be5c3e7__tasks", + "name": "QA", + "relationshipType": "many-to-one", + "fieldName": "qaid", + "type": "link", + "main": true, + "_id": "ccb68481c80c34217a4540a2c6c27fe46", + "foreignKey": "personid" + }, + "executor": { + "tableId": "datasource_plus_8066e56456784eb2a00129d31be5c3e7__tasks", + "name": "executor", + "relationshipType": "many-to-one", + "fieldName": "executorid", + "type": "link", + "main": true, + "_id": "c89530b9770d94bec851e062b5cff3001", + "foreignKey": "personid", + "tableName": "persons" + } + }, + "sourceId": "datasource_plus_8066e56456784eb2a00129d31be5c3e7", + "sourceType": "external", + "primaryDisplay": "firstname", + "views": {} + }, + "tableAliases": { + "persons": "a", + "tasks": "b" + } +} From fc75728a1e70f86f214ba2071fc0eaaf6a32a008 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 16 Dec 2024 17:52:30 +0100 Subject: [PATCH 26/51] Add more tests --- .../src/integrations/tests/sqlAlias.spec.ts | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index 4d39fa73382..647c18d4ae3 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -8,6 +8,7 @@ import { TableSchema, Table, TableSourceType, + FieldType, } from "@budibase/types" import { sql } from "@budibase/backend-core" import { join } from "path" @@ -73,8 +74,20 @@ describe("Captures of real examples", () => { }) describe("read", () => { + it("should retrieve all fields if non are specified", () => { + const queryJson = getJson("basicFetch.json") + delete queryJson.resource + + let query = new Sql(SqlClient.POSTGRES)._query(queryJson) + expect(query).toEqual({ + bindings: [primaryLimit], + sql: `select * from "persons" as "a" order by "a"."firstname" asc nulls first, "a"."personid" asc limit $1`, + }) + }) + it("should retrieve only requested fields", () => { const queryJson = getJson("basicFetch.json") + let query = new Sql(SqlClient.POSTGRES)._query(queryJson) expect(query).toEqual({ bindings: [primaryLimit], @@ -82,6 +95,22 @@ describe("Captures of real examples", () => { }) }) + it("should retrieve all fields if a formula column is requested", () => { + const queryJson = getJson("basicFetch.json") + queryJson.table.schema["formula"] = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } + queryJson.resource!.fields.push("formula") + + let query = new Sql(SqlClient.POSTGRES)._query(queryJson) + expect(query).toEqual({ + bindings: [primaryLimit], + sql: `select "a".* from "persons" as "a" order by "a"."firstname" asc nulls first, "a"."personid" asc limit $1`, + }) + }) + it("should handle basic retrieval with relationships", () => { const queryJson = getJson("basicFetchWithRelationships.json") let query = new Sql(SqlClient.POSTGRES, relationshipLimit)._query( From 7932ee76204b96bc19b1295403449ed87762dfc1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 17 Dec 2024 10:42:11 +0100 Subject: [PATCH 27/51] Fix sqs calculations --- .../src/sdk/app/rows/search/internal/sqs.ts | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index 6eb9bd913b1..e1ccb4fb85c 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -62,7 +62,8 @@ async function buildInternalFieldList( ) { const { relationships, allowedFields } = opts || {} let schemaFields: string[] = [] - if (sdk.views.isView(source)) { + const isView = sdk.views.isView(source) + if (isView) { schemaFields = Object.keys(helpers.views.basicFields(source)) } else { schemaFields = Object.keys(source.schema).filter( @@ -75,7 +76,7 @@ async function buildInternalFieldList( } let table: Table - if (sdk.views.isView(source)) { + if (isView) { table = await sdk.views.getTable(source.id) } else { table = source @@ -125,6 +126,13 @@ async function buildInternalFieldList( fieldList = fieldList.concat(relatedFields) } } + + if (isView && !helpers.views.isCalculationView(source)) { + for (const field of ["_id", "_rev", "tableId"]) { + fieldList.push(field) + } + } + return [...new Set(fieldList)] } @@ -234,17 +242,6 @@ async function runSqlQuery( json.operation = Operation.COUNT } const processSQLQuery = async (json: EnrichedQueryJson) => { - const fields = json.resource?.fields - if (fields) { - const tableId = json.tableAliases?.[json.table._id!] ?? json.table._id! - for (const key of ["_id", "_rev", "tableId"]) { - const field = `${tableId}.${key}` - if (fields.includes(field)) { - continue - } - fields.push(field) - } - } const query = builder._query(json, { disableReturning: true, }) @@ -334,8 +331,9 @@ export async function search( } let aggregations: Aggregation[] = [] - if (sdk.views.isView(source)) { + if (sdk.views.isView(source) && helpers.views.isCalculationView(source)) { const calculationFields = helpers.views.calculationFields(source) + for (const [key, field] of Object.entries(calculationFields)) { if (options.fields && !options.fields.includes(key)) { continue From 499d3f204128393719fcc166b7c984e2fe17b4a2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 17 Dec 2024 10:47:43 +0100 Subject: [PATCH 28/51] Fix sql calculations --- .../src/api/controllers/row/utils/sqlUtils.ts | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 9f5e14d732a..ce611b502ba 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -169,20 +169,22 @@ export async function buildSqlFieldList( fields = Object.keys(helpers.views.basicFields(source)) .filter(f => table.schema[f].type !== FieldType.LINK) .map(c => `${table.name}.${c}`) + + if (!helpers.views.isCalculationView(source)) { + fields.push( + ...getRequiredFields( + { + ...table, + primaryDisplay: source.primaryDisplay || table.primaryDisplay, + }, + fields + ) + ) + } } else { fields = extractRealFields(source) } - fields.push( - ...getRequiredFields( - { - ...table, - primaryDisplay: source.primaryDisplay || table.primaryDisplay, - }, - fields - ) - ) - for (const field of Object.values(table.schema)) { if (field.type !== FieldType.LINK || !relationships || !field.tableId) { continue From 51ba1f072bd940b5c70f2edad483762dd7c7e12c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 17 Dec 2024 11:02:54 +0100 Subject: [PATCH 29/51] Add required fields on table search as well --- packages/server/src/sdk/app/rows/search/internal/sqs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index e1ccb4fb85c..a5967acfb05 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -127,7 +127,7 @@ async function buildInternalFieldList( } } - if (isView && !helpers.views.isCalculationView(source)) { + if (!isView || !helpers.views.isCalculationView(source)) { for (const field of ["_id", "_rev", "tableId"]) { fieldList.push(field) } From c398412e00b52eb1aa8bd9298010dc072ea8f0b7 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 17 Dec 2024 11:16:27 +0100 Subject: [PATCH 30/51] Fix pg money --- packages/backend-core/src/sql/sql.ts | 4 +--- packages/server/src/integration-test/postgres.spec.ts | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 394ab92fd32..015b7c47519 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -274,9 +274,7 @@ class InternalBuilder { private isFullSelectStatementRequired(includedFields: string[]): boolean { for (const 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)) { + if (this.SPECIAL_SELECT_CASES.MSSQL_DATES(column)) { return true } else if ( column.type === FieldType.FORMULA && diff --git a/packages/server/src/integration-test/postgres.spec.ts b/packages/server/src/integration-test/postgres.spec.ts index 6e674aa58ef..713605eadb1 100644 --- a/packages/server/src/integration-test/postgres.spec.ts +++ b/packages/server/src/integration-test/postgres.spec.ts @@ -13,7 +13,7 @@ const mainDescriptions = datasourceDescribe({ if (mainDescriptions.length) { describe.each(mainDescriptions)( - "/postgres integrations", + "/postgres integrations ($dbName)", ({ config, dsProvider }) => { let datasource: Datasource let client: Knex From 95f7eeacce750c1b050f59e94bbdf140f58b3ae0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 17 Dec 2024 11:30:26 +0100 Subject: [PATCH 31/51] Don't add breaking changes --- packages/server/src/sdk/app/rows/search/internal/sqs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index a5967acfb05..4eca0e4c097 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -128,7 +128,7 @@ async function buildInternalFieldList( } if (!isView || !helpers.views.isCalculationView(source)) { - for (const field of ["_id", "_rev", "tableId"]) { + for (const field of PROTECTED_INTERNAL_COLUMNS) { fieldList.push(field) } } From 8765a28f0412aa3cb5353c23d655c34ab0116864 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 17 Dec 2024 13:02:34 +0100 Subject: [PATCH 32/51] Move responsability to sql utils --- packages/backend-core/src/sql/sql.ts | 21 ------- .../src/api/controllers/row/utils/sqlUtils.ts | 59 +++++++++---------- .../src/integrations/tests/sqlAlias.spec.ts | 17 ------ 3 files changed, 29 insertions(+), 68 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 015b7c47519..cc5f226d567 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -272,20 +272,6 @@ class InternalBuilder { return parts.join(".") } - private isFullSelectStatementRequired(includedFields: string[]): boolean { - for (const column of Object.values(this.table.schema)) { - if (this.SPECIAL_SELECT_CASES.MSSQL_DATES(column)) { - return true - } else if ( - column.type === FieldType.FORMULA && - includedFields.includes(column.name) - ) { - return true - } - } - return false - } - private generateSelectStatement(): (string | Knex.Raw)[] | "*" { const { table, resource } = this.query @@ -313,13 +299,6 @@ class InternalBuilder { }) .filter(({ table }) => !table || table === alias) - const requestedTableColumns = tableFields.map(({ column }) => - column.replace(new RegExp(`^${this.query.meta?.columnPrefix}`), "") - ) - if (this.isFullSelectStatementRequired(requestedTableColumns)) { - return [this.knex.raw("??", [`${alias}.*`])] - } - return tableFields.map(({ table, column, field }) => { const columnSchema = schema[column] diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index ce611b502ba..750b53861ff 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -14,7 +14,7 @@ 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 @@ -126,11 +126,9 @@ export async function buildSqlFieldList( column.type !== FieldType.LINK && column.type !== FieldType.FORMULA && column.type !== FieldType.AI && - !existing.find( - (field: string) => field === `${table.name}.${columnName}` - ) + !existing.find((field: string) => field === columnName) ) - .map(([columnName]) => `${table.name}.${columnName}`) + .map(([columnName]) => columnName) } function getRequiredFields(table: Table, existing: string[] = []) { @@ -143,15 +141,12 @@ export async function buildSqlFieldList( } if (!sql.utils.isExternalTable(table)) { - requiredFields.push(...["_id", "_rev", "_tableId"]) + requiredFields.push(...PROTECTED_INTERNAL_COLUMNS) } - return requiredFields - .filter( - column => - !existing.find((field: string) => field === `${table.name}.${column}`) - ) - .map(column => `${table.name}.${column}`) + return requiredFields.filter( + column => !existing.find((field: string) => field === column) + ) } let fields: string[] = [] @@ -161,30 +156,34 @@ export async function buildSqlFieldList( let table: Table 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) } - if (isView) { - fields = Object.keys(helpers.views.basicFields(source)) - .filter(f => table.schema[f].type !== FieldType.LINK) - .map(c => `${table.name}.${c}`) - - if (!helpers.views.isCalculationView(source)) { - fields.push( - ...getRequiredFields( - { - ...table, - primaryDisplay: source.primaryDisplay || table.primaryDisplay, - }, - fields - ) + // If are requesting for a formula field, we need to retrieve all fields + if (fields.find(f => table.schema[f]?.type === FieldType.FORMULA)) { + fields = extractRealFields(table) + } + + if (!isView || !helpers.views.isCalculationView(source)) { + fields.push( + ...getRequiredFields( + { + ...table, + primaryDisplay: source.primaryDisplay || table.primaryDisplay, + }, + fields ) - } - } else { - fields = extractRealFields(source) + ) } + fields = fields.map(c => `${table.name}.${c}`) + for (const field of Object.values(table.schema)) { if (field.type !== FieldType.LINK || !relationships || !field.tableId) { continue @@ -227,7 +226,7 @@ export async function buildSqlFieldList( } } - return fields + return [...new Set(fields)] } export function isKnexEmptyReadResponse(resp: DatasourcePlusQueryResponse) { diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index 647c18d4ae3..898ab9314ad 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -8,7 +8,6 @@ import { TableSchema, Table, TableSourceType, - FieldType, } from "@budibase/types" import { sql } from "@budibase/backend-core" import { join } from "path" @@ -95,22 +94,6 @@ describe("Captures of real examples", () => { }) }) - it("should retrieve all fields if a formula column is requested", () => { - const queryJson = getJson("basicFetch.json") - queryJson.table.schema["formula"] = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } - queryJson.resource!.fields.push("formula") - - let query = new Sql(SqlClient.POSTGRES)._query(queryJson) - expect(query).toEqual({ - bindings: [primaryLimit], - sql: `select "a".* from "persons" as "a" order by "a"."firstname" asc nulls first, "a"."personid" asc limit $1`, - }) - }) - it("should handle basic retrieval with relationships", () => { const queryJson = getJson("basicFetchWithRelationships.json") let query = new Sql(SqlClient.POSTGRES, relationshipLimit)._query( From 8eb82d3b0597cf6f8151f8e5ef871e8e4df2d3d7 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 17 Dec 2024 16:10:15 +0100 Subject: [PATCH 33/51] Fix mysql formula test --- .../src/api/controllers/row/utils/sqlUtils.ts | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 750b53861ff..0cfc429afe9 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -165,8 +165,11 @@ export async function buildSqlFieldList( fields = extractRealFields(source) } + 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 (fields.find(f => table.schema[f]?.type === FieldType.FORMULA)) { + if (containsFormula) { fields = extractRealFields(table) } @@ -192,15 +195,22 @@ export async function buildSqlFieldList( if ( isView && source.schema?.[field.name] && - !helpers.views.isVisible(source.schema[field.name]) + !helpers.views.isVisible(source.schema[field.name]) && + !containsFormula ) { continue } const { tableName } = breakExternalTableId(field.tableId) const relatedTable = tables[tableName] - if (relatedTable) { - const viewFields = new Set() + if (!relatedTable) { + continue + } + + const viewFields = new Set() + if (containsFormula) { + extractRealFields(relatedTable).forEach(f => viewFields.add(f)) + } else { relatedTable.primary?.forEach(f => viewFields.add(f)) if (relatedTable.primaryDisplay) { viewFields.add(relatedTable.primaryDisplay) @@ -218,12 +228,12 @@ export async function buildSqlFieldList( ) .forEach(([field]) => viewFields.add(field)) } - - const fieldsToAdd = Array.from(viewFields) - .map(f => `${relatedTable.name}.${f}`) - .filter(f => !fields.includes(f)) - fields.push(...fieldsToAdd) } + + const fieldsToAdd = Array.from(viewFields) + .map(f => `${relatedTable.name}.${f}`) + .filter(f => !fields.includes(f)) + fields.push(...fieldsToAdd) } return [...new Set(fields)] From 23531e15096fec99f100a71c406658ee47230978 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 17 Dec 2024 16:10:21 +0100 Subject: [PATCH 34/51] Fix sqs formula --- .../src/sdk/app/rows/search/internal/sqs.ts | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index 4eca0e4c097..c92d186a373 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -62,7 +62,15 @@ async function buildInternalFieldList( ) { const { relationships, allowedFields } = opts || {} let schemaFields: string[] = [] + const isView = sdk.views.isView(source) + let table: Table + if (isView) { + table = await sdk.views.getTable(source.id) + } else { + table = source + } + if (isView) { schemaFields = Object.keys(helpers.views.basicFields(source)) } else { @@ -71,17 +79,16 @@ async function buildInternalFieldList( ) } - if (allowedFields) { + const containsFormula = schemaFields.some( + f => table.schema[f]?.type === FieldType.FORMULA + ) + // If are requesting for a formula field, we need to retrieve all fields + if (containsFormula) { + schemaFields = Object.keys(table.schema) + } else if (allowedFields) { schemaFields = schemaFields.filter(field => allowedFields.includes(field)) } - let table: Table - if (isView) { - table = await sdk.views.getTable(source.id) - } else { - table = source - } - let fieldList: string[] = [] const getJunctionFields = (relatedTable: Table, fields: string[]) => { const junctionFields: string[] = [] From bcc9bba254ff4e75ee275041f5488875de2575dd Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 17 Dec 2024 16:58:15 +0100 Subject: [PATCH 35/51] Fix test --- packages/server/src/api/routes/tests/search.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index e97f48afbe1..e94e567b43e 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -3399,7 +3399,7 @@ if (descriptions.length) { type: FieldType.LINK, relationshipType: RelationshipType.MANY_TO_ONE, tableId: toRelateTableId, - fieldName: "link", + fieldName: "main", }, }) @@ -3408,7 +3408,7 @@ if (descriptions.length) { ) await config.api.table.save({ ...toRelateTable, - primaryDisplay: "link", + primaryDisplay: "name", }) const relatedRows = await Promise.all([ config.api.row.save(toRelateTable._id!, { From b05b523a6e96169e23dc1cfc7a78b038493c2fb7 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 17 Dec 2024 17:01:06 +0100 Subject: [PATCH 36/51] Fix issues with display names not being sql tables --- .../src/api/controllers/row/utils/sqlUtils.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 0cfc429afe9..f28075bf844 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -119,13 +119,14 @@ 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 && + !nonMappedColumns.includes(column.type) && !existing.find((field: string) => field === columnName) ) .map(([columnName]) => columnName) @@ -145,7 +146,10 @@ export async function buildSqlFieldList( } return requiredFields.filter( - column => !existing.find((field: string) => field === column) + column => + !existing.find((field: string) => field === column) && + table.schema[column] && + !nonMappedColumns.includes(table.schema[column].type) ) } @@ -231,6 +235,7 @@ export async function buildSqlFieldList( } 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) From 27d1929388de83c56798dd2433685ec3376e67e2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 18 Dec 2024 14:09:26 +0100 Subject: [PATCH 37/51] Add basic sqlUtils test --- .../src/api/controllers/row/utils/sqlUtils.ts | 4 +- .../row/utils/tests/sqlUtils.spec.ts | 79 +++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index f28075bf844..e66a0b5bf69 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -166,7 +166,9 @@ export async function buildSqlFieldList( ) } else { table = source - fields = extractRealFields(source) + fields = extractRealFields(source).filter( + f => table.schema[f].visible !== false + ) } const containsFormula = (isView ? fields : Object.keys(table.schema)).some( diff --git a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts new file mode 100644 index 00000000000..2ab8e251eae --- /dev/null +++ b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts @@ -0,0 +1,79 @@ +import { + AIOperationEnum, + FieldType, + RelationshipType, + Table, +} from "@budibase/types" +import { buildSqlFieldList } from "../sqlUtils" +import { structures } from "../../../../routes/tests/utilities" +import { cloneDeep } from "lodash" + +describe("buildSqlFieldList", () => { + const basicTable: Table = { + ...structures.basicTable(), + name: "table", + schema: { + name: { + type: FieldType.STRING, + name: "name", + }, + description: { + type: FieldType.STRING, + name: "description", + }, + amount: { + type: FieldType.NUMBER, + name: "amount", + }, + }, + } + + it("extracts fields from table schema", async () => { + const result = await buildSqlFieldList(basicTable, {}) + expect(result).toEqual(["table.name", "table.description", "table.amount"]) + }) + + it("excludes hidden fields", async () => { + const table = cloneDeep(basicTable) + table.schema.description.visible = false + const result = await buildSqlFieldList(table, {}) + expect(result).toEqual(["table.name", "table.amount"]) + }) + + it("excludes non-sql fields fields", async () => { + const table = cloneDeep(basicTable) + table.schema.formula = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } + table.schema.ai = { + name: "ai", + type: FieldType.AI, + operation: AIOperationEnum.PROMPT, + } + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: "otherTableId", + } + + const result = await buildSqlFieldList(table, {}) + expect(result).toEqual(["table.name", "table.description", "table.amount"]) + }) + + it("includes hidden fields if there is a formula column", async () => { + const table = cloneDeep(basicTable) + table.schema.description.visible = false + table.schema.formula = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } + + const result = await buildSqlFieldList(table, {}) + expect(result).toEqual(["table.name", "table.description", "table.amount"]) + }) +}) From 875319e85c1fa97abc4e052845ea61ef1dfde449 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 18 Dec 2024 15:50:51 +0100 Subject: [PATCH 38/51] Relationship tests --- .../row/utils/tests/sqlUtils.spec.ts | 105 +++++++++++++++++- 1 file changed, 101 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts index 2ab8e251eae..5a1b8ae49e7 100644 --- a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts +++ b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts @@ -2,28 +2,34 @@ import { AIOperationEnum, FieldType, RelationshipType, + SourceName, Table, } from "@budibase/types" import { buildSqlFieldList } from "../sqlUtils" import { structures } from "../../../../routes/tests/utilities" import { cloneDeep } from "lodash" +import { sql } from "@budibase/backend-core" describe("buildSqlFieldList", () => { const basicTable: Table = { - ...structures.basicTable(), + ...structures.tableForDatasource({ + type: "datasource", + source: SourceName.POSTGRES, + }), name: "table", + _id: sql.utils.buildExternalTableId("ds_id", "table"), schema: { name: { - type: FieldType.STRING, name: "name", + type: FieldType.STRING, }, description: { - type: FieldType.STRING, name: "description", + type: FieldType.STRING, }, amount: { - type: FieldType.NUMBER, name: "amount", + type: FieldType.NUMBER, }, }, } @@ -76,4 +82,95 @@ describe("buildSqlFieldList", () => { const result = await buildSqlFieldList(table, {}) expect(result).toEqual(["table.name", "table.description", "table.amount"]) }) + + it("includes relationships fields when flag", async () => { + const table = cloneDeep(basicTable) + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), + } + + const otherTable: Table = { + ...cloneDeep(basicTable), + name: "linkedTable", + primary: ["id"], + primaryDisplay: "name", + schema: { + ...cloneDeep(basicTable).schema, + id: { + name: "id", + type: FieldType.NUMBER, + }, + }, + } + + const allTables: Record = { + otherTableId: otherTable, + } + + const result = await buildSqlFieldList(table, allTables, { + relationships: true, + }) + expect(result).toEqual([ + "table.name", + "table.description", + "table.amount", + "linkedTable.id", + "linkedTable.name", + ]) + }) + + it("includes all relationship fields if there is a formula column", async () => { + const table = cloneDeep(basicTable) + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), + } + table.schema.formula = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } + + const otherTable: Table = { + ...cloneDeep(basicTable), + name: "linkedTable", + schema: { + ...cloneDeep(basicTable).schema, + id: { + name: "id", + type: FieldType.NUMBER, + }, + hidden: { + name: "other", + type: FieldType.STRING, + visible: false, + }, + }, + } + + const allTables: Record = { + otherTableId: otherTable, + } + + const result = await buildSqlFieldList(table, allTables, { + relationships: true, + }) + expect(result).toEqual([ + "table.name", + "table.description", + "table.amount", + "linkedTable.name", + "linkedTable.description", + "linkedTable.amount", + "linkedTable.id", + "linkedTable.hidden", + ]) + }) }) From 96bddbb545b8ddc1602e20e981d2999f9b5d46a0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 18 Dec 2024 15:53:09 +0100 Subject: [PATCH 39/51] More relationship tests --- .../row/utils/tests/sqlUtils.spec.ts | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts index 5a1b8ae49e7..767a325c7ae 100644 --- a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts +++ b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts @@ -152,6 +152,23 @@ describe("buildSqlFieldList", () => { type: FieldType.STRING, visible: false, }, + formula: { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + }, + ai: { + name: "ai", + type: FieldType.AI, + operation: AIOperationEnum.PROMPT, + }, + link: { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: "otherTableId", + }, }, } @@ -173,4 +190,68 @@ describe("buildSqlFieldList", () => { "linkedTable.hidden", ]) }) + + it("never includes non-sql columns from relationships", async () => { + const table = cloneDeep(basicTable) + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), + } + table.schema.formula = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } + + const otherTable: Table = { + ...cloneDeep(basicTable), + name: "linkedTable", + schema: { + id: { + name: "id", + type: FieldType.NUMBER, + }, + hidden: { + name: "other", + type: FieldType.STRING, + visible: false, + }, + formula: { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + }, + ai: { + name: "ai", + type: FieldType.AI, + operation: AIOperationEnum.PROMPT, + }, + link: { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: "otherTableId", + }, + }, + } + + const allTables: Record = { + otherTableId: otherTable, + } + + const result = await buildSqlFieldList(table, allTables, { + relationships: true, + }) + expect(result).toEqual([ + "table.name", + "table.description", + "table.amount", + "linkedTable.id", + "linkedTable.hidden", + ]) + }) }) From e23753ac6134c33482ddcd1802907ec3d9ab9f11 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 18 Dec 2024 15:54:07 +0100 Subject: [PATCH 40/51] Add describe --- .../row/utils/tests/sqlUtils.spec.ts | 406 +++++++++--------- 1 file changed, 210 insertions(+), 196 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts index 767a325c7ae..41cc9f6a03a 100644 --- a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts +++ b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts @@ -34,224 +34,238 @@ describe("buildSqlFieldList", () => { }, } - it("extracts fields from table schema", async () => { - const result = await buildSqlFieldList(basicTable, {}) - expect(result).toEqual(["table.name", "table.description", "table.amount"]) - }) + describe("table", () => { + it("extracts fields from table schema", async () => { + const result = await buildSqlFieldList(basicTable, {}) + expect(result).toEqual([ + "table.name", + "table.description", + "table.amount", + ]) + }) - it("excludes hidden fields", async () => { - const table = cloneDeep(basicTable) - table.schema.description.visible = false - const result = await buildSqlFieldList(table, {}) - expect(result).toEqual(["table.name", "table.amount"]) - }) + it("excludes hidden fields", async () => { + const table = cloneDeep(basicTable) + table.schema.description.visible = false + const result = await buildSqlFieldList(table, {}) + expect(result).toEqual(["table.name", "table.amount"]) + }) - it("excludes non-sql fields fields", async () => { - const table = cloneDeep(basicTable) - table.schema.formula = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } - table.schema.ai = { - name: "ai", - type: FieldType.AI, - operation: AIOperationEnum.PROMPT, - } - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: "otherTableId", - } + it("excludes non-sql fields fields", async () => { + const table = cloneDeep(basicTable) + table.schema.formula = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } + table.schema.ai = { + name: "ai", + type: FieldType.AI, + operation: AIOperationEnum.PROMPT, + } + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: "otherTableId", + } - const result = await buildSqlFieldList(table, {}) - expect(result).toEqual(["table.name", "table.description", "table.amount"]) - }) + const result = await buildSqlFieldList(table, {}) + expect(result).toEqual([ + "table.name", + "table.description", + "table.amount", + ]) + }) - it("includes hidden fields if there is a formula column", async () => { - const table = cloneDeep(basicTable) - table.schema.description.visible = false - table.schema.formula = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } + it("includes hidden fields if there is a formula column", async () => { + const table = cloneDeep(basicTable) + table.schema.description.visible = false + table.schema.formula = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } - const result = await buildSqlFieldList(table, {}) - expect(result).toEqual(["table.name", "table.description", "table.amount"]) - }) + const result = await buildSqlFieldList(table, {}) + expect(result).toEqual([ + "table.name", + "table.description", + "table.amount", + ]) + }) - it("includes relationships fields when flag", async () => { - const table = cloneDeep(basicTable) - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), - } + it("includes relationships fields when flag", async () => { + const table = cloneDeep(basicTable) + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), + } - const otherTable: Table = { - ...cloneDeep(basicTable), - name: "linkedTable", - primary: ["id"], - primaryDisplay: "name", - schema: { - ...cloneDeep(basicTable).schema, - id: { - name: "id", - type: FieldType.NUMBER, + const otherTable: Table = { + ...cloneDeep(basicTable), + name: "linkedTable", + primary: ["id"], + primaryDisplay: "name", + schema: { + ...cloneDeep(basicTable).schema, + id: { + name: "id", + type: FieldType.NUMBER, + }, }, - }, - } + } - const allTables: Record = { - otherTableId: otherTable, - } + const allTables: Record = { + otherTableId: otherTable, + } - const result = await buildSqlFieldList(table, allTables, { - relationships: true, + const result = await buildSqlFieldList(table, allTables, { + relationships: true, + }) + expect(result).toEqual([ + "table.name", + "table.description", + "table.amount", + "linkedTable.id", + "linkedTable.name", + ]) }) - expect(result).toEqual([ - "table.name", - "table.description", - "table.amount", - "linkedTable.id", - "linkedTable.name", - ]) - }) - it("includes all relationship fields if there is a formula column", async () => { - const table = cloneDeep(basicTable) - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), - } - table.schema.formula = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } + it("includes all relationship fields if there is a formula column", async () => { + const table = cloneDeep(basicTable) + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), + } + table.schema.formula = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } - const otherTable: Table = { - ...cloneDeep(basicTable), - name: "linkedTable", - schema: { - ...cloneDeep(basicTable).schema, - id: { - name: "id", - type: FieldType.NUMBER, - }, - hidden: { - name: "other", - type: FieldType.STRING, - visible: false, + const otherTable: Table = { + ...cloneDeep(basicTable), + name: "linkedTable", + schema: { + ...cloneDeep(basicTable).schema, + id: { + name: "id", + type: FieldType.NUMBER, + }, + hidden: { + name: "other", + type: FieldType.STRING, + visible: false, + }, + formula: { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + }, + ai: { + name: "ai", + type: FieldType.AI, + operation: AIOperationEnum.PROMPT, + }, + link: { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: "otherTableId", + }, }, - formula: { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - }, - ai: { - name: "ai", - type: FieldType.AI, - operation: AIOperationEnum.PROMPT, - }, - link: { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: "otherTableId", - }, - }, - } + } - const allTables: Record = { - otherTableId: otherTable, - } + const allTables: Record = { + otherTableId: otherTable, + } - const result = await buildSqlFieldList(table, allTables, { - relationships: true, + const result = await buildSqlFieldList(table, allTables, { + relationships: true, + }) + expect(result).toEqual([ + "table.name", + "table.description", + "table.amount", + "linkedTable.name", + "linkedTable.description", + "linkedTable.amount", + "linkedTable.id", + "linkedTable.hidden", + ]) }) - expect(result).toEqual([ - "table.name", - "table.description", - "table.amount", - "linkedTable.name", - "linkedTable.description", - "linkedTable.amount", - "linkedTable.id", - "linkedTable.hidden", - ]) - }) - it("never includes non-sql columns from relationships", async () => { - const table = cloneDeep(basicTable) - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), - } - table.schema.formula = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } + it("never includes non-sql columns from relationships", async () => { + const table = cloneDeep(basicTable) + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), + } + table.schema.formula = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } - const otherTable: Table = { - ...cloneDeep(basicTable), - name: "linkedTable", - schema: { - id: { - name: "id", - type: FieldType.NUMBER, + const otherTable: Table = { + ...cloneDeep(basicTable), + name: "linkedTable", + schema: { + id: { + name: "id", + type: FieldType.NUMBER, + }, + hidden: { + name: "other", + type: FieldType.STRING, + visible: false, + }, + formula: { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + }, + ai: { + name: "ai", + type: FieldType.AI, + operation: AIOperationEnum.PROMPT, + }, + link: { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: "otherTableId", + }, }, - hidden: { - name: "other", - type: FieldType.STRING, - visible: false, - }, - formula: { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - }, - ai: { - name: "ai", - type: FieldType.AI, - operation: AIOperationEnum.PROMPT, - }, - link: { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: "otherTableId", - }, - }, - } + } - const allTables: Record = { - otherTableId: otherTable, - } + const allTables: Record = { + otherTableId: otherTable, + } - const result = await buildSqlFieldList(table, allTables, { - relationships: true, + const result = await buildSqlFieldList(table, allTables, { + relationships: true, + }) + expect(result).toEqual([ + "table.name", + "table.description", + "table.amount", + "linkedTable.id", + "linkedTable.hidden", + ]) }) - expect(result).toEqual([ - "table.name", - "table.description", - "table.amount", - "linkedTable.id", - "linkedTable.hidden", - ]) }) }) From 14da90296b4d44fa56146d0fef304a1bdee79eba Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 18 Dec 2024 16:23:53 +0100 Subject: [PATCH 41/51] Regenerate table before each test --- .../row/utils/tests/sqlUtils.spec.ts | 93 +++++++++---------- 1 file changed, 45 insertions(+), 48 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts index 41cc9f6a03a..b1be459f75b 100644 --- a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts +++ b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts @@ -7,11 +7,13 @@ import { } from "@budibase/types" import { buildSqlFieldList } from "../sqlUtils" import { structures } from "../../../../routes/tests/utilities" -import { cloneDeep } from "lodash" import { sql } from "@budibase/backend-core" describe("buildSqlFieldList", () => { - const basicTable: Table = { + let table: Table & { _id: string } + + beforeEach(() => { + table = { ...structures.tableForDatasource({ type: "datasource", source: SourceName.POSTGRES, @@ -33,10 +35,11 @@ describe("buildSqlFieldList", () => { }, }, } + }) describe("table", () => { it("extracts fields from table schema", async () => { - const result = await buildSqlFieldList(basicTable, {}) + const result = await buildSqlFieldList(table, {}) expect(result).toEqual([ "table.name", "table.description", @@ -45,14 +48,12 @@ describe("buildSqlFieldList", () => { }) it("excludes hidden fields", async () => { - const table = cloneDeep(basicTable) table.schema.description.visible = false const result = await buildSqlFieldList(table, {}) expect(result).toEqual(["table.name", "table.amount"]) }) it("excludes non-sql fields fields", async () => { - const table = cloneDeep(basicTable) table.schema.formula = { name: "formula", type: FieldType.FORMULA, @@ -80,7 +81,6 @@ describe("buildSqlFieldList", () => { }) it("includes hidden fields if there is a formula column", async () => { - const table = cloneDeep(basicTable) table.schema.description.visible = false table.schema.formula = { name: "formula", @@ -97,22 +97,13 @@ describe("buildSqlFieldList", () => { }) it("includes relationships fields when flag", async () => { - const table = cloneDeep(basicTable) - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), - } - const otherTable: Table = { - ...cloneDeep(basicTable), + ...table, name: "linkedTable", primary: ["id"], primaryDisplay: "name", schema: { - ...cloneDeep(basicTable).schema, + ...table.schema, id: { name: "id", type: FieldType.NUMBER, @@ -120,6 +111,14 @@ describe("buildSqlFieldList", () => { }, } + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), + } + const allTables: Record = { otherTableId: otherTable, } @@ -137,25 +136,11 @@ describe("buildSqlFieldList", () => { }) it("includes all relationship fields if there is a formula column", async () => { - const table = cloneDeep(basicTable) - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), - } - table.schema.formula = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } - const otherTable: Table = { - ...cloneDeep(basicTable), + ...table, name: "linkedTable", schema: { - ...cloneDeep(basicTable).schema, + ...table.schema, id: { name: "id", type: FieldType.NUMBER, @@ -185,6 +170,19 @@ describe("buildSqlFieldList", () => { }, } + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), + } + table.schema.formula = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } + const allTables: Record = { otherTableId: otherTable, } @@ -205,22 +203,8 @@ describe("buildSqlFieldList", () => { }) it("never includes non-sql columns from relationships", async () => { - const table = cloneDeep(basicTable) - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), - } - table.schema.formula = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } - const otherTable: Table = { - ...cloneDeep(basicTable), + ...table, name: "linkedTable", schema: { id: { @@ -252,6 +236,19 @@ describe("buildSqlFieldList", () => { }, } + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), + } + table.schema.formula = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } + const allTables: Record = { otherTableId: otherTable, } From 8da96ab271c09e8c805608d095116235b4a9f840 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 18 Dec 2024 16:36:20 +0100 Subject: [PATCH 42/51] Add initial view tests --- .../row/utils/tests/sqlUtils.spec.ts | 119 ++++++++++++++---- 1 file changed, 98 insertions(+), 21 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts index b1be459f75b..6b7f0d9ce06 100644 --- a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts +++ b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts @@ -4,37 +4,51 @@ import { RelationshipType, SourceName, Table, + ViewV2, } from "@budibase/types" import { buildSqlFieldList } from "../sqlUtils" import { structures } from "../../../../routes/tests/utilities" import { sql } from "@budibase/backend-core" +import { generator } from "@budibase/backend-core/tests" +import { generateViewID } from "../../../../../db/utils" + +import sdk from "../../../../../sdk" + +jest.mock("../../../../../sdk/app/views", () => ({ + ...jest.requireActual("../../../../../sdk/app/views"), + getTable: jest.fn(), +})) +const getTableMock = sdk.views.getTable as jest.MockedFunction< + typeof sdk.views.getTable +> describe("buildSqlFieldList", () => { let table: Table & { _id: string } beforeEach(() => { + jest.clearAllMocks() table = { - ...structures.tableForDatasource({ - type: "datasource", - source: SourceName.POSTGRES, - }), - name: "table", - _id: sql.utils.buildExternalTableId("ds_id", "table"), - schema: { - name: { - name: "name", - type: FieldType.STRING, - }, - description: { - name: "description", - type: FieldType.STRING, - }, - amount: { - name: "amount", - type: FieldType.NUMBER, + ...structures.tableForDatasource({ + type: "datasource", + source: SourceName.POSTGRES, + }), + name: "table", + _id: sql.utils.buildExternalTableId("ds_id", "table"), + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + description: { + name: "description", + type: FieldType.STRING, + }, + amount: { + name: "amount", + type: FieldType.NUMBER, + }, }, - }, - } + } }) describe("table", () => { @@ -96,7 +110,7 @@ describe("buildSqlFieldList", () => { ]) }) - it("includes relationships fields when flag", async () => { + it("includes relationships fields when flagged", async () => { const otherTable: Table = { ...table, name: "linkedTable", @@ -265,4 +279,67 @@ describe("buildSqlFieldList", () => { ]) }) }) + + describe("view", () => { + let view: ViewV2 + + beforeEach(() => { + getTableMock.mockResolvedValueOnce(table) + + view = { + version: 2, + id: generateViewID(table._id), + name: generator.word(), + tableId: table._id, + } + }) + + it("extracts fields from table schema", async () => { + view.schema = { + name: { visible: false }, + amount: { visible: true }, + } + + const result = await buildSqlFieldList(view, {}) + expect(result).toEqual(["table.amount"]) + }) + + it("includes all fields if there is a formula column", async () => { + table.schema.formula = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } + + view.schema = { + name: { visible: false }, + amount: { visible: true }, + formula: { visible: true }, + } + + const result = await buildSqlFieldList(view, {}) + expect(result).toEqual([ + "table.name", + "table.description", + "table.amount", + ]) + }) + + it("does not includes all fields if the formula column is not included", async () => { + table.schema.formula = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } + + view.schema = { + name: { visible: false }, + amount: { visible: true }, + formula: { visible: false }, + } + + const result = await buildSqlFieldList(view, {}) + expect(result).toEqual(["table.amount"]) + }) + }) }) From 460b5fd7dd5c8dc839c361d857a774009feda72a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 18 Dec 2024 16:45:08 +0100 Subject: [PATCH 43/51] More tests --- .../row/utils/tests/sqlUtils.spec.ts | 224 ++++++++++++++++++ 1 file changed, 224 insertions(+) diff --git a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts index 6b7f0d9ce06..e1719d980b3 100644 --- a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts +++ b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts @@ -341,5 +341,229 @@ describe("buildSqlFieldList", () => { const result = await buildSqlFieldList(view, {}) expect(result).toEqual(["table.amount"]) }) + + it("includes relationships fields when flagged", async () => { + const otherTable: Table = { + ...table, + name: "linkedTable", + primary: ["id"], + primaryDisplay: "name", + schema: { + ...table.schema, + id: { + name: "id", + type: FieldType.NUMBER, + }, + }, + } + + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), + } + + view.schema = { + name: { visible: true }, + link: { visible: true }, + } + + const allTables: Record = { + otherTableId: otherTable, + } + + const result = await buildSqlFieldList(view, allTables, { + relationships: true, + }) + expect(result).toEqual([ + "table.name", + "linkedTable.id", + "linkedTable.name", + ]) + }) + + it("includes relationships columns", async () => { + const otherTable: Table = { + ...table, + name: "linkedTable", + primary: ["id"], + schema: { + ...table.schema, + id: { + name: "id", + type: FieldType.NUMBER, + }, + formula: { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + }, + }, + } + + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), + } + + view.schema = { + name: { visible: true }, + link: { + visible: true, + columns: { + name: { visible: false }, + amount: { visible: true }, + formula: { visible: false }, + }, + }, + } + + const allTables: Record = { + otherTableId: otherTable, + } + + const result = await buildSqlFieldList(view, allTables, { + relationships: true, + }) + expect(result).toEqual([ + "table.name", + "linkedTable.id", + "linkedTable.amount", + ]) + }) + + it("does not include relationships columns for hidden links", async () => { + const otherTable: Table = { + ...table, + name: "linkedTable", + primary: ["id"], + schema: { + ...table.schema, + id: { + name: "id", + type: FieldType.NUMBER, + }, + formula: { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + }, + }, + } + + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), + } + + view.schema = { + name: { visible: true }, + link: { + visible: false, + columns: { + name: { visible: false }, + amount: { visible: true }, + formula: { visible: false }, + }, + }, + } + + const allTables: Record = { + otherTableId: otherTable, + } + + const result = await buildSqlFieldList(view, allTables, { + relationships: true, + }) + expect(result).toEqual(["table.name"]) + }) + + it("includes all relationship fields if there is a formula column", async () => { + const otherTable: Table = { + ...table, + name: "linkedTable", + schema: { + ...table.schema, + id: { + name: "id", + type: FieldType.NUMBER, + }, + hidden: { + name: "other", + type: FieldType.STRING, + visible: false, + }, + formula: { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + }, + ai: { + name: "ai", + type: FieldType.AI, + operation: AIOperationEnum.PROMPT, + }, + link: { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: "otherTableId", + }, + }, + } + + table.schema.link = { + name: "link", + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), + } + table.schema.formula = { + name: "formula", + type: FieldType.FORMULA, + formula: "any", + } + + view.schema = { + name: { visible: true }, + formula: { visible: true }, + link: { + visible: false, + columns: { + name: { visible: false }, + amount: { visible: true }, + formula: { visible: false }, + }, + }, + } + + const allTables: Record = { + otherTableId: otherTable, + } + + const result = await buildSqlFieldList(view, allTables, { + relationships: true, + }) + expect(result).toEqual([ + "table.name", + "table.description", + "table.amount", + "linkedTable.name", + "linkedTable.description", + "linkedTable.amount", + "linkedTable.id", + "linkedTable.hidden", + ]) + }) }) }) From 9396292c4a451d83bb76002cb816f56011dfbbe0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 18 Dec 2024 18:34:40 +0100 Subject: [PATCH 44/51] Create table test builder --- .../row/utils/tests/sqlUtils.spec.ts | 673 ++++++++---------- 1 file changed, 289 insertions(+), 384 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts index e1719d980b3..eafdd170f8b 100644 --- a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts +++ b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts @@ -13,6 +13,8 @@ import { generator } from "@budibase/backend-core/tests" import { generateViewID } from "../../../../../db/utils" import sdk from "../../../../../sdk" +import { cloneDeep } from "lodash" +import { utils } from "@budibase/shared-core" jest.mock("../../../../../sdk/app/views", () => ({ ...jest.requireActual("../../../../../sdk/app/views"), @@ -23,36 +25,161 @@ const getTableMock = sdk.views.getTable as jest.MockedFunction< > describe("buildSqlFieldList", () => { - let table: Table & { _id: string } + let allTables: Record + + class TableConfig { + private _table: Table & { _id: string } + + constructor(name: string) { + this._table = { + ...structures.tableForDatasource({ + type: "datasource", + source: SourceName.POSTGRES, + }), + name, + _id: sql.utils.buildExternalTableId("ds_id", name), + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + description: { + name: "description", + type: FieldType.STRING, + }, + amount: { + name: "amount", + type: FieldType.NUMBER, + }, + }, + } + + allTables[name] = this._table + } + + withHiddenField(field: string) { + this._table.schema[field].visible = false + return this + } + + withField( + name: string, + type: + | FieldType.STRING + | FieldType.NUMBER + | FieldType.FORMULA + | FieldType.AI, + options?: { visible: boolean } + ) { + switch (type) { + case FieldType.NUMBER: + case FieldType.STRING: + this._table.schema[name] = { + name, + type, + ...options, + } + break + case FieldType.FORMULA: + this._table.schema[name] = { + name, + type, + formula: "any", + ...options, + } + break + case FieldType.AI: + this._table.schema[name] = { + name, + type, + operation: AIOperationEnum.PROMPT, + ...options, + } + break + default: + utils.unreachable(type) + } + return this + } + + withRelation(name: string, toTableId: string) { + this._table.schema[name] = { + name, + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: toTableId, + } + return this + } + + withPrimary(field: string) { + this._table.primary = [field] + return this + } + + withDisplay(field: string) { + this._table.primaryDisplay = field + return this + } + + create() { + return cloneDeep(this._table) + } + } + + class ViewConfig { + private _table: Table + private _view: ViewV2 + + constructor(table: Table) { + this._table = table + this._view = { + version: 2, + id: generateViewID(table._id!), + name: generator.word(), + tableId: table._id!, + } + } + + withVisible(field: string) { + this._view.schema ??= {} + this._view.schema[field] ??= {} + this._view.schema[field].visible = true + return this + } + + withHidden(field: string) { + this._view.schema ??= {} + this._view.schema[field] ??= {} + this._view.schema[field].visible = false + return this + } + + withRelationshipColumns( + field: string, + columns: Record + ) { + this._view.schema ??= {} + this._view.schema[field] ??= {} + this._view.schema[field].columns = columns + return this + } + + create() { + getTableMock.mockResolvedValueOnce(this._table) + return cloneDeep(this._view) + } + } beforeEach(() => { jest.clearAllMocks() - table = { - ...structures.tableForDatasource({ - type: "datasource", - source: SourceName.POSTGRES, - }), - name: "table", - _id: sql.utils.buildExternalTableId("ds_id", "table"), - schema: { - name: { - name: "name", - type: FieldType.STRING, - }, - description: { - name: "description", - type: FieldType.STRING, - }, - amount: { - name: "amount", - type: FieldType.NUMBER, - }, - }, - } + allTables = {} }) describe("table", () => { it("extracts fields from table schema", async () => { + const table = new TableConfig("table").create() const result = await buildSqlFieldList(table, {}) expect(result).toEqual([ "table.name", @@ -62,29 +189,19 @@ describe("buildSqlFieldList", () => { }) it("excludes hidden fields", async () => { - table.schema.description.visible = false + const table = new TableConfig("table") + .withHiddenField("description") + .create() const result = await buildSqlFieldList(table, {}) expect(result).toEqual(["table.name", "table.amount"]) }) it("excludes non-sql fields fields", async () => { - table.schema.formula = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } - table.schema.ai = { - name: "ai", - type: FieldType.AI, - operation: AIOperationEnum.PROMPT, - } - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: "otherTableId", - } + const table = new TableConfig("table") + .withField("formula", FieldType.FORMULA) + .withField("ai", FieldType.AI) + .withRelation("link", "otherTableId") + .create() const result = await buildSqlFieldList(table, {}) expect(result).toEqual([ @@ -95,12 +212,10 @@ describe("buildSqlFieldList", () => { }) it("includes hidden fields if there is a formula column", async () => { - table.schema.description.visible = false - table.schema.formula = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } + const table = new TableConfig("table") + .withHiddenField("description") + .withField("formula", FieldType.FORMULA) + .create() const result = await buildSqlFieldList(table, {}) expect(result).toEqual([ @@ -111,31 +226,15 @@ describe("buildSqlFieldList", () => { }) it("includes relationships fields when flagged", async () => { - const otherTable: Table = { - ...table, - name: "linkedTable", - primary: ["id"], - primaryDisplay: "name", - schema: { - ...table.schema, - id: { - name: "id", - type: FieldType.NUMBER, - }, - }, - } + const otherTable = new TableConfig("linkedTable") + .withField("id", FieldType.NUMBER) + .withPrimary("id") + .withDisplay("name") + .create() - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), - } - - const allTables: Record = { - otherTableId: otherTable, - } + const table = new TableConfig("table") + .withRelation("link", otherTable._id) + .create() const result = await buildSqlFieldList(table, allTables, { relationships: true, @@ -150,56 +249,14 @@ describe("buildSqlFieldList", () => { }) it("includes all relationship fields if there is a formula column", async () => { - const otherTable: Table = { - ...table, - name: "linkedTable", - schema: { - ...table.schema, - id: { - name: "id", - type: FieldType.NUMBER, - }, - hidden: { - name: "other", - type: FieldType.STRING, - visible: false, - }, - formula: { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - }, - ai: { - name: "ai", - type: FieldType.AI, - operation: AIOperationEnum.PROMPT, - }, - link: { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: "otherTableId", - }, - }, - } - - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), - } - table.schema.formula = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } + const otherTable = new TableConfig("linkedTable") + .withField("hidden", FieldType.STRING, { visible: false }) + .create() - const allTables: Record = { - otherTableId: otherTable, - } + const table = new TableConfig("table") + .withRelation("link", otherTable._id) + .withField("formula", FieldType.FORMULA) + .create() const result = await buildSqlFieldList(table, allTables, { relationships: true, @@ -211,61 +268,23 @@ describe("buildSqlFieldList", () => { "linkedTable.name", "linkedTable.description", "linkedTable.amount", - "linkedTable.id", "linkedTable.hidden", ]) }) it("never includes non-sql columns from relationships", async () => { - const otherTable: Table = { - ...table, - name: "linkedTable", - schema: { - id: { - name: "id", - type: FieldType.NUMBER, - }, - hidden: { - name: "other", - type: FieldType.STRING, - visible: false, - }, - formula: { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - }, - ai: { - name: "ai", - type: FieldType.AI, - operation: AIOperationEnum.PROMPT, - }, - link: { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: "otherTableId", - }, - }, - } - - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), - } - table.schema.formula = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } - - const allTables: Record = { - otherTableId: otherTable, - } + const otherTable = new TableConfig("linkedTable") + .withField("id", FieldType.NUMBER) + .withField("hidden", FieldType.STRING, { visible: false }) + .withField("formula", FieldType.FORMULA) + .withField("ai", FieldType.AI) + .withRelation("link", "otherTableId") + .create() + + const table = new TableConfig("table") + .withRelation("link", otherTable._id) + .withField("formula", FieldType.FORMULA) + .create() const result = await buildSqlFieldList(table, allTables, { relationships: true, @@ -274,6 +293,9 @@ describe("buildSqlFieldList", () => { "table.name", "table.description", "table.amount", + "linkedTable.name", + "linkedTable.description", + "linkedTable.amount", "linkedTable.id", "linkedTable.hidden", ]) @@ -281,41 +303,25 @@ describe("buildSqlFieldList", () => { }) describe("view", () => { - let view: ViewV2 - - beforeEach(() => { - getTableMock.mockResolvedValueOnce(table) - - view = { - version: 2, - id: generateViewID(table._id), - name: generator.word(), - tableId: table._id, - } - }) - it("extracts fields from table schema", async () => { - view.schema = { - name: { visible: false }, - amount: { visible: true }, - } + const view = new ViewConfig(new TableConfig("table").create()) + .withVisible("amount") + .withHidden("name") + .create() const result = await buildSqlFieldList(view, {}) expect(result).toEqual(["table.amount"]) }) it("includes all fields if there is a formula column", async () => { - table.schema.formula = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } - - view.schema = { - name: { visible: false }, - amount: { visible: true }, - formula: { visible: true }, - } + const table = new TableConfig("table") + .withField("formula", FieldType.FORMULA) + .create() + const view = new ViewConfig(table) + .withHidden("name") + .withVisible("amount") + .withVisible("formula") + .create() const result = await buildSqlFieldList(view, {}) expect(result).toEqual([ @@ -326,53 +332,35 @@ describe("buildSqlFieldList", () => { }) it("does not includes all fields if the formula column is not included", async () => { - table.schema.formula = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } - - view.schema = { - name: { visible: false }, - amount: { visible: true }, - formula: { visible: false }, - } + const table = new TableConfig("table") + .withField("formula", FieldType.FORMULA) + .create() + const view = new ViewConfig(table) + .withHidden("name") + .withVisible("amount") + .withHidden("formula") + .create() const result = await buildSqlFieldList(view, {}) expect(result).toEqual(["table.amount"]) }) it("includes relationships fields when flagged", async () => { - const otherTable: Table = { - ...table, - name: "linkedTable", - primary: ["id"], - primaryDisplay: "name", - schema: { - ...table.schema, - id: { - name: "id", - type: FieldType.NUMBER, - }, - }, - } - - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), - } - - view.schema = { - name: { visible: true }, - link: { visible: true }, - } - - const allTables: Record = { - otherTableId: otherTable, - } + const otherTable = new TableConfig("linkedTable") + .withField("id", FieldType.NUMBER) + .withPrimary("id") + .withDisplay("name") + .create() + + const table = new TableConfig("table") + .withRelation("link", otherTable._id) + .withField("formula", FieldType.FORMULA) + .create() + + const view = new ViewConfig(table) + .withVisible("name") + .withHidden("amount") + .create() const result = await buildSqlFieldList(view, allTables, { relationships: true, @@ -385,47 +373,25 @@ describe("buildSqlFieldList", () => { }) it("includes relationships columns", async () => { - const otherTable: Table = { - ...table, - name: "linkedTable", - primary: ["id"], - schema: { - ...table.schema, - id: { - name: "id", - type: FieldType.NUMBER, - }, - formula: { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - }, - }, - } - - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), - } - - view.schema = { - name: { visible: true }, - link: { - visible: true, - columns: { - name: { visible: false }, - amount: { visible: true }, - formula: { visible: false }, - }, - }, - } - - const allTables: Record = { - otherTableId: otherTable, - } + const otherTable = new TableConfig("linkedTable") + .withField("id", FieldType.NUMBER) + .withField("formula", FieldType.FORMULA) + .withPrimary("id") + .create() + + const table = new TableConfig("table") + .withRelation("link", otherTable._id) + .create() + + const view = new ViewConfig(table) + .withVisible("name") + .withVisible("link") + .withRelationshipColumns("link", { + name: { visible: false }, + amount: { visible: true }, + formula: { visible: false }, + }) + .create() const result = await buildSqlFieldList(view, allTables, { relationships: true, @@ -438,47 +404,25 @@ describe("buildSqlFieldList", () => { }) it("does not include relationships columns for hidden links", async () => { - const otherTable: Table = { - ...table, - name: "linkedTable", - primary: ["id"], - schema: { - ...table.schema, - id: { - name: "id", - type: FieldType.NUMBER, - }, - formula: { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - }, - }, - } - - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), - } - - view.schema = { - name: { visible: true }, - link: { - visible: false, - columns: { - name: { visible: false }, - amount: { visible: true }, - formula: { visible: false }, - }, - }, - } - - const allTables: Record = { - otherTableId: otherTable, - } + const otherTable = new TableConfig("linkedTable") + .withField("id", FieldType.NUMBER) + .withField("formula", FieldType.FORMULA) + .withPrimary("id") + .create() + + const table = new TableConfig("table") + .withRelation("link", otherTable._id) + .create() + + const view = new ViewConfig(table) + .withVisible("name") + .withHidden("link") + .withRelationshipColumns("link", { + name: { visible: false }, + amount: { visible: true }, + formula: { visible: false }, + }) + .create() const result = await buildSqlFieldList(view, allTables, { relationships: true, @@ -487,69 +431,30 @@ describe("buildSqlFieldList", () => { }) it("includes all relationship fields if there is a formula column", async () => { - const otherTable: Table = { - ...table, - name: "linkedTable", - schema: { - ...table.schema, - id: { - name: "id", - type: FieldType.NUMBER, - }, - hidden: { - name: "other", - type: FieldType.STRING, - visible: false, - }, - formula: { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - }, - ai: { - name: "ai", - type: FieldType.AI, - operation: AIOperationEnum.PROMPT, - }, - link: { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: "otherTableId", - }, - }, - } - - table.schema.link = { - name: "link", - type: FieldType.LINK, - relationshipType: RelationshipType.ONE_TO_MANY, - fieldName: "link", - tableId: sql.utils.buildExternalTableId("ds_id", "otherTableId"), - } - table.schema.formula = { - name: "formula", - type: FieldType.FORMULA, - formula: "any", - } - - view.schema = { - name: { visible: true }, - formula: { visible: true }, - link: { - visible: false, - columns: { - name: { visible: false }, - amount: { visible: true }, - formula: { visible: false }, - }, - }, - } - - const allTables: Record = { - otherTableId: otherTable, - } + const otherTable = new TableConfig("linkedTable") + .withField("id", FieldType.NUMBER) + .withField("hidden", FieldType.STRING, { visible: false }) + .withField("formula", FieldType.FORMULA) + .withField("ai", FieldType.AI) + .withRelation("link", "otherTableId") + .withPrimary("id") + .create() + + const table = new TableConfig("table") + .withRelation("link", otherTable._id) + .withField("formula", FieldType.FORMULA) + .create() + + const view = new ViewConfig(table) + .withVisible("name") + .withVisible("formula") + .withHidden("link") + .withRelationshipColumns("link", { + name: { visible: false }, + amount: { visible: true }, + formula: { visible: false }, + }) + .create() const result = await buildSqlFieldList(view, allTables, { relationships: true, From 92e791f9a715235856c2f318965329d713c7b893 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 19 Dec 2024 12:48:33 +0100 Subject: [PATCH 45/51] Add calculation tests --- .../row/utils/tests/sqlUtils.spec.ts | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts index eafdd170f8b..12a5edd30ba 100644 --- a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts +++ b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts @@ -1,10 +1,12 @@ import { AIOperationEnum, + CalculationType, FieldType, RelationshipType, SourceName, Table, ViewV2, + ViewV2Type, } from "@budibase/types" import { buildSqlFieldList } from "../sqlUtils" import { structures } from "../../../../routes/tests/utilities" @@ -166,6 +168,21 @@ describe("buildSqlFieldList", () => { return this } + withCalculation( + name: string, + field: string, + calculationType: CalculationType + ) { + this._view.type = ViewV2Type.CALCULATION + this._view.schema ??= {} + this._view.schema[name] = { + field, + calculationType, + visible: true, + } + return this + } + create() { getTableMock.mockResolvedValueOnce(this._table) return cloneDeep(this._view) @@ -471,4 +488,28 @@ describe("buildSqlFieldList", () => { ]) }) }) + + describe("calculation view", () => { + it("does not include calculation fields", async () => { + const view = new ViewConfig(new TableConfig("table").create()) + .withCalculation("average", "amount", CalculationType.AVG) + + .create() + + const result = await buildSqlFieldList(view, {}) + expect(result).toEqual([]) + }) + + it("includes visible fields calculation fields", async () => { + const view = new ViewConfig(new TableConfig("table").create()) + .withCalculation("average", "amount", CalculationType.AVG) + .withHidden("name") + .withVisible("amount") + + .create() + + const result = await buildSqlFieldList(view, {}) + expect(result).toEqual(["table.amount"]) + }) + }) }) From 889197679e1595b78bff806292372997000cedc2 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 19 Dec 2024 12:39:00 +0000 Subject: [PATCH 46/51] Comment to explain the behaviour of the junction document select. --- packages/server/src/sdk/app/rows/search/internal/sqs.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index c92d186a373..449ba477b75 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -121,6 +121,12 @@ async function buildInternalFieldList( if (!relatedTable) { continue } + // a quirk of how junction documents work in Budibase, refer to the "LinkDocument" type to see the full + // structure - essentially all relationships between two tables will be inserted into a single "table" + // we don't use an independent junction table ID for each separate relationship between two tables. For + // example if we have table A and B, with two relationships between them, all the junction documents will + // end up in the same junction table ID. We need to retrieve the field name property of the junction documents + // as part of the relationship to tell us which relationship column the junction is related to. const relatedFields = ( await buildInternalFieldList(relatedTable, tables) ).concat( From 0ee432dd9a34e478705a0c74a0535965fd33cd37 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 19 Dec 2024 13:32:47 +0100 Subject: [PATCH 47/51] Always use tableid prefix for sqs --- packages/server/src/sdk/app/rows/search/internal/sqs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index 449ba477b75..5b03887d365 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -142,7 +142,7 @@ async function buildInternalFieldList( if (!isView || !helpers.views.isCalculationView(source)) { for (const field of PROTECTED_INTERNAL_COLUMNS) { - fieldList.push(field) + fieldList.push(`${table._id}.${field}`) } } From 5a9ed4ff52a9c016b65df931f542f08569820948 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 19 Dec 2024 13:48:10 +0100 Subject: [PATCH 48/51] Include hidden fields for formulas --- .../src/sdk/app/rows/search/internal/sqs.ts | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index 5b03887d365..68d9d3420fc 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -55,12 +55,16 @@ const MISSING_COLUMN_REGEX = new RegExp(`no such column: .+`) const MISSING_TABLE_REGX = new RegExp(`no such table: .+`) const DUPLICATE_COLUMN_REGEX = new RegExp(`duplicate column name: .+`) -async function buildInternalFieldList( +export async function buildInternalFieldList( source: Table | ViewV2, tables: Table[], - opts?: { relationships?: RelationshipsJson[]; allowedFields?: string[] } + opts?: { + relationships?: RelationshipsJson[] + allowedFields?: string[] + includeHiddenFields?: boolean + } ) { - const { relationships, allowedFields } = opts || {} + const { relationships, allowedFields, includeHiddenFields } = opts || {} let schemaFields: string[] = [] const isView = sdk.views.isView(source) @@ -75,7 +79,7 @@ async function buildInternalFieldList( schemaFields = Object.keys(helpers.views.basicFields(source)) } else { schemaFields = Object.keys(source.schema).filter( - key => source.schema[key].visible !== false + key => includeHiddenFields || source.schema[key].visible !== false ) } @@ -109,10 +113,16 @@ async function buildInternalFieldList( } for (let key of schemaFields) { const col = table.schema[key] + + if ([FieldType.FORMULA, FieldType.AI].includes(col.type)) { + continue + } + const isRelationship = col.type === FieldType.LINK if (!relationships && isRelationship) { continue } + if (!isRelationship) { fieldList.push(`${table._id}.${mapToUserColumn(key)}`) } else { @@ -121,6 +131,7 @@ async function buildInternalFieldList( if (!relatedTable) { continue } + // a quirk of how junction documents work in Budibase, refer to the "LinkDocument" type to see the full // structure - essentially all relationships between two tables will be inserted into a single "table" // we don't use an independent junction table ID for each separate relationship between two tables. For @@ -128,7 +139,9 @@ async function buildInternalFieldList( // end up in the same junction table ID. We need to retrieve the field name property of the junction documents // as part of the relationship to tell us which relationship column the junction is related to. const relatedFields = ( - await buildInternalFieldList(relatedTable, tables) + await buildInternalFieldList(relatedTable, tables, { + includeHiddenFields: containsFormula, + }) ).concat( getJunctionFields(relatedTable, ["doc1.fieldName", "doc2.fieldName"]) ) From 7cc03b172486c32b98144398a1940e7559dba6bf Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 19 Dec 2024 14:31:45 +0100 Subject: [PATCH 49/51] Create sql tests for sqs --- .../app/rows/search/internal/test/sqs.spec.ts | 674 ++++++++++++++++++ 1 file changed, 674 insertions(+) create mode 100644 packages/server/src/sdk/app/rows/search/internal/test/sqs.spec.ts diff --git a/packages/server/src/sdk/app/rows/search/internal/test/sqs.spec.ts b/packages/server/src/sdk/app/rows/search/internal/test/sqs.spec.ts new file mode 100644 index 00000000000..31c25defad7 --- /dev/null +++ b/packages/server/src/sdk/app/rows/search/internal/test/sqs.spec.ts @@ -0,0 +1,674 @@ +import { + AIOperationEnum, + CalculationType, + FieldType, + RelationshipType, + SourceName, + Table, + ViewV2, + ViewV2Type, +} from "@budibase/types" +import { buildInternalFieldList } from "../sqs" +import { structures } from "../../../../../../api/routes/tests/utilities" +import { sql } from "@budibase/backend-core" +import { generator } from "@budibase/backend-core/tests" +import { + generateJunctionTableID, + generateViewID, +} from "../../../../../../db/utils" + +import sdk from "../../../../../../sdk" +import { cloneDeep } from "lodash" +import { utils } from "@budibase/shared-core" + +jest.mock("../../../../../../sdk/app/views", () => ({ + ...jest.requireActual("../../../../../../sdk/app/views"), + getTable: jest.fn(), +})) +const getTableMock = sdk.views.getTable as jest.MockedFunction< + typeof sdk.views.getTable +> + +describe("buildInternalFieldList", () => { + let allTables: Table[] + + class TableConfig { + private _table: Table & { _id: string } + + constructor() { + const name = generator.word() + this._table = { + ...structures.tableForDatasource({ + type: "datasource", + source: SourceName.POSTGRES, + }), + name, + _id: sql.utils.buildExternalTableId("ds_id", name), + schema: { + name: { + name: "name", + type: FieldType.STRING, + }, + description: { + name: "description", + type: FieldType.STRING, + }, + amount: { + name: "amount", + type: FieldType.NUMBER, + }, + }, + } + + allTables.push(this._table) + } + + withHiddenField(field: string) { + this._table.schema[field].visible = false + return this + } + + withField( + name: string, + type: + | FieldType.STRING + | FieldType.NUMBER + | FieldType.FORMULA + | FieldType.AI, + options?: { visible: boolean } + ) { + switch (type) { + case FieldType.NUMBER: + case FieldType.STRING: + this._table.schema[name] = { + name, + type, + ...options, + } + break + case FieldType.FORMULA: + this._table.schema[name] = { + name, + type, + formula: "any", + ...options, + } + break + case FieldType.AI: + this._table.schema[name] = { + name, + type, + operation: AIOperationEnum.PROMPT, + ...options, + } + break + default: + utils.unreachable(type) + } + return this + } + + withRelation(name: string, toTableId: string) { + this._table.schema[name] = { + name, + type: FieldType.LINK, + relationshipType: RelationshipType.ONE_TO_MANY, + fieldName: "link", + tableId: toTableId, + } + return this + } + + withEmptySchema() { + this._table.schema = {} + return this + } + + create() { + return cloneDeep(this._table) + } + } + + class ViewConfig { + private _table: Table + private _view: ViewV2 + + constructor(table: Table) { + this._table = table + this._view = { + version: 2, + id: generateViewID(table._id!), + name: generator.word(), + tableId: table._id!, + } + } + + withVisible(field: string) { + this._view.schema ??= {} + this._view.schema[field] ??= {} + this._view.schema[field].visible = true + return this + } + + withHidden(field: string) { + this._view.schema ??= {} + this._view.schema[field] ??= {} + this._view.schema[field].visible = false + return this + } + + withRelationshipColumns( + field: string, + columns: Record + ) { + this._view.schema ??= {} + this._view.schema[field] ??= {} + this._view.schema[field].columns = columns + return this + } + + withCalculation( + name: string, + field: string, + calculationType: CalculationType + ) { + this._view.type = ViewV2Type.CALCULATION + this._view.schema ??= {} + this._view.schema[name] = { + field, + calculationType, + visible: true, + } + return this + } + + create() { + getTableMock.mockResolvedValueOnce(this._table) + return cloneDeep(this._view) + } + } + + beforeEach(() => { + jest.clearAllMocks() + allTables = [] + }) + + describe("table", () => { + it("includes internal columns by default", async () => { + const table = new TableConfig().withEmptySchema().create() + const result = await buildInternalFieldList(table, []) + expect(result).toEqual([ + `${table._id}._id`, + `${table._id}._rev`, + `${table._id}.type`, + `${table._id}.createdAt`, + `${table._id}.updatedAt`, + `${table._id}.tableId`, + ]) + }) + + it("extracts fields from table schema", async () => { + const table = new TableConfig().create() + const result = await buildInternalFieldList(table, []) + expect(result).toEqual([ + `${table._id}.data_name`, + `${table._id}.data_description`, + `${table._id}.data_amount`, + `${table._id}._id`, + `${table._id}._rev`, + `${table._id}.type`, + `${table._id}.createdAt`, + `${table._id}.updatedAt`, + `${table._id}.tableId`, + ]) + }) + + it("excludes hidden fields", async () => { + const table = new TableConfig().withHiddenField("description").create() + const result = await buildInternalFieldList(table, []) + expect(result).toEqual([ + `${table._id}.data_name`, + `${table._id}.data_amount`, + `${table._id}._id`, + `${table._id}._rev`, + `${table._id}.type`, + `${table._id}.createdAt`, + `${table._id}.updatedAt`, + `${table._id}.tableId`, + ]) + }) + + it("excludes non-sql fields fields", async () => { + const table = new TableConfig() + .withField("formula", FieldType.FORMULA) + .withField("ai", FieldType.AI) + .withRelation("link", "otherTableId") + .create() + + const result = await buildInternalFieldList(table, []) + expect(result).toEqual([ + `${table._id}.data_name`, + `${table._id}.data_description`, + `${table._id}.data_amount`, + `${table._id}._id`, + `${table._id}._rev`, + `${table._id}.type`, + `${table._id}.createdAt`, + `${table._id}.updatedAt`, + `${table._id}.tableId`, + ]) + }) + + it("includes hidden fields if there is a formula column", async () => { + const table = new TableConfig() + .withHiddenField("description") + .withField("formula", FieldType.FORMULA) + .create() + + const result = await buildInternalFieldList(table, []) + expect(result).toEqual([ + `${table._id}.data_name`, + `${table._id}.data_description`, + `${table._id}.data_amount`, + `${table._id}._id`, + `${table._id}._rev`, + `${table._id}.type`, + `${table._id}.createdAt`, + `${table._id}.updatedAt`, + `${table._id}.tableId`, + ]) + }) + + it("includes relationships fields when flagged", async () => { + const otherTable = new TableConfig() + .withHiddenField("description") + .create() + + const table = new TableConfig() + .withHiddenField("amount") + .withRelation("link", otherTable._id) + .create() + + const relationships = [{ tableName: otherTable.name, column: "link" }] + + const result = await buildInternalFieldList(table, allTables, { + relationships, + }) + expect(result).toEqual([ + `${table._id}.data_name`, + `${table._id}.data_description`, + `${otherTable._id}.data_name`, + `${otherTable._id}.data_amount`, + `${otherTable._id}._id`, + `${otherTable._id}._rev`, + `${otherTable._id}.type`, + `${otherTable._id}.createdAt`, + `${otherTable._id}.updatedAt`, + `${otherTable._id}.tableId`, + `${generateJunctionTableID(table._id, otherTable._id)}.doc1.fieldName`, + `${generateJunctionTableID(table._id, otherTable._id)}.doc2.fieldName`, + `${table._id}._id`, + `${table._id}._rev`, + `${table._id}.type`, + `${table._id}.createdAt`, + `${table._id}.updatedAt`, + `${table._id}.tableId`, + ]) + }) + + it("includes all relationship fields if there is a formula column", async () => { + const otherTable = new TableConfig() + .withField("hidden", FieldType.STRING, { visible: false }) + .create() + + const table = new TableConfig() + .withRelation("link", otherTable._id) + .withHiddenField("description") + .withField("formula", FieldType.FORMULA) + .create() + + const relationships = [{ tableName: otherTable.name, column: "link" }] + const result = await buildInternalFieldList(table, allTables, { + relationships, + }) + expect(result).toEqual([ + `${table._id}.data_name`, + `${table._id}.data_description`, + `${table._id}.data_amount`, + `${otherTable._id}.data_name`, + `${otherTable._id}.data_description`, + `${otherTable._id}.data_amount`, + `${otherTable._id}.data_hidden`, + `${otherTable._id}._id`, + `${otherTable._id}._rev`, + `${otherTable._id}.type`, + `${otherTable._id}.createdAt`, + `${otherTable._id}.updatedAt`, + `${otherTable._id}.tableId`, + `${generateJunctionTableID(table._id, otherTable._id)}.doc1.fieldName`, + `${generateJunctionTableID(table._id, otherTable._id)}.doc2.fieldName`, + `${table._id}._id`, + `${table._id}._rev`, + `${table._id}.type`, + `${table._id}.createdAt`, + `${table._id}.updatedAt`, + `${table._id}.tableId`, + ]) + }) + + it("never includes non-sql columns from relationships", async () => { + const otherTable = new TableConfig() + .withField("hidden", FieldType.STRING, { visible: false }) + .withField("formula", FieldType.FORMULA) + .withField("ai", FieldType.AI) + .withRelation("link", "otherTableId") + .create() + + const table = new TableConfig() + .withRelation("link", otherTable._id) + .withField("formula", FieldType.FORMULA) + .create() + + const relationships = [{ tableName: otherTable.name, column: "link" }] + const result = await buildInternalFieldList(table, allTables, { + relationships, + }) + expect(result).toEqual([ + `${table._id}.data_name`, + `${table._id}.data_description`, + `${table._id}.data_amount`, + `${otherTable._id}.data_name`, + `${otherTable._id}.data_description`, + `${otherTable._id}.data_amount`, + `${otherTable._id}.data_hidden`, + `${otherTable._id}._id`, + `${otherTable._id}._rev`, + `${otherTable._id}.type`, + `${otherTable._id}.createdAt`, + `${otherTable._id}.updatedAt`, + `${otherTable._id}.tableId`, + `${generateJunctionTableID(table._id, otherTable._id)}.doc1.fieldName`, + `${generateJunctionTableID(table._id, otherTable._id)}.doc2.fieldName`, + `${table._id}._id`, + `${table._id}._rev`, + `${table._id}.type`, + `${table._id}.createdAt`, + `${table._id}.updatedAt`, + `${table._id}.tableId`, + ]) + }) + }) + + describe("view", () => { + it("includes internal columns by default", async () => { + const view = new ViewConfig(new TableConfig().create()).create() + const result = await buildInternalFieldList(view, []) + expect(result).toEqual([ + `${view.tableId}._id`, + `${view.tableId}._rev`, + `${view.tableId}.type`, + `${view.tableId}.createdAt`, + `${view.tableId}.updatedAt`, + `${view.tableId}.tableId`, + ]) + }) + + it("extracts fields from table schema", async () => { + const view = new ViewConfig(new TableConfig().create()) + .withVisible("amount") + .withHidden("name") + .create() + + const result = await buildInternalFieldList(view, []) + expect(result).toEqual([ + `${view.tableId}.data_amount`, + `${view.tableId}._id`, + `${view.tableId}._rev`, + `${view.tableId}.type`, + `${view.tableId}.createdAt`, + `${view.tableId}.updatedAt`, + `${view.tableId}.tableId`, + ]) + }) + + it("includes all fields if there is a formula column", async () => { + const table = new TableConfig() + .withField("formula", FieldType.FORMULA) + .create() + const view = new ViewConfig(table) + .withHidden("name") + .withVisible("amount") + .withVisible("formula") + .create() + + const result = await buildInternalFieldList(view, []) + expect(result).toEqual([ + `${view.tableId}.data_name`, + `${view.tableId}.data_description`, + `${view.tableId}.data_amount`, + `${view.tableId}._id`, + `${view.tableId}._rev`, + `${view.tableId}.type`, + `${view.tableId}.createdAt`, + `${view.tableId}.updatedAt`, + `${view.tableId}.tableId`, + ]) + }) + + it("does not includes all fields if the formula column is not included", async () => { + const table = new TableConfig() + .withField("formula", FieldType.FORMULA) + .create() + const view = new ViewConfig(table) + .withHidden("name") + .withVisible("amount") + .withHidden("formula") + .create() + + const result = await buildInternalFieldList(view, []) + expect(result).toEqual([ + `${view.tableId}.data_amount`, + `${view.tableId}._id`, + `${view.tableId}._rev`, + `${view.tableId}.type`, + `${view.tableId}.createdAt`, + `${view.tableId}.updatedAt`, + `${view.tableId}.tableId`, + ]) + }) + + it("includes relationships fields", async () => { + const otherTable = new TableConfig().create() + + const table = new TableConfig() + .withRelation("link", otherTable._id) + .withField("formula", FieldType.FORMULA) + .create() + + const view = new ViewConfig(table) + .withVisible("name") + .withVisible("link") + .withHidden("amount") + .create() + + const relationships = [{ tableName: otherTable.name, column: "link" }] + const result = await buildInternalFieldList(view, allTables, { + relationships, + }) + expect(result).toEqual([ + `${table._id}.data_name`, + `${otherTable._id}.data_name`, + `${otherTable._id}.data_description`, + `${otherTable._id}.data_amount`, + `${otherTable._id}._id`, + `${otherTable._id}._rev`, + `${otherTable._id}.type`, + `${otherTable._id}.createdAt`, + `${otherTable._id}.updatedAt`, + `${otherTable._id}.tableId`, + `${generateJunctionTableID(table._id, otherTable._id)}.doc1.fieldName`, + `${generateJunctionTableID(table._id, otherTable._id)}.doc2.fieldName`, + `${table._id}._id`, + `${table._id}._rev`, + `${table._id}.type`, + `${table._id}.createdAt`, + `${table._id}.updatedAt`, + `${table._id}.tableId`, + ]) + }) + + it("includes relationships columns", async () => { + const otherTable = new TableConfig() + .withField("formula", FieldType.FORMULA) + .create() + + const table = new TableConfig() + .withRelation("link", otherTable._id) + .create() + + const view = new ViewConfig(table) + .withVisible("name") + .withVisible("link") + .withRelationshipColumns("link", { + name: { visible: false }, + amount: { visible: true }, + formula: { visible: false }, + }) + .create() + + const relationships = [{ tableName: otherTable.name, column: "link" }] + const result = await buildInternalFieldList(view, allTables, { + relationships, + }) + expect(result).toEqual([ + `${table._id}.data_name`, + `${otherTable._id}.data_name`, + `${otherTable._id}.data_description`, + `${otherTable._id}.data_amount`, + `${otherTable._id}._id`, + `${otherTable._id}._rev`, + `${otherTable._id}.type`, + `${otherTable._id}.createdAt`, + `${otherTable._id}.updatedAt`, + `${otherTable._id}.tableId`, + `${generateJunctionTableID(table._id, otherTable._id)}.doc1.fieldName`, + `${generateJunctionTableID(table._id, otherTable._id)}.doc2.fieldName`, + `${table._id}._id`, + `${table._id}._rev`, + `${table._id}.type`, + `${table._id}.createdAt`, + `${table._id}.updatedAt`, + `${table._id}.tableId`, + ]) + }) + + it("does not include relationships columns for hidden links", async () => { + const otherTable = new TableConfig() + .withField("formula", FieldType.FORMULA) + .create() + + const table = new TableConfig() + .withRelation("link", otherTable._id) + .create() + + const view = new ViewConfig(table) + .withVisible("name") + .withHidden("link") + .withRelationshipColumns("link", { + name: { visible: false }, + amount: { visible: true }, + formula: { visible: false }, + }) + .create() + + const relationships = [{ tableName: otherTable.name, column: "link" }] + const result = await buildInternalFieldList(view, allTables, { + relationships, + }) + expect(result).toEqual([ + `${table._id}.data_name`, + `${table._id}._id`, + `${table._id}._rev`, + `${table._id}.type`, + `${table._id}.createdAt`, + `${table._id}.updatedAt`, + `${table._id}.tableId`, + ]) + }) + + it("includes all relationship fields if there is a formula column", async () => { + const otherTable = new TableConfig() + .withField("hidden", FieldType.STRING, { visible: false }) + .withField("formula", FieldType.FORMULA) + .withField("ai", FieldType.AI) + .withRelation("link", "otherTableId") + .create() + + const table = new TableConfig() + .withRelation("link", otherTable._id) + .withField("formula", FieldType.FORMULA) + .create() + + const view = new ViewConfig(table) + .withVisible("name") + .withVisible("formula") + .withHidden("link") + .withRelationshipColumns("link", { + name: { visible: false }, + amount: { visible: true }, + formula: { visible: false }, + }) + .create() + + const relationships = [{ tableName: otherTable.name, column: "link" }] + const result = await buildInternalFieldList(view, allTables, { + relationships, + }) + expect(result).toEqual([ + `${table._id}.data_name`, + `${table._id}.data_description`, + `${table._id}.data_amount`, + `${otherTable._id}.data_name`, + `${otherTable._id}.data_description`, + `${otherTable._id}.data_amount`, + `${otherTable._id}.data_hidden`, + `${otherTable._id}._id`, + `${otherTable._id}._rev`, + `${otherTable._id}.type`, + `${otherTable._id}.createdAt`, + `${otherTable._id}.updatedAt`, + `${otherTable._id}.tableId`, + `${generateJunctionTableID(table._id, otherTable._id)}.doc1.fieldName`, + `${generateJunctionTableID(table._id, otherTable._id)}.doc2.fieldName`, + `${table._id}._id`, + `${table._id}._rev`, + `${table._id}.type`, + `${table._id}.createdAt`, + `${table._id}.updatedAt`, + `${table._id}.tableId`, + ]) + }) + }) + + describe("calculation view", () => { + it("does not include calculation fields", async () => { + const view = new ViewConfig(new TableConfig().create()) + .withCalculation("average", "amount", CalculationType.AVG) + .create() + + const result = await buildInternalFieldList(view, []) + expect(result).toEqual([]) + }) + + it("includes visible fields calculation fields", async () => { + const view = new ViewConfig(new TableConfig().create()) + .withCalculation("average", "amount", CalculationType.AVG) + .withHidden("name") + .withVisible("amount") + .create() + + const result = await buildInternalFieldList(view, []) + expect(result).toEqual([`${view.tableId}.data_amount`]) + }) + }) +}) From 16fb86549b7cb47148cb92f695d2e6be6449905c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 19 Dec 2024 14:42:49 +0100 Subject: [PATCH 50/51] Fix tests --- .../src/api/controllers/row/utils/sqlUtils.ts | 4 +-- .../row/utils/tests/sqlUtils.spec.ts | 32 ++++++++----------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index e66a0b5bf69..1793d64b895 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -200,8 +200,8 @@ export async function buildSqlFieldList( if ( isView && - source.schema?.[field.name] && - !helpers.views.isVisible(source.schema[field.name]) && + (!source.schema?.[field.name] || + !helpers.views.isVisible(source.schema[field.name])) && !containsFormula ) { continue diff --git a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts index 12a5edd30ba..365f571fcf6 100644 --- a/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts +++ b/packages/server/src/api/controllers/row/utils/tests/sqlUtils.spec.ts @@ -362,21 +362,25 @@ describe("buildSqlFieldList", () => { expect(result).toEqual(["table.amount"]) }) - it("includes relationships fields when flagged", async () => { + it("includes relationships columns", async () => { const otherTable = new TableConfig("linkedTable") .withField("id", FieldType.NUMBER) + .withField("formula", FieldType.FORMULA) .withPrimary("id") - .withDisplay("name") .create() const table = new TableConfig("table") .withRelation("link", otherTable._id) - .withField("formula", FieldType.FORMULA) .create() const view = new ViewConfig(table) .withVisible("name") - .withHidden("amount") + .withVisible("link") + .withRelationshipColumns("link", { + name: { visible: false }, + amount: { visible: true }, + formula: { visible: false }, + }) .create() const result = await buildSqlFieldList(view, allTables, { @@ -385,39 +389,31 @@ describe("buildSqlFieldList", () => { expect(result).toEqual([ "table.name", "linkedTable.id", - "linkedTable.name", + "linkedTable.amount", ]) }) - it("includes relationships columns", async () => { + it("excludes relationships fields when view is not included in the view", async () => { const otherTable = new TableConfig("linkedTable") .withField("id", FieldType.NUMBER) - .withField("formula", FieldType.FORMULA) .withPrimary("id") + .withDisplay("name") .create() const table = new TableConfig("table") .withRelation("link", otherTable._id) + .withField("formula", FieldType.FORMULA) .create() const view = new ViewConfig(table) .withVisible("name") - .withVisible("link") - .withRelationshipColumns("link", { - name: { visible: false }, - amount: { visible: true }, - formula: { visible: false }, - }) + .withHidden("amount") .create() const result = await buildSqlFieldList(view, allTables, { relationships: true, }) - expect(result).toEqual([ - "table.name", - "linkedTable.id", - "linkedTable.amount", - ]) + expect(result).toEqual(["table.name"]) }) it("does not include relationships columns for hidden links", async () => { From 8905f9dd5d5cc6ad174eb6d3a4ba7420b19bad2c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Thu, 19 Dec 2024 15:27:18 +0100 Subject: [PATCH 51/51] Fix tests --- .../src/sdk/app/rows/search/internal/sqs.ts | 4 -- .../app/rows/search/internal/test/sqs.spec.ts | 70 ++----------------- 2 files changed, 7 insertions(+), 67 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index 68d9d3420fc..84162a67af8 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -114,10 +114,6 @@ export async function buildInternalFieldList( for (let key of schemaFields) { const col = table.schema[key] - if ([FieldType.FORMULA, FieldType.AI].includes(col.type)) { - continue - } - const isRelationship = col.type === FieldType.LINK if (!relationships && isRelationship) { continue diff --git a/packages/server/src/sdk/app/rows/search/internal/test/sqs.spec.ts b/packages/server/src/sdk/app/rows/search/internal/test/sqs.spec.ts index 31c25defad7..91674089dc5 100644 --- a/packages/server/src/sdk/app/rows/search/internal/test/sqs.spec.ts +++ b/packages/server/src/sdk/app/rows/search/internal/test/sqs.spec.ts @@ -238,27 +238,6 @@ describe("buildInternalFieldList", () => { ]) }) - it("excludes non-sql fields fields", async () => { - const table = new TableConfig() - .withField("formula", FieldType.FORMULA) - .withField("ai", FieldType.AI) - .withRelation("link", "otherTableId") - .create() - - const result = await buildInternalFieldList(table, []) - expect(result).toEqual([ - `${table._id}.data_name`, - `${table._id}.data_description`, - `${table._id}.data_amount`, - `${table._id}._id`, - `${table._id}._rev`, - `${table._id}.type`, - `${table._id}.createdAt`, - `${table._id}.updatedAt`, - `${table._id}.tableId`, - ]) - }) - it("includes hidden fields if there is a formula column", async () => { const table = new TableConfig() .withHiddenField("description") @@ -270,6 +249,7 @@ describe("buildInternalFieldList", () => { `${table._id}.data_name`, `${table._id}.data_description`, `${table._id}.data_amount`, + `${table._id}.data_formula`, `${table._id}._id`, `${table._id}._rev`, `${table._id}.type`, @@ -347,48 +327,7 @@ describe("buildInternalFieldList", () => { `${otherTable._id}.tableId`, `${generateJunctionTableID(table._id, otherTable._id)}.doc1.fieldName`, `${generateJunctionTableID(table._id, otherTable._id)}.doc2.fieldName`, - `${table._id}._id`, - `${table._id}._rev`, - `${table._id}.type`, - `${table._id}.createdAt`, - `${table._id}.updatedAt`, - `${table._id}.tableId`, - ]) - }) - - it("never includes non-sql columns from relationships", async () => { - const otherTable = new TableConfig() - .withField("hidden", FieldType.STRING, { visible: false }) - .withField("formula", FieldType.FORMULA) - .withField("ai", FieldType.AI) - .withRelation("link", "otherTableId") - .create() - - const table = new TableConfig() - .withRelation("link", otherTable._id) - .withField("formula", FieldType.FORMULA) - .create() - - const relationships = [{ tableName: otherTable.name, column: "link" }] - const result = await buildInternalFieldList(table, allTables, { - relationships, - }) - expect(result).toEqual([ - `${table._id}.data_name`, - `${table._id}.data_description`, - `${table._id}.data_amount`, - `${otherTable._id}.data_name`, - `${otherTable._id}.data_description`, - `${otherTable._id}.data_amount`, - `${otherTable._id}.data_hidden`, - `${otherTable._id}._id`, - `${otherTable._id}._rev`, - `${otherTable._id}.type`, - `${otherTable._id}.createdAt`, - `${otherTable._id}.updatedAt`, - `${otherTable._id}.tableId`, - `${generateJunctionTableID(table._id, otherTable._id)}.doc1.fieldName`, - `${generateJunctionTableID(table._id, otherTable._id)}.doc2.fieldName`, + `${table._id}.data_formula`, `${table._id}._id`, `${table._id}._rev`, `${table._id}.type`, @@ -446,6 +385,7 @@ describe("buildInternalFieldList", () => { `${view.tableId}.data_name`, `${view.tableId}.data_description`, `${view.tableId}.data_amount`, + `${view.tableId}.data_formula`, `${view.tableId}._id`, `${view.tableId}._rev`, `${view.tableId}.type`, @@ -545,6 +485,7 @@ describe("buildInternalFieldList", () => { `${otherTable._id}.data_name`, `${otherTable._id}.data_description`, `${otherTable._id}.data_amount`, + `${otherTable._id}.data_formula`, `${otherTable._id}._id`, `${otherTable._id}._rev`, `${otherTable._id}.type`, @@ -632,6 +573,8 @@ describe("buildInternalFieldList", () => { `${otherTable._id}.data_description`, `${otherTable._id}.data_amount`, `${otherTable._id}.data_hidden`, + `${otherTable._id}.data_formula`, + `${otherTable._id}.data_ai`, `${otherTable._id}._id`, `${otherTable._id}._rev`, `${otherTable._id}.type`, @@ -640,6 +583,7 @@ describe("buildInternalFieldList", () => { `${otherTable._id}.tableId`, `${generateJunctionTableID(table._id, otherTable._id)}.doc1.fieldName`, `${generateJunctionTableID(table._id, otherTable._id)}.doc2.fieldName`, + `${table._id}.data_formula`, `${table._id}._id`, `${table._id}._rev`, `${table._id}.type`,