Merge pull request #13187 from Budibase/fix/13177
Fix SQL aliasing issue when creating rows
This commit is contained in:
commit
6ba1930816
14 changed files with 162 additions and 62 deletions
|
@ -10,6 +10,11 @@ CREATE TABLE Persons (
|
||||||
City varchar(255),
|
City varchar(255),
|
||||||
PRIMARY KEY (PersonID)
|
PRIMARY KEY (PersonID)
|
||||||
);
|
);
|
||||||
|
CREATE TABLE Person (
|
||||||
|
PersonID int NOT NULL AUTO_INCREMENT,
|
||||||
|
Name varchar(255),
|
||||||
|
PRIMARY KEY (PersonID)
|
||||||
|
);
|
||||||
CREATE TABLE Tasks (
|
CREATE TABLE Tasks (
|
||||||
TaskID int NOT NULL AUTO_INCREMENT,
|
TaskID int NOT NULL AUTO_INCREMENT,
|
||||||
PersonID INT,
|
PersonID INT,
|
||||||
|
@ -27,6 +32,7 @@ CREATE TABLE Products (
|
||||||
);
|
);
|
||||||
INSERT INTO Persons (FirstName, LastName, Age, Address, City, CreatedAt) VALUES ('Mike', 'Hughes', 28.2, '123 Fake Street', 'Belfast', '2021-01-19 03:14:07');
|
INSERT INTO Persons (FirstName, LastName, Age, Address, City, CreatedAt) VALUES ('Mike', 'Hughes', 28.2, '123 Fake Street', 'Belfast', '2021-01-19 03:14:07');
|
||||||
INSERT INTO Persons (FirstName, LastName, Age, Address, City, CreatedAt) VALUES ('Dave', 'Johnson', 29, '124 Fake Street', 'Belfast', '2022-04-01 00:11:11');
|
INSERT INTO Persons (FirstName, LastName, Age, Address, City, CreatedAt) VALUES ('Dave', 'Johnson', 29, '124 Fake Street', 'Belfast', '2022-04-01 00:11:11');
|
||||||
|
INSERT INTO Person (Name) VALUES ('Elf');
|
||||||
INSERT INTO Tasks (PersonID, TaskName, CreatedAt) VALUES (1, 'assembling', '2020-01-01');
|
INSERT INTO Tasks (PersonID, TaskName, CreatedAt) VALUES (1, 'assembling', '2020-01-01');
|
||||||
INSERT INTO Tasks (PersonID, TaskName, CreatedAt) VALUES (2, 'processing', '2019-12-31');
|
INSERT INTO Tasks (PersonID, TaskName, CreatedAt) VALUES (2, 'processing', '2019-12-31');
|
||||||
INSERT INTO Products (name, updated) VALUES ('Meat', '11:00:22'), ('Fruit', '10:00:00');
|
INSERT INTO Products (name, updated) VALUES ('Meat', '11:00:22'), ('Fruit', '10:00:00');
|
||||||
|
|
|
@ -62,7 +62,11 @@ export default class AliasTables {
|
||||||
if (idx === -1 || idx > 1) {
|
if (idx === -1 || idx > 1) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
return Math.abs(tableName.length - name.length) <= 2
|
// this might look a bit mad, but the idea is if the field is wrapped, say in "", `` or []
|
||||||
|
// then the idx of the table name will be 1, and we should allow for it ending in a closing
|
||||||
|
// character - otherwise it should be the full length if the index is zero
|
||||||
|
const allowedCharacterDiff = idx * 2
|
||||||
|
return Math.abs(tableName.length - name.length) <= allowedCharacterDiff
|
||||||
})
|
})
|
||||||
if (foundTableName) {
|
if (foundTableName) {
|
||||||
const aliasedTableName = tableName.replace(
|
const aliasedTableName = tableName.replace(
|
||||||
|
@ -107,11 +111,10 @@ export default class AliasTables {
|
||||||
}
|
}
|
||||||
|
|
||||||
async queryWithAliasing(json: QueryJson): DatasourcePlusQueryResponse {
|
async queryWithAliasing(json: QueryJson): DatasourcePlusQueryResponse {
|
||||||
|
const fieldLength = json.resource?.fields?.length
|
||||||
|
const aliasingEnabled = fieldLength && fieldLength > 0
|
||||||
|
if (aliasingEnabled) {
|
||||||
json = cloneDeep(json)
|
json = cloneDeep(json)
|
||||||
const aliasTable = (table: Table) => ({
|
|
||||||
...table,
|
|
||||||
name: this.getAlias(table.name),
|
|
||||||
})
|
|
||||||
// run through the query json to update anywhere a table may be used
|
// run through the query json to update anywhere a table may be used
|
||||||
if (json.resource?.fields) {
|
if (json.resource?.fields) {
|
||||||
json.resource.fields = json.resource.fields.map(field =>
|
json.resource.fields = json.resource.fields.map(field =>
|
||||||
|
@ -130,6 +133,14 @@ export default class AliasTables {
|
||||||
json.filters[filterKey as keyof SearchFilters] = aliasedFilters
|
json.filters[filterKey as keyof SearchFilters] = aliasedFilters
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if (json.meta?.table) {
|
||||||
|
this.getAlias(json.meta.table.name)
|
||||||
|
}
|
||||||
|
if (json.meta?.tables) {
|
||||||
|
Object.keys(json.meta.tables).forEach(tableName =>
|
||||||
|
this.getAlias(tableName)
|
||||||
|
)
|
||||||
|
}
|
||||||
if (json.relationships) {
|
if (json.relationships) {
|
||||||
json.relationships = json.relationships.map(relationship => ({
|
json.relationships = json.relationships.map(relationship => ({
|
||||||
...relationship,
|
...relationship,
|
||||||
|
@ -140,24 +151,15 @@ export default class AliasTables {
|
||||||
]),
|
]),
|
||||||
}))
|
}))
|
||||||
}
|
}
|
||||||
if (json.meta?.table) {
|
|
||||||
json.meta.table = aliasTable(json.meta.table)
|
|
||||||
}
|
|
||||||
if (json.meta?.tables) {
|
|
||||||
const aliasedTables: Record<string, Table> = {}
|
|
||||||
for (let [tableName, table] of Object.entries(json.meta.tables)) {
|
|
||||||
aliasedTables[this.getAlias(tableName)] = aliasTable(table)
|
|
||||||
}
|
|
||||||
json.meta.tables = aliasedTables
|
|
||||||
}
|
|
||||||
// invert and return
|
// invert and return
|
||||||
const invertedTableAliases: Record<string, string> = {}
|
const invertedTableAliases: Record<string, string> = {}
|
||||||
for (let [key, value] of Object.entries(this.tableAliases)) {
|
for (let [key, value] of Object.entries(this.tableAliases)) {
|
||||||
invertedTableAliases[value] = key
|
invertedTableAliases[value] = key
|
||||||
}
|
}
|
||||||
json.tableAliases = invertedTableAliases
|
json.tableAliases = invertedTableAliases
|
||||||
|
}
|
||||||
const response = await getDatasourceAndQuery(json)
|
const response = await getDatasourceAndQuery(json)
|
||||||
if (Array.isArray(response)) {
|
if (Array.isArray(response) && aliasingEnabled) {
|
||||||
return this.reverse(response)
|
return this.reverse(response)
|
||||||
} else {
|
} else {
|
||||||
return response
|
return response
|
||||||
|
|
|
@ -12,6 +12,8 @@ import {
|
||||||
} from "@budibase/types"
|
} from "@budibase/types"
|
||||||
import environment from "../../environment"
|
import environment from "../../environment"
|
||||||
|
|
||||||
|
type QueryFunction = (query: Knex.SqlNative, operation: Operation) => any
|
||||||
|
|
||||||
const envLimit = environment.SQL_MAX_ROWS
|
const envLimit = environment.SQL_MAX_ROWS
|
||||||
? parseInt(environment.SQL_MAX_ROWS)
|
? parseInt(environment.SQL_MAX_ROWS)
|
||||||
: null
|
: null
|
||||||
|
@ -322,15 +324,18 @@ class InternalBuilder {
|
||||||
addSorting(query: Knex.QueryBuilder, json: QueryJson): Knex.QueryBuilder {
|
addSorting(query: Knex.QueryBuilder, json: QueryJson): Knex.QueryBuilder {
|
||||||
let { sort, paginate } = json
|
let { sort, paginate } = json
|
||||||
const table = json.meta?.table
|
const table = json.meta?.table
|
||||||
|
const aliases = json.tableAliases
|
||||||
|
const aliased =
|
||||||
|
table?.name && aliases?.[table.name] ? aliases[table.name] : table?.name
|
||||||
if (sort && Object.keys(sort || {}).length > 0) {
|
if (sort && Object.keys(sort || {}).length > 0) {
|
||||||
for (let [key, value] of Object.entries(sort)) {
|
for (let [key, value] of Object.entries(sort)) {
|
||||||
const direction =
|
const direction =
|
||||||
value.direction === SortDirection.ASCENDING ? "asc" : "desc"
|
value.direction === SortDirection.ASCENDING ? "asc" : "desc"
|
||||||
query = query.orderBy(`${table?.name}.${key}`, direction)
|
query = query.orderBy(`${aliased}.${key}`, direction)
|
||||||
}
|
}
|
||||||
} else if (this.client === SqlClient.MS_SQL && paginate?.limit) {
|
} else if (this.client === SqlClient.MS_SQL && paginate?.limit) {
|
||||||
// @ts-ignore
|
// @ts-ignore
|
||||||
query = query.orderBy(`${table?.name}.${table?.primary[0]}`)
|
query = query.orderBy(`${aliased}.${table?.primary[0]}`)
|
||||||
}
|
}
|
||||||
return query
|
return query
|
||||||
}
|
}
|
||||||
|
@ -605,7 +610,7 @@ class SqlQueryBuilder extends SqlTableQueryBuilder {
|
||||||
return query.toSQL().toNative()
|
return query.toSQL().toNative()
|
||||||
}
|
}
|
||||||
|
|
||||||
async getReturningRow(queryFn: Function, json: QueryJson) {
|
async getReturningRow(queryFn: QueryFunction, json: QueryJson) {
|
||||||
if (!json.extra || !json.extra.idFilter) {
|
if (!json.extra || !json.extra.idFilter) {
|
||||||
return {}
|
return {}
|
||||||
}
|
}
|
||||||
|
@ -617,7 +622,7 @@ class SqlQueryBuilder extends SqlTableQueryBuilder {
|
||||||
resource: {
|
resource: {
|
||||||
fields: [],
|
fields: [],
|
||||||
},
|
},
|
||||||
filters: json.extra.idFilter,
|
filters: json.extra?.idFilter,
|
||||||
paginate: {
|
paginate: {
|
||||||
limit: 1,
|
limit: 1,
|
||||||
},
|
},
|
||||||
|
@ -646,7 +651,7 @@ class SqlQueryBuilder extends SqlTableQueryBuilder {
|
||||||
// this function recreates the returning functionality of postgres
|
// this function recreates the returning functionality of postgres
|
||||||
async queryWithReturning(
|
async queryWithReturning(
|
||||||
json: QueryJson,
|
json: QueryJson,
|
||||||
queryFn: Function,
|
queryFn: QueryFunction,
|
||||||
processFn: Function = (result: any) => result
|
processFn: Function = (result: any) => result
|
||||||
) {
|
) {
|
||||||
const sqlClient = this.getSqlClient()
|
const sqlClient = this.getSqlClient()
|
||||||
|
|
|
@ -4,6 +4,7 @@ import Sql from "../base/sql"
|
||||||
import { SqlClient } from "../utils"
|
import { SqlClient } from "../utils"
|
||||||
import AliasTables from "../../api/controllers/row/alias"
|
import AliasTables from "../../api/controllers/row/alias"
|
||||||
import { generator } from "@budibase/backend-core/tests"
|
import { generator } from "@budibase/backend-core/tests"
|
||||||
|
import { Knex } from "knex"
|
||||||
|
|
||||||
function multiline(sql: string) {
|
function multiline(sql: string) {
|
||||||
return sql.replace(/\n/g, "").replace(/ +/g, " ")
|
return sql.replace(/\n/g, "").replace(/ +/g, " ")
|
||||||
|
@ -160,6 +161,28 @@ describe("Captures of real examples", () => {
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("returning (everything bar Postgres)", () => {
|
||||||
|
it("should be able to handle row returning", () => {
|
||||||
|
const queryJson = getJson("createSimple.json")
|
||||||
|
const SQL = new Sql(SqlClient.MS_SQL, limit)
|
||||||
|
let query = SQL._query(queryJson, { disableReturning: true })
|
||||||
|
expect(query).toEqual({
|
||||||
|
sql: "insert into [people] ([age], [name]) values (@p0, @p1)",
|
||||||
|
bindings: [22, "Test"],
|
||||||
|
})
|
||||||
|
|
||||||
|
// now check returning
|
||||||
|
let returningQuery: Knex.SqlNative = { sql: "", bindings: [] }
|
||||||
|
SQL.getReturningRow((input: Knex.SqlNative) => {
|
||||||
|
returningQuery = input
|
||||||
|
}, queryJson)
|
||||||
|
expect(returningQuery).toEqual({
|
||||||
|
sql: "select * from (select top (@p0) * from [people] where [people].[name] = @p1 and [people].[age] = @p2 order by [people].[name] asc) as [people]",
|
||||||
|
bindings: [1, "Test", 22],
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
describe("check max character aliasing", () => {
|
describe("check max character aliasing", () => {
|
||||||
it("should handle over 'z' max character alias", () => {
|
it("should handle over 'z' max character alias", () => {
|
||||||
const tableNames = []
|
const tableNames = []
|
||||||
|
|
|
@ -68,7 +68,7 @@
|
||||||
"primary": [
|
"primary": [
|
||||||
"personid"
|
"personid"
|
||||||
],
|
],
|
||||||
"name": "a",
|
"name": "persons",
|
||||||
"schema": {
|
"schema": {
|
||||||
"year": {
|
"year": {
|
||||||
"type": "number",
|
"type": "number",
|
||||||
|
|
|
@ -0,0 +1,64 @@
|
||||||
|
{
|
||||||
|
"endpoint": {
|
||||||
|
"datasourceId": "datasource_plus_0ed5835e5552496285df546030f7c4ae",
|
||||||
|
"entityId": "people",
|
||||||
|
"operation": "CREATE"
|
||||||
|
},
|
||||||
|
"resource": {
|
||||||
|
"fields": [
|
||||||
|
"a.name",
|
||||||
|
"a.age"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"filters": {},
|
||||||
|
"relationships": [],
|
||||||
|
"body": {
|
||||||
|
"name": "Test",
|
||||||
|
"age": 22
|
||||||
|
},
|
||||||
|
"extra": {
|
||||||
|
"idFilter": {
|
||||||
|
"equal": {
|
||||||
|
"name": "Test",
|
||||||
|
"age": 22
|
||||||
|
}
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"meta": {
|
||||||
|
"table": {
|
||||||
|
"_id": "datasource_plus_0ed5835e5552496285df546030f7c4ae__people",
|
||||||
|
"type": "table",
|
||||||
|
"sourceId": "datasource_plus_0ed5835e5552496285df546030f7c4ae",
|
||||||
|
"sourceType": "external",
|
||||||
|
"primary": [
|
||||||
|
"name",
|
||||||
|
"age"
|
||||||
|
],
|
||||||
|
"name": "people",
|
||||||
|
"schema": {
|
||||||
|
"name": {
|
||||||
|
"type": "string",
|
||||||
|
"externalType": "varchar",
|
||||||
|
"autocolumn": false,
|
||||||
|
"name": "name",
|
||||||
|
"constraints": {
|
||||||
|
"presence": true
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"age": {
|
||||||
|
"type": "number",
|
||||||
|
"externalType": "int",
|
||||||
|
"autocolumn": false,
|
||||||
|
"name": "age",
|
||||||
|
"constraints": {
|
||||||
|
"presence": false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"primaryDisplay": "name"
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"tableAliases": {
|
||||||
|
"people": "a"
|
||||||
|
}
|
||||||
|
}
|
|
@ -58,7 +58,7 @@
|
||||||
"primary": [
|
"primary": [
|
||||||
"personid"
|
"personid"
|
||||||
],
|
],
|
||||||
"name": "a",
|
"name": "persons",
|
||||||
"schema": {
|
"schema": {
|
||||||
"year": {
|
"year": {
|
||||||
"type": "number",
|
"type": "number",
|
||||||
|
|
|
@ -34,7 +34,7 @@
|
||||||
"keypartone",
|
"keypartone",
|
||||||
"keyparttwo"
|
"keyparttwo"
|
||||||
],
|
],
|
||||||
"name": "a",
|
"name": "compositetable",
|
||||||
"schema": {
|
"schema": {
|
||||||
"keyparttwo": {
|
"keyparttwo": {
|
||||||
"type": "string",
|
"type": "string",
|
||||||
|
|
|
@ -49,7 +49,7 @@
|
||||||
"primary": [
|
"primary": [
|
||||||
"taskid"
|
"taskid"
|
||||||
],
|
],
|
||||||
"name": "a",
|
"name": "tasks",
|
||||||
"schema": {
|
"schema": {
|
||||||
"executorid": {
|
"executorid": {
|
||||||
"type": "number",
|
"type": "number",
|
||||||
|
|
|
@ -63,7 +63,7 @@
|
||||||
"primary": [
|
"primary": [
|
||||||
"productid"
|
"productid"
|
||||||
],
|
],
|
||||||
"name": "a",
|
"name": "products",
|
||||||
"schema": {
|
"schema": {
|
||||||
"productname": {
|
"productname": {
|
||||||
"type": "string",
|
"type": "string",
|
||||||
|
|
|
@ -53,7 +53,7 @@
|
||||||
"primary": [
|
"primary": [
|
||||||
"productid"
|
"productid"
|
||||||
],
|
],
|
||||||
"name": "a",
|
"name": "products",
|
||||||
"schema": {
|
"schema": {
|
||||||
"productname": {
|
"productname": {
|
||||||
"type": "string",
|
"type": "string",
|
||||||
|
|
|
@ -109,7 +109,7 @@
|
||||||
"primary": [
|
"primary": [
|
||||||
"taskid"
|
"taskid"
|
||||||
],
|
],
|
||||||
"name": "a",
|
"name": "tasks",
|
||||||
"schema": {
|
"schema": {
|
||||||
"executorid": {
|
"executorid": {
|
||||||
"type": "number",
|
"type": "number",
|
||||||
|
|
|
@ -66,7 +66,7 @@
|
||||||
"primary": [
|
"primary": [
|
||||||
"personid"
|
"personid"
|
||||||
],
|
],
|
||||||
"name": "a",
|
"name": "persons",
|
||||||
"schema": {
|
"schema": {
|
||||||
"year": {
|
"year": {
|
||||||
"type": "number",
|
"type": "number",
|
||||||
|
|
|
@ -66,7 +66,7 @@
|
||||||
"primary": [
|
"primary": [
|
||||||
"personid"
|
"personid"
|
||||||
],
|
],
|
||||||
"name": "a",
|
"name": "persons",
|
||||||
"schema": {
|
"schema": {
|
||||||
"year": {
|
"year": {
|
||||||
"type": "number",
|
"type": "number",
|
||||||
|
|
Loading…
Reference in a new issue