From aea9cda8f5408fe4eab068d89c2b9d1b50cdf761 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 17 Jul 2024 15:45:35 +0100 Subject: [PATCH] wip --- packages/backend-core/src/sql/sql.ts | 101 ++++++++++-------- packages/backend-core/src/sql/sqlTable.ts | 6 +- .../src/api/routes/tests/search.spec.ts | 35 ++++-- 3 files changed, 90 insertions(+), 52 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 4936e4da68..161c2a7488 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -42,27 +42,28 @@ const envLimit = environment.SQL_MAX_ROWS : null const BASE_LIMIT = envLimit || 5000 -function likeKey(client: string | string[], key: string): string { - let start: string, end: string +// Takes a string like foo and returns a quoted string like [foo] for SQL Server +// and "foo" for Postgres. +function quote(client: SqlClient, str: string) { switch (client) { - case SqlClient.MY_SQL: - start = end = "`" - break case SqlClient.SQL_LITE: case SqlClient.ORACLE: case SqlClient.POSTGRES: - start = end = '"' - break + return `"${str}"` case SqlClient.MS_SQL: - start = "[" - end = "]" - break + return `[${str}]` default: - throw new Error("Unknown client generating like key") + return `\`${str}\`` } - const parts = key.split(".") - key = parts.map(part => `${start}${part}${end}`).join(".") +} + +// Takes a string like a.b.c and returns a quoted identifier like [a].[b].[c] +// for SQL Server and `a`.`b`.`c` for MySQL. +function quotedIdentifier(client: SqlClient, key: string): string { return key + .split(".") + .map(part => quote(client, part)) + .join(".") } function parse(input: any) { @@ -113,34 +114,37 @@ function generateSelectStatement( knex: Knex ): (string | Knex.Raw)[] | "*" { const { resource, meta } = json + const client = knex.client.config.client as SqlClient if (!resource || !resource.fields || resource.fields.length === 0) { return "*" } - const schema = meta?.table?.schema + const schema = meta.table.schema return resource.fields.map(field => { - const fieldNames = field.split(/\./g) - const tableName = fieldNames[0] - const columnName = fieldNames[1] - const columnSchema = schema?.[columnName] - if (columnSchema && knex.client.config.client === SqlClient.POSTGRES) { - const externalType = schema[columnName].externalType - if (externalType?.includes("money")) { - return knex.raw( - `"${tableName}"."${columnName}"::money::numeric as "${field}"` - ) - } + const [table, column, ..._rest] = field.split(/\./g) + if ( + client === SqlClient.POSTGRES && + schema[column].externalType?.includes("money") + ) { + return knex.raw(`"${table}"."${column}"::money::numeric as "${field}"`) } if ( - knex.client.config.client === SqlClient.MS_SQL && - columnSchema?.type === FieldType.DATETIME && - columnSchema.timeOnly + client === SqlClient.MS_SQL && + schema[column]?.type === FieldType.DATETIME && + schema[column].timeOnly ) { - // Time gets returned as timestamp from mssql, not matching the expected HH:mm format + // Time gets returned as timestamp from mssql, not matching the expected + // HH:mm format return knex.raw(`CONVERT(varchar, ${field}, 108) as "${field}"`) } return `${field} as ${field}` + // return knex.raw( + // `${quote(client, table)}.${quote(client, column)} as ${quote( + // client, + // field + // )}` + // ) }) } @@ -173,9 +177,9 @@ function convertBooleans(query: SqlQuery | SqlQuery[]): SqlQuery | SqlQuery[] { } class InternalBuilder { - private readonly client: string + private readonly client: SqlClient - constructor(client: string) { + constructor(client: SqlClient) { this.client = client } @@ -250,9 +254,10 @@ class InternalBuilder { } else { const rawFnc = `${fnc}Raw` // @ts-ignore - query = query[rawFnc](`LOWER(${likeKey(this.client, key)}) LIKE ?`, [ - `%${value.toLowerCase()}%`, - ]) + query = query[rawFnc]( + `LOWER(${quotedIdentifier(this.client, key)}) LIKE ?`, + [`%${value.toLowerCase()}%`] + ) } } @@ -302,7 +307,10 @@ class InternalBuilder { } statement += (statement ? andOr : "") + - `COALESCE(LOWER(${likeKey(this.client, key)}), '') LIKE ?` + `COALESCE(LOWER(${quotedIdentifier( + this.client, + key + )}), '') LIKE ?` } if (statement === "") { @@ -336,9 +344,10 @@ class InternalBuilder { } else { const rawFnc = `${fnc}Raw` // @ts-ignore - query = query[rawFnc](`LOWER(${likeKey(this.client, key)}) LIKE ?`, [ - `${value.toLowerCase()}%`, - ]) + query = query[rawFnc]( + `LOWER(${quotedIdentifier(this.client, key)}) LIKE ?`, + [`${value.toLowerCase()}%`] + ) } }) } @@ -376,12 +385,15 @@ class InternalBuilder { const fnc = allOr ? "orWhereRaw" : "whereRaw" if (this.client === SqlClient.MS_SQL) { query = query[fnc]( - `CASE WHEN ${likeKey(this.client, key)} = ? THEN 1 ELSE 0 END = 1`, + `CASE WHEN ${quotedIdentifier( + this.client, + key + )} = ? THEN 1 ELSE 0 END = 1`, [value] ) } else { query = query[fnc]( - `COALESCE(${likeKey(this.client, key)} = ?, FALSE)`, + `COALESCE(${quotedIdentifier(this.client, key)} = ?, FALSE)`, [value] ) } @@ -392,12 +404,15 @@ class InternalBuilder { const fnc = allOr ? "orWhereRaw" : "whereRaw" if (this.client === SqlClient.MS_SQL) { query = query[fnc]( - `CASE WHEN ${likeKey(this.client, key)} = ? THEN 1 ELSE 0 END = 0`, + `CASE WHEN ${quotedIdentifier( + this.client, + key + )} = ? THEN 1 ELSE 0 END = 0`, [value] ) } else { query = query[fnc]( - `COALESCE(${likeKey(this.client, key)} != ?, TRUE)`, + `COALESCE(${quotedIdentifier(this.client, key)} != ?, TRUE)`, [value] ) } @@ -769,7 +784,7 @@ class SqlQueryBuilder extends SqlTableQueryBuilder { private readonly limit: number // pass through client to get flavour of SQL - constructor(client: string, limit: number = BASE_LIMIT) { + constructor(client: SqlClient, limit: number = BASE_LIMIT) { super(client) this.limit = limit } diff --git a/packages/backend-core/src/sql/sqlTable.ts b/packages/backend-core/src/sql/sqlTable.ts index bdc8a3dd69..02acc8af85 100644 --- a/packages/backend-core/src/sql/sqlTable.ts +++ b/packages/backend-core/src/sql/sqlTable.ts @@ -195,14 +195,14 @@ function buildDeleteTable(knex: SchemaBuilder, table: Table): SchemaBuilder { } class SqlTableQueryBuilder { - private readonly sqlClient: string + private readonly sqlClient: SqlClient // pass through client to get flavour of SQL - constructor(client: string) { + constructor(client: SqlClient) { this.sqlClient = client } - getSqlClient(): string { + getSqlClient(): SqlClient { return this.sqlClient } diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index ae35c4c5eb..24197462ee 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -38,13 +38,13 @@ import { structures } from "@budibase/backend-core/tests" import { DEFAULT_EMPLOYEE_TABLE_SCHEMA } from "../../../db/defaultData/datasource_bb_default" describe.each([ - ["in-memory", undefined], - ["lucene", undefined], + // ["in-memory", undefined], + // ["lucene", undefined], ["sqs", undefined], - [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], - [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], - [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], - [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], + // [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], + // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], + // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], + // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], ])("search (%s)", (name, dsProvider) => { const isSqs = name === "sqs" const isLucene = name === "lucene" @@ -735,6 +735,29 @@ describe.each([ query: {}, }).toHaveLength(1) }) + + isInternal && + describe("space at end of column name", () => { + beforeAll(async () => { + table = await createTable({ + "name ": { + name: "name ", + type: FieldType.STRING, + }, + }) + await createRows([{ ["name "]: "foo" }, { ["name "]: "bar" }]) + }) + + it("should be able to query a column that starts with a space", async () => { + await expectSearch({ + query: { + string: { + "1:name ": "foo", + }, + }, + }).toContainExactly([{ ["name "]: "foo" }]) + }) + }) }) describe("equal", () => {