Merge pull request #14588 from Budibase/fix/sql-param-limits

Fix Postgres "cannot pass more than 100 arguments" issue
This commit is contained in:
Michael Drury 2024-09-16 19:30:42 +01:00 committed by GitHub
commit bef0fc9ac4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 98 additions and 22 deletions

View File

@ -40,7 +40,6 @@ import { dataFilters, helpers } from "@budibase/shared-core"
import { cloneDeep } from "lodash"
type QueryFunction = (query: SqlQuery | SqlQuery[], operation: Operation) => any
const MAX_SQS_RELATIONSHIP_FIELDS = 63
function getBaseLimit() {
const envLimit = environment.SQL_MAX_ROWS
@ -56,6 +55,20 @@ function getRelationshipLimit() {
return envLimit || 500
}
function prioritisedArraySort(toSort: string[], priorities: string[]) {
return toSort.sort((a, b) => {
const aPriority = priorities.find(field => field && a.endsWith(field))
const bPriority = priorities.find(field => field && b.endsWith(field))
if (aPriority && !bPriority) {
return -1
}
if (!aPriority && bPriority) {
return 1
}
return a.localeCompare(b)
})
}
function getTableName(table?: Table): string | undefined {
// SQS uses the table ID rather than the table name
if (
@ -877,6 +890,22 @@ class InternalBuilder {
return `'${unaliased}'${separator}${tableField}`
}
maxFunctionParameters() {
// functions like say json_build_object() in SQL have a limit as to how many can be performed
// before a limit is met, this limit exists in Postgres/SQLite. This can be very important, such as
// for JSON column building as part of relationships. We also have a default limit to avoid very complex
// functions being built - it is likely this is not necessary or the best way to do it.
switch (this.client) {
case SqlClient.SQL_LITE:
return 127
case SqlClient.POSTGRES:
return 100
// other DBs don't have a limit, but set some sort of limit
default:
return 200
}
}
addJsonRelationships(
query: Knex.QueryBuilder,
fromTable: string,
@ -884,7 +913,7 @@ class InternalBuilder {
): Knex.QueryBuilder {
const sqlClient = this.client
const knex = this.knex
const { resource, tableAliases: aliases, endpoint } = this.query
const { resource, tableAliases: aliases, endpoint, meta } = this.query
const fields = resource?.fields || []
for (let relationship of relationships) {
const {
@ -899,21 +928,27 @@ class InternalBuilder {
if (!toTable || !fromTable) {
continue
}
const relatedTable = meta.tables?.[toTable]
const toAlias = aliases?.[toTable] || toTable,
fromAlias = aliases?.[fromTable] || fromTable
let toTableWithSchema = this.tableNameWithSchema(toTable, {
alias: toAlias,
schema: endpoint.schema,
})
let relationshipFields = fields.filter(
field => field.split(".")[0] === toAlias
const requiredFields = [
...(relatedTable?.primary || []),
relatedTable?.primaryDisplay,
].filter(field => field) as string[]
// sort the required fields to first in the list, so they don't get sliced out
let relationshipFields = prioritisedArraySort(
fields.filter(field => field.split(".")[0] === toAlias),
requiredFields
)
relationshipFields = relationshipFields.slice(
0,
Math.floor(this.maxFunctionParameters() / 2)
)
if (this.client === SqlClient.SQL_LITE) {
relationshipFields = relationshipFields.slice(
0,
MAX_SQS_RELATIONSHIP_FIELDS
)
}
const fieldList: string = relationshipFields
.map(field => this.buildJsonField(field))
.join(",")

View File

@ -12,6 +12,7 @@ import {
OneToManyRelationshipFieldMetadata,
Operation,
PaginationJson,
QueryJson,
RelationshipFieldMetadata,
Row,
SearchFilters,
@ -161,7 +162,6 @@ export class ExternalRequest<T extends Operation> {
private readonly tableId: string
private datasource?: Datasource
private tables: { [key: string]: Table } = {}
private tableList: Table[]
constructor(operation: T, tableId: string, datasource?: Datasource) {
this.operation = operation
@ -170,7 +170,6 @@ export class ExternalRequest<T extends Operation> {
if (datasource && datasource.entities) {
this.tables = datasource.entities
}
this.tableList = Object.values(this.tables)
}
private prepareFilters(
@ -301,7 +300,6 @@ export class ExternalRequest<T extends Operation> {
throw "No tables found, fetch tables before query."
}
this.tables = this.datasource.entities
this.tableList = Object.values(this.tables)
}
return { tables: this.tables, datasource: this.datasource }
}
@ -463,7 +461,7 @@ export class ExternalRequest<T extends Operation> {
breakExternalTableId(relatedTableId)
// @ts-ignore
const linkPrimaryKey = this.tables[relatedTableName].primary[0]
if (!lookupField || !row[lookupField]) {
if (!lookupField || !row?.[lookupField] == null) {
continue
}
const endpoint = getEndpoint(relatedTableId, Operation.READ)
@ -631,7 +629,8 @@ export class ExternalRequest<T extends Operation> {
const { datasource: ds } = await this.retrieveMetadata(datasourceId)
datasource = ds
}
const table = this.tables[tableName]
const tables = this.tables
const table = tables[tableName]
let isSql = isSQL(datasource)
if (!table) {
throw new Error(
@ -686,7 +685,7 @@ export class ExternalRequest<T extends Operation> {
) {
throw "Deletion must be filtered"
}
let json = {
let json: QueryJson = {
endpoint: {
datasourceId: datasourceId!,
entityId: tableName,
@ -715,7 +714,7 @@ export class ExternalRequest<T extends Operation> {
},
meta: {
table,
id: config.id,
tables: tables,
},
}

View File

@ -3080,4 +3080,46 @@ describe.each([
}).toHaveLength(4)
})
})
isSql &&
describe("max related columns", () => {
let relatedRows: Row[]
beforeAll(async () => {
const relatedSchema: TableSchema = {}
const row: Row = {}
for (let i = 0; i < 100; i++) {
const name = `column${i}`
relatedSchema[name] = { name, type: FieldType.NUMBER }
row[name] = i
}
const relatedTable = await createTable(relatedSchema)
table = await createTable({
name: { name: "name", type: FieldType.STRING },
related1: {
type: FieldType.LINK,
name: "related1",
fieldName: "main1",
tableId: relatedTable._id!,
relationshipType: RelationshipType.MANY_TO_MANY,
},
})
relatedRows = await Promise.all([
config.api.row.save(relatedTable._id!, row),
])
await config.api.row.save(table._id!, {
name: "foo",
related1: [relatedRows[0]._id],
})
})
it("retrieve the row with relationships", async () => {
await expectQuery({}).toContainExactly([
{
name: "foo",
related1: [{ _id: relatedRows[0]._id }],
},
])
})
})
})

View File

@ -162,7 +162,7 @@ describe("SQL query builder", () => {
const query = sql._query(generateRelationshipJson({ schema: "production" }))
expect(query).toEqual({
bindings: [limit, relationshipLimit],
sql: `with "paginated" as (select "brands".* from "production"."brands" order by "test"."id" asc limit $1) select "brands".*, (select json_agg(json_build_object('product_id',"products"."product_id",'product_name',"products"."product_name",'brand_id',"products"."brand_id")) from (select "products".* from "production"."products" as "products" where "products"."brand_id" = "brands"."brand_id" order by "products"."brand_id" asc limit $2) as "products") as "products" from "paginated" as "brands" order by "test"."id" asc`,
sql: `with "paginated" as (select "brands".* from "production"."brands" order by "test"."id" asc limit $1) select "brands".*, (select json_agg(json_build_object('brand_id',"products"."brand_id",'product_id',"products"."product_id",'product_name',"products"."product_name")) from (select "products".* from "production"."products" as "products" where "products"."brand_id" = "brands"."brand_id" order by "products"."brand_id" asc limit $2) as "products") as "products" from "paginated" as "brands" order by "test"."id" asc`,
})
})
@ -170,7 +170,7 @@ describe("SQL query builder", () => {
const query = sql._query(generateRelationshipJson())
expect(query).toEqual({
bindings: [limit, relationshipLimit],
sql: `with "paginated" as (select "brands".* from "brands" order by "test"."id" asc limit $1) select "brands".*, (select json_agg(json_build_object('product_id',"products"."product_id",'product_name',"products"."product_name",'brand_id',"products"."brand_id")) from (select "products".* from "products" as "products" where "products"."brand_id" = "brands"."brand_id" order by "products"."brand_id" asc limit $2) as "products") as "products" from "paginated" as "brands" order by "test"."id" asc`,
sql: `with "paginated" as (select "brands".* from "brands" order by "test"."id" asc limit $1) select "brands".*, (select json_agg(json_build_object('brand_id',"products"."brand_id",'product_id',"products"."product_id",'product_name',"products"."product_name")) from (select "products".* from "products" as "products" where "products"."brand_id" = "brands"."brand_id" order by "products"."brand_id" asc limit $2) as "products") as "products" from "paginated" as "brands" order by "test"."id" asc`,
})
})

View File

@ -63,7 +63,7 @@ describe("Captures of real examples", () => {
bindings: [primaryLimit, relationshipLimit, relationshipLimit],
sql: expect.stringContaining(
multiline(
`select json_agg(json_build_object('executorid',"b"."executorid",'taskname',"b"."taskname",'taskid',"b"."taskid",'completed',"b"."completed",'qaid',"b"."qaid",'executorid',"b"."executorid",'taskname',"b"."taskname",'taskid',"b"."taskid",'completed',"b"."completed",'qaid',"b"."qaid")`
`select json_agg(json_build_object('completed',"b"."completed",'completed',"b"."completed",'executorid',"b"."executorid",'executorid',"b"."executorid",'qaid',"b"."qaid",'qaid',"b"."qaid",'taskid',"b"."taskid",'taskid',"b"."taskid",'taskname',"b"."taskname",'taskname',"b"."taskname")`
)
),
})
@ -95,7 +95,7 @@ describe("Captures of real examples", () => {
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",'taskname',"b"."taskname",'taskid',"b"."taskid",'completed',"b"."completed",'qaid',"b"."qaid"))
select "a".*, (select json_agg(json_build_object('completed',"b"."completed",'executorid',"b"."executorid",'qaid',"b"."qaid",'taskid',"b"."taskid",'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`
)
@ -113,7 +113,7 @@ describe("Captures of real examples", () => {
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('productname',"b"."productname",'productid',"b"."productid"))
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`
),