Making progress on converting raw calls to use bindings.

This commit is contained in:
Sam Rose 2024-10-21 18:20:52 +01:00
parent 00bdd6fc00
commit 44bd00a0d7
No known key found for this signature in database
3 changed files with 90 additions and 112 deletions

View File

@ -258,30 +258,38 @@ class InternalBuilder {
const columnSchema = schema[column] const columnSchema = schema[column]
if (this.SPECIAL_SELECT_CASES.POSTGRES_MONEY(columnSchema)) { if (this.SPECIAL_SELECT_CASES.POSTGRES_MONEY(columnSchema)) {
return this.knex.raw( return this.knex.raw("??::money::numeric as ??", [
`${this.quotedIdentifier( [table, column].join("."),
[table, column].join(".") field,
)}::money::numeric as ${this.quote(field)}` ])
)
} }
if (this.SPECIAL_SELECT_CASES.MSSQL_DATES(columnSchema)) { if (this.SPECIAL_SELECT_CASES.MSSQL_DATES(columnSchema)) {
// Time gets returned as timestamp from mssql, not matching the expected // Time gets returned as timestamp from mssql, not matching the expected
// HH:mm format // HH:mm format
return this.knex.raw(`CONVERT(varchar, ${field}, 108) as "${field}"`)
// TODO: figure out how to express this safely without string
// interpolation.
return this.knex.raw(`CONVERT(varchar, ??, 108) as "${field}"`, [
field,
])
} }
const quoted = table if (table) {
? `${this.quote(table)}.${this.quote(column)}` return this.knex.raw("??", [`${table}.${column}`])
: this.quote(field) } else {
return this.knex.raw(quoted) return this.knex.raw("??", [field])
}
}) })
} }
// OracleDB can't use character-large-objects (CLOBs) in WHERE clauses, // OracleDB can't use character-large-objects (CLOBs) in WHERE clauses,
// so when we use them we need to wrap them in to_char(). This function // so when we use them we need to wrap them in to_char(). This function
// converts a field name to the appropriate identifier. // converts a field name to the appropriate identifier.
private convertClobs(field: string, opts?: { forSelect?: boolean }): string { private convertClobs(
field: string,
opts?: { forSelect?: boolean }
): Knex.Raw {
if (this.client !== SqlClient.ORACLE) { if (this.client !== SqlClient.ORACLE) {
throw new Error( throw new Error(
"you've called convertClobs on a DB that's not Oracle, this is a mistake" "you've called convertClobs on a DB that's not Oracle, this is a mistake"
@ -290,7 +298,7 @@ class InternalBuilder {
const parts = this.splitIdentifier(field) const parts = this.splitIdentifier(field)
const col = parts.pop()! const col = parts.pop()!
const schema = this.table.schema[col] const schema = this.table.schema[col]
let identifier = this.quotedIdentifier(field) let identifier = this.knex.raw("??", [field])
if ( if (
schema.type === FieldType.STRING || schema.type === FieldType.STRING ||
@ -301,9 +309,9 @@ class InternalBuilder {
schema.type === FieldType.BARCODEQR schema.type === FieldType.BARCODEQR
) { ) {
if (opts?.forSelect) { if (opts?.forSelect) {
identifier = `to_char(${identifier}) as ${this.quotedIdentifier(col)}` identifier = this.knex.raw("to_char(??) as ??", [identifier, col])
} else { } else {
identifier = `to_char(${identifier})` identifier = this.knex.raw("to_char(??)", [identifier])
} }
} }
return identifier return identifier
@ -485,9 +493,7 @@ class InternalBuilder {
.where( .where(
`${throughAlias}.${manyToMany.from}`, `${throughAlias}.${manyToMany.from}`,
"=", "=",
mainKnex.raw( mainKnex.raw(`??`, [`${fromAlias}.${manyToMany.fromPrimary}`])
this.quotedIdentifier(`${fromAlias}.${manyToMany.fromPrimary}`)
)
) )
// in SQS the same junction table is used for different many-to-many relationships between the // in SQS the same junction table is used for different many-to-many relationships between the
// two same tables, this is needed to avoid rows ending up in all columns // two same tables, this is needed to avoid rows ending up in all columns
@ -516,7 +522,7 @@ class InternalBuilder {
subQuery = subQuery.where( subQuery = subQuery.where(
toKey, toKey,
"=", "=",
mainKnex.raw(this.quotedIdentifier(foreignKey)) mainKnex.raw("??", [foreignKey])
) )
query = query.where(q => { query = query.where(q => {
@ -736,39 +742,29 @@ class InternalBuilder {
ArrayOperator.ONE_OF, ArrayOperator.ONE_OF,
(q, key: string, array) => { (q, key: string, array) => {
if (this.client === SqlClient.ORACLE) { if (this.client === SqlClient.ORACLE) {
// @ts-ignore
key = this.convertClobs(key) key = this.convertClobs(key)
array = Array.isArray(array) ? array : [array]
const binding = new Array(array.length).fill("?").join(",")
return q.whereRaw(`${key} IN (${binding})`, array)
} else {
return q[fnc](key, Array.isArray(array) ? array : [array])
} }
return q[fnc](key, Array.isArray(array) ? array : [array])
}, },
(q, key: string[], array) => { (q, key: string[], array) => {
if (this.client === SqlClient.ORACLE) { if (this.client === SqlClient.ORACLE) {
const keyStr = `(${key.map(k => this.convertClobs(k)).join(",")})` // @ts-ignore
const binding = `(${array key = key.map(k => this.convertClobs(k))
.map((a: any) => `(${new Array(a.length).fill("?").join(",")})`)
.join(",")})`
return q.whereRaw(`${keyStr} IN ${binding}`, array.flat())
} else {
return q[fnc](key, Array.isArray(array) ? array : [array])
} }
return q[fnc](key, Array.isArray(array) ? array : [array])
} }
) )
} }
if (filters.string) { if (filters.string) {
iterate(filters.string, BasicOperator.STRING, (q, key, value) => { iterate(filters.string, BasicOperator.STRING, (q, key, value) => {
const fnc = allOr ? "orWhere" : "where"
// postgres supports ilike, nothing else does // postgres supports ilike, nothing else does
if (this.client === SqlClient.POSTGRES) { if (this.client === SqlClient.POSTGRES) {
const fnc = allOr ? "orWhere" : "where"
return q[fnc](key, "ilike", `${value}%`) return q[fnc](key, "ilike", `${value}%`)
} else { } else {
const rawFnc = `${fnc}Raw` const fnc = allOr ? "orWhereRaw" : "whereRaw"
// @ts-ignore return q[fnc](`LOWER(??) LIKE ?`, [key, `${value.toLowerCase()}%`])
return q[rawFnc](`LOWER(${this.quotedIdentifier(key)}) LIKE ?`, [
`${value.toLowerCase()}%`,
])
} }
}) })
} }
@ -795,48 +791,33 @@ class InternalBuilder {
const schema = this.getFieldSchema(key) const schema = this.getFieldSchema(key)
let rawKey: string | Knex.Raw = key
let high = value.high
let low = value.low
if (this.client === SqlClient.ORACLE) { if (this.client === SqlClient.ORACLE) {
// @ts-ignore rawKey = this.convertClobs(key)
key = this.knex.raw(this.convertClobs(key)) } else if (
this.client === SqlClient.SQL_LITE &&
schema?.type === FieldType.BIGINT
) {
rawKey = this.knex.raw("CAST(?? AS INTEGER)", [key])
high = this.knex.raw("CAST(? AS INTEGER)", [value.high])
low = this.knex.raw("CAST(? AS INTEGER)", [value.low])
} }
if (lowValid && highValid) { if (lowValid && highValid) {
if ( const fnc = allOr ? "orWhereBetween" : "whereBetween"
schema?.type === FieldType.BIGINT && // @ts-ignore
this.client === SqlClient.SQL_LITE return q[fnc](rawKey, [low, high])
) {
return q.whereRaw(
`CAST(${key} AS INTEGER) BETWEEN CAST(? AS INTEGER) AND CAST(? AS INTEGER)`,
[value.low, value.high]
)
} else {
const fnc = allOr ? "orWhereBetween" : "whereBetween"
return q[fnc](key, [value.low, value.high])
}
} else if (lowValid) { } else if (lowValid) {
if ( const fnc = allOr ? "orWhere" : "where"
schema?.type === FieldType.BIGINT && // @ts-ignore
this.client === SqlClient.SQL_LITE return q[fnc](rawKey, ">=", low)
) {
return q.whereRaw(`CAST(${key} AS INTEGER) >= CAST(? AS INTEGER)`, [
value.low,
])
} else {
const fnc = allOr ? "orWhere" : "where"
return q[fnc](key, ">=", value.low)
}
} else if (highValid) { } else if (highValid) {
if ( const fnc = allOr ? "orWhere" : "where"
schema?.type === FieldType.BIGINT && // @ts-ignore
this.client === SqlClient.SQL_LITE return q[fnc](rawKey, "<=", high)
) {
return q.whereRaw(`CAST(${key} AS INTEGER) <= CAST(? AS INTEGER)`, [
value.high,
])
} else {
const fnc = allOr ? "orWhere" : "where"
return q[fnc](key, "<=", value.high)
}
} }
return q return q
}) })
@ -976,9 +957,7 @@ class InternalBuilder {
const selectFields = qualifiedFields.map(field => const selectFields = qualifiedFields.map(field =>
this.convertClobs(field, { forSelect: true }) this.convertClobs(field, { forSelect: true })
) )
query = query query = query.groupBy(groupByFields).select(selectFields)
.groupByRaw(groupByFields.join(", "))
.select(this.knex.raw(selectFields.join(", ")))
} else { } else {
query = query.groupBy(qualifiedFields).select(qualifiedFields) query = query.groupBy(qualifiedFields).select(qualifiedFields)
} }
@ -990,11 +969,10 @@ class InternalBuilder {
if (this.client === SqlClient.ORACLE) { if (this.client === SqlClient.ORACLE) {
const field = this.convertClobs(`${tableName}.${aggregation.field}`) const field = this.convertClobs(`${tableName}.${aggregation.field}`)
query = query.select( query = query.select(
this.knex.raw( this.knex.raw(`COUNT(DISTINCT ??) as ??`, [
`COUNT(DISTINCT ${field}) as ${this.quotedIdentifier( field,
aggregation.name aggregation.name,
)}` ])
)
) )
} else { } else {
query = query.countDistinct( query = query.countDistinct(

View File

@ -48,14 +48,14 @@ import { generateRowIdField } from "../../../integrations/utils"
import { cloneDeep } from "lodash/fp" import { cloneDeep } from "lodash/fp"
describe.each([ describe.each([
// ["in-memory", undefined], ["in-memory", undefined],
// ["lucene", undefined], ["lucene", undefined],
// ["sqs", undefined], ["sqs", undefined],
[DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)],
// [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
// [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
// [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
// [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)],
])("search (%s)", (name, dsProvider) => { ])("search (%s)", (name, dsProvider) => {
const isSqs = name === "sqs" const isSqs = name === "sqs"
const isLucene = name === "lucene" const isLucene = name === "lucene"
@ -155,24 +155,24 @@ describe.each([
describe.each([ describe.each([
["table", createTable], ["table", createTable],
[ // [
"view", // "view",
async (schema: TableSchema) => { // async (schema: TableSchema) => {
const tableId = await createTable(schema) // const tableId = await createTable(schema)
const viewId = await createView( // const viewId = await createView(
tableId, // tableId,
Object.keys(schema).reduce<ViewV2Schema>((viewSchema, fieldName) => { // Object.keys(schema).reduce<ViewV2Schema>((viewSchema, fieldName) => {
const field = schema[fieldName] // const field = schema[fieldName]
viewSchema[fieldName] = { // viewSchema[fieldName] = {
visible: field.visible ?? true, // visible: field.visible ?? true,
readonly: false, // readonly: false,
} // }
return viewSchema // return viewSchema
}, {}) // }, {})
) // )
return viewId // return viewId
}, // },
], // ],
])("from %s", (sourceType, createTableOrView) => { ])("from %s", (sourceType, createTableOrView) => {
const isView = sourceType === "view" const isView = sourceType === "view"
@ -792,7 +792,7 @@ describe.each([
}) })
}) })
describe.each([FieldType.STRING, FieldType.LONGFORM])("%s", () => { describe.only.each([FieldType.STRING, FieldType.LONGFORM])("%s", () => {
beforeAll(async () => { beforeAll(async () => {
tableOrViewId = await createTableOrView({ tableOrViewId = await createTableOrView({
name: { name: "name", type: FieldType.STRING }, name: { name: "name", type: FieldType.STRING },

View File

@ -35,12 +35,12 @@ import { quotas } from "@budibase/pro"
import { db, roles, features } from "@budibase/backend-core" import { db, roles, features } from "@budibase/backend-core"
describe.each([ describe.each([
["lucene", undefined], // ["lucene", undefined],
["sqs", undefined], // ["sqs", undefined],
[DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], // [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)],
[DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)],
[DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)],
[DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)],
[DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)],
])("/v2/views (%s)", (name, dsProvider) => { ])("/v2/views (%s)", (name, dsProvider) => {
const config = setup.getConfig() const config = setup.getConfig()