Merge pull request #2724 from Budibase/fix/mysql-issues

Fix MySQL issues
This commit is contained in:
Michael Drury 2021-09-24 13:18:23 +01:00 committed by GitHub
commit dfc023e896
7 changed files with 63 additions and 37 deletions

View File

@ -15,7 +15,7 @@ services:
- ./init.sql:/docker-entrypoint-initdb.d/init.sql - ./init.sql:/docker-entrypoint-initdb.d/init.sql
pgadmin: pgadmin:
container_name: pgadmin container_name: pgadmin-pg
image: dpage/pgadmin4 image: dpage/pgadmin4
restart: always restart: always
environment: environment:

View File

@ -544,6 +544,9 @@ module External {
extra: { extra: {
idFilter: buildFilters(id || generateIdForRow(row, table), {}, table), idFilter: buildFilters(id || generateIdForRow(row, table), {}, table),
}, },
meta: {
table,
}
} }
// can't really use response right now // can't really use response right now
const response = await makeExternalQuery(appId, json) const response = await makeExternalQuery(appId, json)

View File

@ -94,7 +94,8 @@ describe("/datasources", () => {
.expect(200) .expect(200)
// this is mock data, can't test it // this is mock data, can't test it
expect(res.body).toBeDefined() expect(res.body).toBeDefined()
expect(pg.queryMock).toHaveBeenCalledWith(`select "users"."name" as "users.name", "users"."age" as "users.age" from "users" where "users"."name" ilike $1 limit $2`, ["John%", 5000]) const expSql = `select "users"."name" as "users.name", "users"."age" as "users.age" from (select * from "users" where "users"."name" ilike $1 limit $2) as "users"`
expect(pg.queryMock).toHaveBeenCalledWith(expSql, ["John%", 5000])
}) })
}) })

View File

@ -1,3 +1,5 @@
import {Table} from "./common";
export enum Operation { export enum Operation {
CREATE = "CREATE", CREATE = "CREATE",
READ = "READ", READ = "READ",
@ -136,6 +138,9 @@ export interface QueryJson {
sort?: SortJson sort?: SortJson
paginate?: PaginationJson paginate?: PaginationJson
body?: object body?: object
meta?: {
table?: Table,
}
extra?: { extra?: {
idFilter?: SearchFilters idFilter?: SearchFilters
} }

View File

@ -1,7 +1,5 @@
import { Knex, knex } from "knex" import { Knex, knex } from "knex"
const BASE_LIMIT = 5000 const BASE_LIMIT = 5000
// if requesting a single row then need to up the limit for the sake of joins
const SINGLE_ROW_LIMIT = 100
import { import {
QueryJson, QueryJson,
SearchFilters, SearchFilters,
@ -146,46 +144,48 @@ function buildCreate(
function buildRead(knex: Knex, json: QueryJson, limit: number): KnexQuery { function buildRead(knex: Knex, json: QueryJson, limit: number): KnexQuery {
let { endpoint, resource, filters, sort, paginate, relationships } = json let { endpoint, resource, filters, sort, paginate, relationships } = json
const tableName = endpoint.entityId const tableName = endpoint.entityId
let query: KnexQuery = knex(tableName)
// select all if not specified // select all if not specified
if (!resource) { if (!resource) {
resource = { fields: [] } resource = { fields: [] }
} }
let selectStatement: string|string[] = "*"
// handle select // handle select
if (resource.fields && resource.fields.length > 0) { if (resource.fields && resource.fields.length > 0) {
// select the resources as the format "table.columnName" - this is what is provided // select the resources as the format "table.columnName" - this is what is provided
// by the resource builder further up // by the resource builder further up
query = query.select(resource.fields.map(field => `${field} as ${field}`)) selectStatement = resource.fields.map(field => `${field} as ${field}`)
} else { }
query = query.select("*") let foundLimit = limit || BASE_LIMIT
// handle pagination
let foundOffset: number | null = null
if (paginate && paginate.page && paginate.limit) {
// @ts-ignore
const page = paginate.page <= 1 ? 0 : paginate.page - 1
const offset = page * paginate.limit
foundLimit = paginate.limit
foundOffset = offset
} else if (paginate && paginate.limit) {
foundLimit = paginate.limit
}
// start building the query
let query: KnexQuery = knex(tableName).limit(foundLimit)
if (foundOffset) {
query = query.offset(foundOffset)
} }
// handle where
query = addFilters(tableName, query, filters)
// handle join
query = addRelationships(query, tableName, relationships)
// handle sorting
if (sort) { if (sort) {
for (let [key, value] of Object.entries(sort)) { for (let [key, value] of Object.entries(sort)) {
const direction = value === SortDirection.ASCENDING ? "asc" : "desc" const direction = value === SortDirection.ASCENDING ? "asc" : "desc"
query = query.orderBy(key, direction) query = query.orderBy(key, direction)
} }
} }
let foundLimit = limit || BASE_LIMIT query = addFilters(tableName, query, filters)
// handle pagination // @ts-ignore
if (paginate && paginate.page && paginate.limit) { let preQuery: KnexQuery = knex({
// @ts-ignore // @ts-ignore
const page = paginate.page <= 1 ? 0 : paginate.page - 1 [tableName]: query,
const offset = page * paginate.limit }).select(selectStatement)
foundLimit = paginate.limit // handle joins
query = query.offset(offset) return addRelationships(preQuery, tableName, relationships)
} else if (paginate && paginate.limit) {
foundLimit = paginate.limit
}
if (foundLimit === 1) {
foundLimit = SINGLE_ROW_LIMIT
}
query = query.limit(foundLimit)
return query
} }
function buildUpdate( function buildUpdate(

View File

@ -104,7 +104,7 @@ module MySQLModule {
client: any, client: any,
query: SqlQuery, query: SqlQuery,
connect: boolean = true connect: boolean = true
): Promise<any[]> { ): Promise<any[]|any> {
// Node MySQL is callback based, so we must wrap our call in a promise // Node MySQL is callback based, so we must wrap our call in a promise
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
if (connect) { if (connect) {
@ -238,6 +238,23 @@ module MySQLModule {
return internalQuery(this.client, input, false) return internalQuery(this.client, input, false)
} }
// when creating if an ID has been inserted need to make sure
// the id filter is enriched with it before trying to retrieve the row
checkLookupKeys(results: any, json: QueryJson) {
if (!results?.insertId || !json.meta?.table || !json.meta.table.primary) {
return json
}
const primaryKey = json.meta.table.primary?.[0]
json.extra = {
idFilter: {
equal: {
[primaryKey]: results.insertId
},
}
}
return json
}
async query(json: QueryJson) { async query(json: QueryJson) {
const operation = this._operation(json) const operation = this._operation(json)
this.client.connect() this.client.connect()
@ -250,7 +267,7 @@ module MySQLModule {
const results = await internalQuery(this.client, input, false) const results = await internalQuery(this.client, input, false)
// same as delete, manage returning // same as delete, manage returning
if (operation === Operation.CREATE || operation === Operation.UPDATE) { if (operation === Operation.CREATE || operation === Operation.UPDATE) {
row = this.getReturningRow(json) row = this.getReturningRow(this.checkLookupKeys(results, json))
} }
this.client.end() this.client.end()
if (operation !== Operation.READ) { if (operation !== Operation.READ) {

View File

@ -57,7 +57,7 @@ describe("SQL query builder", () => {
const query = sql._query(generateReadJson()) const query = sql._query(generateReadJson())
expect(query).toEqual({ expect(query).toEqual({
bindings: [limit], bindings: [limit],
sql: `select * from "${TABLE_NAME}" limit $1` sql: `select * from (select * from "${TABLE_NAME}" limit $1) as "${TABLE_NAME}"`
}) })
}) })
@ -68,7 +68,7 @@ describe("SQL query builder", () => {
})) }))
expect(query).toEqual({ expect(query).toEqual({
bindings: [limit], bindings: [limit],
sql: `select "${TABLE_NAME}"."name" as "${nameProp}", "${TABLE_NAME}"."age" as "${ageProp}" from "${TABLE_NAME}" limit $1` sql: `select "${TABLE_NAME}"."name" as "${nameProp}", "${TABLE_NAME}"."age" as "${ageProp}" from (select * from "${TABLE_NAME}" limit $1) as "${TABLE_NAME}"`
}) })
}) })
@ -82,7 +82,7 @@ describe("SQL query builder", () => {
})) }))
expect(query).toEqual({ expect(query).toEqual({
bindings: ["John%", limit], bindings: ["John%", limit],
sql: `select * from "${TABLE_NAME}" where "${TABLE_NAME}"."name" ilike $1 limit $2` sql: `select * from (select * from "${TABLE_NAME}" where "${TABLE_NAME}"."name" ilike $1 limit $2) as "${TABLE_NAME}"`
}) })
}) })
@ -99,7 +99,7 @@ describe("SQL query builder", () => {
})) }))
expect(query).toEqual({ expect(query).toEqual({
bindings: [2, 10, limit], bindings: [2, 10, limit],
sql: `select * from "${TABLE_NAME}" where "${TABLE_NAME}"."age" between $1 and $2 limit $3` sql: `select * from (select * from "${TABLE_NAME}" where "${TABLE_NAME}"."age" between $1 and $2 limit $3) as "${TABLE_NAME}"`
}) })
}) })
@ -115,7 +115,7 @@ describe("SQL query builder", () => {
})) }))
expect(query).toEqual({ expect(query).toEqual({
bindings: [10, "John", limit], bindings: [10, "John", limit],
sql: `select * from "${TABLE_NAME}" where ("${TABLE_NAME}"."age" = $1) or ("${TABLE_NAME}"."name" = $2) limit $3` sql: `select * from (select * from "${TABLE_NAME}" where ("${TABLE_NAME}"."age" = $1) or ("${TABLE_NAME}"."name" = $2) limit $3) as "${TABLE_NAME}"`
}) })
}) })
@ -160,7 +160,7 @@ describe("SQL query builder", () => {
const query = new Sql("mssql", 10)._query(generateReadJson()) const query = new Sql("mssql", 10)._query(generateReadJson())
expect(query).toEqual({ expect(query).toEqual({
bindings: [10], bindings: [10],
sql: `select top (@p0) * from [${TABLE_NAME}]` sql: `select * from (select top (@p0) * from [${TABLE_NAME}]) as [${TABLE_NAME}]`
}) })
}) })
@ -168,7 +168,7 @@ describe("SQL query builder", () => {
const query = new Sql("mysql", 10)._query(generateReadJson()) const query = new Sql("mysql", 10)._query(generateReadJson())
expect(query).toEqual({ expect(query).toEqual({
bindings: [10], bindings: [10],
sql: `select * from \`${TABLE_NAME}\` limit ?` sql: `select * from (select * from \`${TABLE_NAME}\` limit ?) as \`${TABLE_NAME}\``
}) })
}) })
}) })