Skip to content

Commit

Permalink
fix: use full path for table lookups (typeorm#8097)
Browse files Browse the repository at this point in the history 
The changes made as part of typeorm#7575 fixed some issues and introduced others. The problem became that the table name would be compared against the EntityMetadata tablePath. This would fail in some cases because while the EntityMetadata would specify database, it'd be the "default"/current database & would be omitted from the table name.

To work around that this PR introduces the database & schema on the table objects as well as a fully-qualified table path property which will always include all of them for comparing against EntityMetadata.
  • Loading branch information
@imnotjames @nebkat
imnotjames authored and nebkat committed Sep 27, 2021
1 parent 50b9230 commit 27e02da
Show file tree
Hide file tree
Showing 18 changed files with 753 additions and 275 deletions.
25 changes: 15 additions & 10 deletions src/driver/Driver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import {Expression} from "../expression-builder/Expression";
import {ColumnMode} from "../metadata-args/types/ColumnMode";
import {ColumnOptions} from "../decorator/options/ColumnOptions";
import {DriverConnectionOptions} from "../data-source/DriverConnectionOptions";
import {Table} from "../schema-builder/table/Table";
import {View} from "../schema-builder/view/View";
import {TableForeignKey} from "../schema-builder/table/TableForeignKey";

/**
* Driver organizes TypeORM communication with specific database management system.
Expand All @@ -31,12 +34,17 @@ export interface Driver {
options: DriverConnectionOptions;

/**
* Master database used to perform all write queries.
* Database name used to perform all write queries.
*
* todo: probably move into query runner.
*/
database?: string;

/**
* Schema name used to perform all write queries.
*/
schema?: string;

/**
* Indicates if replication is enabled.
*/
Expand Down Expand Up @@ -115,12 +123,6 @@ export interface Driver {
*/
createRepository?(): Repository<any>;

/**
* Replaces parameters in the given sql with special escaping character
* and an array of parameter names to be passed to a query.
*/
escapeQueryWithParameters(sql: string, parameters: ObjectLiteral, nativeParameters: ObjectLiteral): [string, any[]];

/**
* Escapes a table name, column name or an alias.
*
Expand All @@ -130,12 +132,15 @@ export interface Driver {

/**
* Build full table path with database name, schema name and table name.
* E.g. "myDB"."mySchema"."myTable"
*
* TODO: Rename to buildTablePath
* E.g. myDB.mySchema.myTable
*/
buildTableName(tableName: string, schema?: string, database?: string): string;

/**
* Parse a target table name or other types and return a normalized table definition.
*/
parseTableName(target: EntityMetadata | Table | View | TableForeignKey | string): { tableName: string, schema?: string, database?: string };

/**
* Build full schema path with database name and schema name.
* E.g. "myDB"."mySchema"
Expand Down
95 changes: 65 additions & 30 deletions src/driver/mysql/MysqlDriver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ import {Expression, ExpressionBuilder} from "../../expression-builder/Expression
import {Fn} from "../../expression-builder/expression/Function";
import {CurrentTimestamp} from "../../expression-builder/expression/datetime/CurrentTimestamp";
import {DefaultExpressionBuildInterface} from "../../expression-builder/DefaultExpressionBuildInterface";
import {Table} from "../../schema-builder/table/Table";
import {View} from "../../schema-builder/view/View";
import {TableForeignKey} from "../../schema-builder/table/TableForeignKey";

/**
* Organizes communication with MySQL DBMS.
Expand Down Expand Up @@ -101,9 +104,15 @@ export class MysqlDriver implements Driver {

/**
* Master database used to perform all write queries.
* Database name used to perform all write queries.
*/
database?: string;

/**
* Schema name used to performn all write queries.
*/
schema?: string;

/**
* Indicates if replication is enabled.
*/
Expand Down Expand Up @@ -349,7 +358,7 @@ export class MysqlDriver implements Driver {
// load mysql package
this.loadDependencies();

this.database = this.options.replication ? this.options.replication.master.database : this.options.database;
this.database = DriverUtils.buildDriverOptions(this.options.replication ? this.options.replication.master : this.options).database;

// validate options to make sure everything is set
// todo: revisit validation with replication in mind
Expand Down Expand Up @@ -382,6 +391,14 @@ export class MysqlDriver implements Driver {
} else {
this.pool = await this.createPool(this.createConnectionOptions(this.options, this.options));
}

if (this.database === undefined) {
const queryRunner = await this.createQueryRunner("master");

this.database = await queryRunner.getCurrentDatabase();

await queryRunner.release();
}
}

/**
Expand Down Expand Up @@ -420,35 +437,6 @@ export class MysqlDriver implements Driver {
return new MysqlQueryRunner(this, mode);
}

/**
* Replaces parameters in the given sql with special escaping character
* and an array of parameter names to be passed to a query.
*/
escapeQueryWithParameters(sql: string, parameters: ObjectLiteral, nativeParameters: ObjectLiteral): [string, any[]] {
const escapedParameters: any[] = Object.keys(nativeParameters).map(key => nativeParameters[key]);
if (!parameters || !Object.keys(parameters).length)
return [sql, escapedParameters];

const keys = Object.keys(parameters).map(parameter => "(:(\\.\\.\\.)?" + parameter + "\\b)").join("|");
sql = sql.replace(new RegExp(keys, "g"), (key: string) => {
let value: any;
if (key.substr(0, 4) === ":...") {
value = parameters[key.substr(4)];
} else {
value = parameters[key.substr(1)];
}

if (value instanceof Function) {
return value();

} else {
escapedParameters.push(value);
return "?";
}
}); // todo: make replace only in value statements, otherwise problems
return [sql, escapedParameters];
}

/**
* Escapes a column name.
*/
Expand All @@ -464,6 +452,53 @@ export class MysqlDriver implements Driver {
return database ? `${database}.${tableName}` : tableName;
}

/**
* Parse a target table name or other types and return a normalized table definition.
*/
parseTableName(target: EntityMetadata | Table | View | TableForeignKey | string): { database?: string, schema?: string, tableName: string } {
const driverDatabase = this.database;
const driverSchema = undefined;

if (target instanceof Table || target instanceof View) {
const parsed = this.parseTableName(target.name);

return {
database: target.database || parsed.database || driverDatabase,
schema: target.schema || parsed.schema || driverSchema,
tableName: parsed.tableName
};
}

if (target instanceof TableForeignKey) {
const parsed = this.parseTableName(target.referencedTableName);

return {
database: target.referencedDatabase || parsed.database || driverDatabase,
schema: target.referencedSchema || parsed.schema || driverSchema,
tableName: parsed.tableName
};
}

if (target instanceof EntityMetadata) {
// EntityMetadata tableName is never a path

return {
database: target.database || driverDatabase,
schema: target.schema || driverSchema,
tableName: target.tableName
};

}

const parts = target.split(".");

return {
database: (parts.length > 1 ? parts[0] : undefined) || driverDatabase,
schema: driverSchema,
tableName: parts.length > 1 ? parts[1] : parts[0]
};
}

/**
* Wraps given value in any additional expressions required based on its column type and metadata.
*/
Expand Down
35 changes: 24 additions & 11 deletions src/driver/mysql/MysqlQueryRunner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,29 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner {
return result.length > 0;
}

/**
* Loads currently using database
*/
async getCurrentDatabase(): Promise<string> {
const query = await this.query(`SELECT DATABASE() AS \`db_name\``);
return query[0]["db_name"];
}

/**
* Checks if schema with the given name exist.
*/
async hasSchema(schema: string): Promise<boolean> {
throw new Error(`MySql driver does not support table schemas`);
}

/**
* Loads currently using database schema
*/
async getCurrentSchema(): Promise<string> {
const query = await this.query(`SELECT SCHEMA() AS \`schema_name\``);
return query[0]["schema_name"];
}

/**
* Checks if table with the given name exist in the database.
*/
Expand Down Expand Up @@ -1160,14 +1176,6 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner {
// Protected Methods
// -------------------------------------------------------------------------

/**
* Returns current database.
*/
protected async getCurrentDatabase(): Promise<string> {
const currentDBQuery = await this.query(`SELECT DATABASE() AS \`db_name\``);
return currentDBQuery[0]["db_name"];
}

protected async loadViews(viewNames: string[]): Promise<View[]> {
const hasTable = await this.hasTable(this.getTypeormMetadataTableName());
if (!hasTable) return [];
Expand Down Expand Up @@ -1808,9 +1816,14 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner {
/**
* Escapes given table or view path.
*/
protected escapePath(target: Table|View|string, disableEscape?: boolean): string {
const tableName = target instanceof Table || target instanceof View ? target.name : target;
return tableName.split(".").map(i => disableEscape ? i : `\`${i}\``).join(".");
protected escapePath(target: Table|View|string): string {
const { database, tableName } = this.driver.parseTableName(target);

if (database && database !== this.driver.database) {
return `\`${database}\`.\`${tableName}\``;
}

return `\`${tableName}\``;
}

/**
Expand Down

0 comments on commit 27e02da

Please sign in to comment.