From aa1eaa1d3d7d7f23d5771a6a53c2ee2881f16905 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 15 Aug 2024 18:35:30 +0100 Subject: [PATCH 1/7] A few fixes for logical operators, there was a lot of cleanup that was not occurring as it is supposed to be recursive, this wasn't happening. --- .../server/src/api/controllers/row/views.ts | 3 +- .../src/sdk/app/rows/search/internal/sqs.ts | 43 ++++++++----------- packages/server/src/sdk/app/rows/sqlAlias.ts | 21 +++++---- packages/shared-core/src/filters.ts | 16 +++++++ 4 files changed, 50 insertions(+), 33 deletions(-) diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index c38a415aa2..bcf9c6b441 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -58,8 +58,9 @@ export async function searchView( }) }) } else { + const operator = query.allOr ? "$or" : "$and" query = { - $and: { + [operator]: { conditions: [query, body.query], }, } 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 4bfa8f8fa5..0c7946339e 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -139,34 +139,29 @@ function cleanupFilters( allTables.some(table => table.schema[key]) const splitter = new dataFilters.ColumnSplitter(allTables) - - const prefixFilters = (filters: SearchFilters) => { - for (const filterKey of Object.keys(filters) as (keyof SearchFilters)[]) { - if (isLogicalSearchOperator(filterKey)) { - for (const condition of filters[filterKey]!.conditions) { - prefixFilters(condition) - } - } else { - const filter = filters[filterKey]! - if (typeof filter !== "object") { - continue - } - for (const key of Object.keys(filter)) { - const { numberPrefix, relationshipPrefix, column } = splitter.run(key) - if (keyInAnyTable(column)) { - filter[ - `${numberPrefix || ""}${ - relationshipPrefix || "" - }${mapToUserColumn(column)}` - ] = filter[key] - delete filter[key] - } + for (const filterKey of Object.keys(filters) as (keyof SearchFilters)[]) { + if (!isLogicalSearchOperator(filterKey)) { + const filter = filters[filterKey]! + if (typeof filter !== "object") { + continue + } + for (const key of Object.keys(filter)) { + const { numberPrefix, relationshipPrefix, column } = splitter.run(key) + if (keyInAnyTable(column)) { + filter[ + `${numberPrefix || ""}${relationshipPrefix || ""}${mapToUserColumn( + column + )}` + ] = filter[key] + delete filter[key] } } } } - prefixFilters(filters) - return filters + + return dataFilters.recurseLogicalOperators(filters, (f: SearchFilters) => { + return cleanupFilters(f, table, allTables) + }) } function buildTableMap(tables: Table[]) { diff --git a/packages/server/src/sdk/app/rows/sqlAlias.ts b/packages/server/src/sdk/app/rows/sqlAlias.ts index 664e64057b..535709791d 100644 --- a/packages/server/src/sdk/app/rows/sqlAlias.ts +++ b/packages/server/src/sdk/app/rows/sqlAlias.ts @@ -12,6 +12,7 @@ import { getSQLClient } from "./utils" import { cloneDeep } from "lodash" import datasources from "../datasources" import { BudibaseInternalDB } from "../../../db/utils" +import { dataFilters } from "@budibase/shared-core" type PerformQueryFunction = ( datasource: Datasource, @@ -199,16 +200,20 @@ export default class AliasTables { ) } if (json.filters) { - for (let [filterKey, filter] of Object.entries(json.filters)) { - if (typeof filter !== "object") { - continue + const aliasFilters = (filters: SearchFilters): SearchFilters => { + for (let [filterKey, filter] of Object.entries(filters)) { + if (typeof filter !== "object") { + continue + } + const aliasedFilters: typeof filter = {} + for (let key of Object.keys(filter)) { + aliasedFilters[this.aliasField(key)] = filter[key] + } + filters[filterKey as keyof SearchFilters] = aliasedFilters } - const aliasedFilters: typeof filter = {} - for (let key of Object.keys(filter)) { - aliasedFilters[this.aliasField(key)] = filter[key] - } - json.filters[filterKey as keyof SearchFilters] = aliasedFilters + return dataFilters.recurseLogicalOperators(filters, aliasFilters) } + json.filters = aliasFilters(json.filters) } if (json.meta?.table) { this.getAlias(json.meta.table.name) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 2dd485a8a0..fa0b5c92ed 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -113,6 +113,20 @@ export const NoEmptyFilterStrings = [ OperatorOptions.In.value, ] as (keyof SearchQueryFields)[] +export function recurseLogicalOperators( + filters: SearchFilters, + fn: (f: SearchFilters) => SearchFilters +) { + for (const logical of Object.values(LogicalOperator)) { + if (filters[logical]) { + filters[logical]!.conditions = filters[logical]!.conditions.map( + condition => fn(condition) + ) + } + } + return filters +} + /** * Removes any fields that contain empty strings that would cause inconsistent * behaviour with how backend tables are filtered (no value means no filter). @@ -145,6 +159,7 @@ export const cleanupQuery = (query: SearchFilters) => { } } } + query = recurseLogicalOperators(query, cleanupQuery) return query } @@ -410,6 +425,7 @@ export function fixupFilterArrays(filters: SearchFilters) { } } } + recurseLogicalOperators(filters, fixupFilterArrays) return filters } From 48672244f4b4135566c0c063cb09afc6fe634619 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 15 Aug 2024 18:46:28 +0100 Subject: [PATCH 2/7] Fixing up column renaming everywhere that it is needed, making sure works for external as well. --- .../api/controllers/row/ExternalRequest.ts | 39 +++++++++-------- .../server/src/sdk/app/rows/search/filters.ts | 5 ++- .../src/sdk/app/rows/search/internal/sqs.ts | 43 +++++++++++-------- 3 files changed, 50 insertions(+), 37 deletions(-) diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index 5ee2d0fe2b..ac2d1e8c39 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -45,6 +45,7 @@ import { db as dbCore } from "@budibase/backend-core" import sdk from "../../../sdk" import env from "../../../environment" import { makeExternalQuery } from "../../../integrations/base/query" +import { dataFilters } from "@budibase/shared-core" export interface ManyRelationship { tableId?: string @@ -195,29 +196,33 @@ export class ExternalRequest { if (filters) { // need to map over the filters and make sure the _id field isn't present let prefix = 1 - for (const [operatorType, operator] of Object.entries(filters)) { - const isArrayOp = sdk.rows.utils.isArrayFilter(operatorType) - for (const field of Object.keys(operator || {})) { - if (dbCore.removeKeyNumbering(field) === "_id") { - if (primary) { - const parts = breakRowIdField(operator[field]) - if (primary.length > 1 && isArrayOp) { - operator[InternalSearchFilterOperator.COMPLEX_ID_OPERATOR] = { - id: primary, - values: parts[0], + const checkFilters = (innerFilters: SearchFilters): SearchFilters => { + for (const [operatorType, operator] of Object.entries(innerFilters)) { + const isArrayOp = sdk.rows.utils.isArrayFilter(operatorType) + for (const field of Object.keys(operator || {})) { + if (dbCore.removeKeyNumbering(field) === "_id") { + if (primary) { + const parts = breakRowIdField(operator[field]) + if (primary.length > 1 && isArrayOp) { + operator[InternalSearchFilterOperator.COMPLEX_ID_OPERATOR] = { + id: primary, + values: parts[0], + } + } else { + for (let field of primary) { + operator[`${prefix}:${field}`] = parts.shift() + } + prefix++ } - } else { - for (let field of primary) { - operator[`${prefix}:${field}`] = parts.shift() - } - prefix++ } + // make sure this field doesn't exist on any filter + delete operator[field] } - // make sure this field doesn't exist on any filter - delete operator[field] } } + return dataFilters.recurseLogicalOperators(innerFilters, checkFilters) } + checkFilters(filters) } // there is no id, just use the user provided filters if (!idCopy || !table) { diff --git a/packages/server/src/sdk/app/rows/search/filters.ts b/packages/server/src/sdk/app/rows/search/filters.ts index ccce0ab86a..4049fc5352 100644 --- a/packages/server/src/sdk/app/rows/search/filters.ts +++ b/packages/server/src/sdk/app/rows/search/filters.ts @@ -5,6 +5,7 @@ import { Table, } from "@budibase/types" import { isPlainObject } from "lodash" +import { dataFilters } from "@budibase/shared-core" export function getRelationshipColumns(table: Table): { name: string @@ -58,5 +59,7 @@ export function updateFilterKeys( } } } - return filters + return dataFilters.recurseLogicalOperators(filters, (f: SearchFilters) => { + return updateFilterKeys(f, updates) + }) } 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 0c7946339e..4bfa8f8fa5 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -139,29 +139,34 @@ function cleanupFilters( allTables.some(table => table.schema[key]) const splitter = new dataFilters.ColumnSplitter(allTables) - for (const filterKey of Object.keys(filters) as (keyof SearchFilters)[]) { - if (!isLogicalSearchOperator(filterKey)) { - const filter = filters[filterKey]! - if (typeof filter !== "object") { - continue - } - for (const key of Object.keys(filter)) { - const { numberPrefix, relationshipPrefix, column } = splitter.run(key) - if (keyInAnyTable(column)) { - filter[ - `${numberPrefix || ""}${relationshipPrefix || ""}${mapToUserColumn( - column - )}` - ] = filter[key] - delete filter[key] + + const prefixFilters = (filters: SearchFilters) => { + for (const filterKey of Object.keys(filters) as (keyof SearchFilters)[]) { + if (isLogicalSearchOperator(filterKey)) { + for (const condition of filters[filterKey]!.conditions) { + prefixFilters(condition) + } + } else { + const filter = filters[filterKey]! + if (typeof filter !== "object") { + continue + } + for (const key of Object.keys(filter)) { + const { numberPrefix, relationshipPrefix, column } = splitter.run(key) + if (keyInAnyTable(column)) { + filter[ + `${numberPrefix || ""}${ + relationshipPrefix || "" + }${mapToUserColumn(column)}` + ] = filter[key] + delete filter[key] + } } } } } - - return dataFilters.recurseLogicalOperators(filters, (f: SearchFilters) => { - return cleanupFilters(f, table, allTables) - }) + prefixFilters(filters) + return filters } function buildTableMap(tables: Table[]) { From a1fae4d79918e308a2978b13c286159f6fd8df5a Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 15 Aug 2024 18:52:17 +0100 Subject: [PATCH 3/7] Making sure filters are always added to end, this is important for OR situations. --- packages/backend-core/src/sql/sql.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index ebae09e156..69ad20505f 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -371,10 +371,11 @@ class InternalBuilder { ), castedTypeValue.values ) - } else if (!opts?.relationship && !isRelationshipField) { + } else if (!isRelationshipField) { const alias = getTableAlias(tableName) fn(alias ? `${alias}.${updatedKey}` : updatedKey, value) - } else if (opts?.relationship && isRelationshipField) { + } + if (opts?.relationship && isRelationshipField) { const [filterTableName, property] = updatedKey.split(".") const alias = getTableAlias(filterTableName) fn(alias ? `${alias}.${property}` : property, value) From f4d5eb31de2596033ae5abcdaeed7a73ab415569 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 19 Aug 2024 16:20:33 +0100 Subject: [PATCH 4/7] Fixing test case. --- .../src/integrations/tests/sqlAlias.spec.ts | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/server/src/integrations/tests/sqlAlias.spec.ts b/packages/server/src/integrations/tests/sqlAlias.spec.ts index 0b433896bf..6f34f4eb89 100644 --- a/packages/server/src/integrations/tests/sqlAlias.spec.ts +++ b/packages/server/src/integrations/tests/sqlAlias.spec.ts @@ -97,13 +97,14 @@ describe("Captures of real examples", () => { const filters = queryJson.filters?.oneOf?.taskid as number[] let query = new Sql(SqlClient.POSTGRES, limit)._query(queryJson) expect(query).toEqual({ - bindings: [...filters, limit, limit], - sql: multiline(`select "a"."executorid" as "a.executorid", "a"."taskname" as "a.taskname", - "a"."taskid" as "a.taskid", "a"."completed" as "a.completed", "a"."qaid" as "a.qaid", - "b"."productname" as "b.productname", "b"."productid" as "b.productid" - from (select * from "tasks" as "a" where "a"."taskid" in ($1, $2) order by "a"."taskid" asc limit $3) as "a" - left join "products_tasks" as "c" on "a"."taskid" = "c"."taskid" - left join "products" as "b" on "b"."productid" = "c"."productid" order by "a"."taskid" asc limit $4`), + bindings: [...filters, limit, ...filters, limit], + sql: multiline( + `select "a"."executorid" as "a.executorid", "a"."taskname" as "a.taskname", "a"."taskid" as "a.taskid", + "a"."completed" as "a.completed", "a"."qaid" as "a.qaid", "b"."productname" as "b.productname", "b"."productid" as "b.productid" + from (select * from "tasks" as "a" where "a"."taskid" in ($1, $2) order by "a"."taskid" asc limit $3) as "a" + left join "products_tasks" as "c" on "a"."taskid" = "c"."taskid" left join "products" as "b" on "b"."productid" = "c"."productid" + where "a"."taskid" in ($4, $5) order by "a"."taskid" asc limit $6` + ), }) }) @@ -123,6 +124,7 @@ describe("Captures of real examples", () => { rangeValue.low, rangeValue.high, equalValue, + true, limit, ], sql: expect.stringContaining( @@ -186,8 +188,9 @@ describe("Captures of real examples", () => { }, queryJson) expect(returningQuery).toEqual({ sql: multiline(`select top (@p0) * from (select top (@p1) * from [people] where CASE WHEN [people].[name] = @p2 - THEN 1 ELSE 0 END = 1 and CASE WHEN [people].[age] = @p3 THEN 1 ELSE 0 END = 1 order by [people].[name] asc) as [people]`), - bindings: [5000, 1, "Test", 22], + THEN 1 ELSE 0 END = 1 and CASE WHEN [people].[age] = @p3 THEN 1 ELSE 0 END = 1 order by [people].[name] asc) as [people] + where CASE WHEN [people].[name] = @p4 THEN 1 ELSE 0 END = 1 and CASE WHEN [people].[age] = @p5 THEN 1 ELSE 0 END = 1`), + bindings: [5000, 1, "Test", 22, "Test", 22], }) }) }) From a5533bb033fffab296feb4cb18d962a3802f4b63 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 19 Aug 2024 16:49:40 +0100 Subject: [PATCH 5/7] Fixing test cases. --- packages/backend-core/src/sql/sql.ts | 2 ++ .../server/src/integrations/tests/sql.spec.ts | 21 ++++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 69ad20505f..02c5e5519a 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -36,6 +36,7 @@ import { } from "@budibase/types" import environment from "../environment" import { dataFilters, helpers } from "@budibase/shared-core" +import { cloneDeep } from "lodash" type QueryFunction = (query: SqlQuery | SqlQuery[], operation: Operation) => any @@ -268,6 +269,7 @@ class InternalBuilder { } private parseFilters(filters: SearchFilters): SearchFilters { + filters = cloneDeep(filters) for (const op of Object.values(BasicOperator)) { const filter = filters[op] if (!filter) { diff --git a/packages/server/src/integrations/tests/sql.spec.ts b/packages/server/src/integrations/tests/sql.spec.ts index c4b2a69f7d..a6e63c434d 100644 --- a/packages/server/src/integrations/tests/sql.spec.ts +++ b/packages/server/src/integrations/tests/sql.spec.ts @@ -194,8 +194,8 @@ describe("SQL query builder", () => { }) ) expect(query).toEqual({ - bindings: ["john%", limit, 5000], - sql: `select * from (select * from (select * from (select * from "test" where LOWER("test"."name") LIKE :1 order by "test"."id" asc) where rownum <= :2) "test" order by "test"."id" asc) where rownum <= :3`, + bindings: ["john%", limit, "john%", 5000], + sql: `select * from (select * from (select * from (select * from "test" where LOWER("test"."name") LIKE :1 order by "test"."id" asc) where rownum <= :2) "test" where LOWER("test"."name") LIKE :3 order by "test"."id" asc) where rownum <= :4`, }) query = new Sql(SqlClient.ORACLE, limit)._query( @@ -208,9 +208,10 @@ describe("SQL query builder", () => { }, }) ) + const filterSet = [`%20%`, `%25%`, `%"john"%`, `%"mary"%`] expect(query).toEqual({ - bindings: ["%20%", "%25%", `%"john"%`, `%"mary"%`, limit, 5000], - sql: `select * from (select * from (select * from (select * from "test" where COALESCE(LOWER("test"."age"), '') LIKE :1 AND COALESCE(LOWER("test"."age"), '') LIKE :2 and COALESCE(LOWER("test"."name"), '') LIKE :3 AND COALESCE(LOWER("test"."name"), '') LIKE :4 order by "test"."id" asc) where rownum <= :5) "test" order by "test"."id" asc) where rownum <= :6`, + bindings: [...filterSet, limit, ...filterSet, 5000], + sql: `select * from (select * from (select * from (select * from "test" where COALESCE(LOWER("test"."age"), '') LIKE :1 AND COALESCE(LOWER("test"."age"), '') LIKE :2 and COALESCE(LOWER("test"."name"), '') LIKE :3 AND COALESCE(LOWER("test"."name"), '') LIKE :4 order by "test"."id" asc) where rownum <= :5) "test" where COALESCE(LOWER("test"."age"), '') LIKE :6 AND COALESCE(LOWER("test"."age"), '') LIKE :7 and COALESCE(LOWER("test"."name"), '') LIKE :8 AND COALESCE(LOWER("test"."name"), '') LIKE :9 order by "test"."id" asc) where rownum <= :10`, }) query = new Sql(SqlClient.ORACLE, limit)._query( @@ -223,8 +224,8 @@ describe("SQL query builder", () => { }) ) expect(query).toEqual({ - bindings: [`%jo%`, limit, 5000], - sql: `select * from (select * from (select * from (select * from "test" where LOWER("test"."name") LIKE :1 order by "test"."id" asc) where rownum <= :2) "test" order by "test"."id" asc) where rownum <= :3`, + bindings: [`%jo%`, limit, `%jo%`, 5000], + sql: `select * from (select * from (select * from (select * from "test" where LOWER("test"."name") LIKE :1 order by "test"."id" asc) where rownum <= :2) "test" where LOWER("test"."name") LIKE :3 order by "test"."id" asc) where rownum <= :4`, }) }) @@ -241,8 +242,8 @@ describe("SQL query builder", () => { ) expect(query).toEqual({ - bindings: ["John", limit, 5000], - sql: `select * from (select * from (select * from (select * from "test" where (to_char("test"."name") IS NOT NULL AND to_char("test"."name") = :1) order by "test"."id" asc) where rownum <= :2) "test" order by "test"."id" asc) where rownum <= :3`, + bindings: ["John", limit, "John", 5000], + sql: `select * from (select * from (select * from (select * from "test" where (to_char("test"."name") IS NOT NULL AND to_char("test"."name") = :1) order by "test"."id" asc) where rownum <= :2) "test" where (to_char("test"."name") IS NOT NULL AND to_char("test"."name") = :3) order by "test"."id" asc) where rownum <= :4`, }) }) @@ -259,8 +260,8 @@ describe("SQL query builder", () => { ) expect(query).toEqual({ - bindings: ["John", limit, 5000], - sql: `select * from (select * from (select * from (select * from "test" where (to_char("test"."name") IS NOT NULL AND to_char("test"."name") != :1) OR to_char("test"."name") IS NULL order by "test"."id" asc) where rownum <= :2) "test" order by "test"."id" asc) where rownum <= :3`, + bindings: ["John", limit, "John", 5000], + sql: `select * from (select * from (select * from (select * from "test" where (to_char("test"."name") IS NOT NULL AND to_char("test"."name") != :1) OR to_char("test"."name") IS NULL order by "test"."id" asc) where rownum <= :2) "test" where (to_char("test"."name") IS NOT NULL AND to_char("test"."name") != :3) OR to_char("test"."name") IS NULL order by "test"."id" asc) where rownum <= :4`, }) }) }) From e6c5a7ecd707b246f6e95721d4a6a98f3465915a Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 21 Aug 2024 11:05:48 +0100 Subject: [PATCH 6/7] PR comments. --- packages/server/src/api/controllers/row/views.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index bcf9c6b441..16d0f0205d 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -57,10 +57,9 @@ export async function searchView( } }) }) - } else { - const operator = query.allOr ? "$or" : "$and" + } else query = { - [operator]: { + $and: { conditions: [query, body.query], }, } From 40e7ab13718718a892b713cbb3ef80342607b4b4 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 21 Aug 2024 13:45:26 +0200 Subject: [PATCH 7/7] Fix build --- packages/server/src/api/controllers/row/views.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index 16d0f0205d..0a76c37dfc 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -63,7 +63,6 @@ export async function searchView( conditions: [query, body.query], }, } - } } await context.ensureSnippetContext(true)