Skip to content

Commit f7e8fa1

Browse files
committed
fix(schema-compiler): Escape quotes in column names
1 parent 5fd13d1 commit f7e8fa1

File tree

7 files changed

+50
-9
lines changed

7 files changed

+50
-9
lines changed

packages/cubejs-schema-compiler/src/adapter/BaseQuery.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -466,12 +466,16 @@ export class BaseQuery {
466466
}
467467

468468
/**
469-
* Wrap specified column/table name with the double quote.
469+
* Wrap specified column/table name with the double quote and escape quotes inside.
470470
* @param {string} name
471471
* @returns {string}
472472
*/
473473
escapeColumnName(name) {
474-
return `"${name}"`;
474+
// Identifier is wrapped with double quotes
475+
// https://ronsavage.github.io/SQL/sql-2003-2.bnf.html#delimited%20identifier
476+
// Double quote inside is represented by double double quote
477+
// https://ronsavage.github.io/SQL/sql-2003-2.bnf.html#doublequote%20symbol
478+
return `"${name.replaceAll('"', '""')}"`;
475479
}
476480

477481
/**

packages/cubejs-schema-compiler/src/adapter/BigqueryQuery.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,12 @@ export class BigqueryQuery extends BaseQuery {
5454
}
5555

5656
public escapeColumnName(name) {
57-
return `\`${name}\``;
57+
// Quoted identifiers must be enclosed by `
58+
// Identifiers have the same escape sequences as string literals
59+
// https://cloud.google.com/bigquery/docs/reference/standard-sql/lexical#quoted_identifiers
60+
// \` is escape sequence for `
61+
// https://cloud.google.com/bigquery/docs/reference/standard-sql/lexical#escape_sequences
62+
return `\`${name.replaceAll('`', '\\`')}\``;
5863
}
5964

6065
public timeGroupedColumn(granularity, dimension) {

packages/cubejs-schema-compiler/src/adapter/ClickHouseQuery.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,13 @@ export class ClickHouseQuery extends BaseQuery {
3636
}
3737

3838
public escapeColumnName(name) {
39-
return `\`${name}\``;
39+
// ClickHouse does not document it's exact rules regarding identifiers
40+
// https://clickhouse.com/docs/en/sql-reference/syntax#identifiers
41+
// But there's a bit about escaping in string
42+
// https://clickhouse.com/docs/en/sql-reference/syntax#string
43+
// Note that documentation does not allow to use \` sequence
44+
// See https://github.com/ClickHouse/ClickHouse/issues/71310
45+
return `\`${name.replaceAll('`', '\\x60')}\``;
4046
}
4147

4248
public convertTz(field) {
@@ -274,7 +280,8 @@ export class ClickHouseQuery extends BaseQuery {
274280
templates.expressions.timestamp_literal = 'parseDateTimeBestEffort(\'{{ value }}\')';
275281
delete templates.expressions.like_escape;
276282
templates.quotes.identifiers = '`';
277-
templates.quotes.escape = '\\`';
283+
// See escapeColumnName comment
284+
templates.quotes.escape = '\\x60';
278285
templates.types.boolean = 'BOOL';
279286
templates.types.timestamp = 'DATETIME';
280287
delete templates.types.time;

packages/cubejs-schema-compiler/src/adapter/CubeStoreQuery.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ export class CubeStoreQuery extends BaseQuery {
130130
}
131131

132132
public escapeColumnName(name) {
133-
return `\`${name}\``;
133+
return `\`${name.replaceAll('`', '``')}\``;
134134
}
135135

136136
public seriesSql(timeDimension) {

packages/cubejs-schema-compiler/src/adapter/HiveQuery.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,9 @@ export class HiveQuery extends BaseQuery {
5555
}
5656

5757
public escapeColumnName(name) {
58-
return `\`${name}\``;
58+
// Within a backtick string, use double backticks (``) to represent a backtick character
59+
// https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=27362043#LanguageManualSelect-SelectSyntax
60+
return `\`${name.replaceAll('`', '``')}\``;
5961
}
6062

6163
public simpleQuery() {
@@ -71,7 +73,7 @@ export class HiveQuery extends BaseQuery {
7173
ungroupedAliases: R.fromPairs(this.forSelect().map((m: any) => [m.measure || m.dimension, m.aliasName()]))
7274
}
7375
);
74-
return `SELECT ${select} FROM (${ungrouped}) AS ${this.escapeColumnName('hive_wrapper')}
76+
return `SELECT ${select} FROM (${ungrouped}) AS ${this.escapeColumnName('hive_wrapper')}
7577
${this.groupByClause()}${this.baseHaving(this.measureFilters)}${this.orderBy()}${this.groupByDimensionLimit()}`;
7678
}
7779

packages/cubejs-schema-compiler/src/adapter/MysqlQuery.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,9 @@ export class MysqlQuery extends BaseQuery {
124124
}
125125

126126
public escapeColumnName(name) {
127-
return `\`${name}\``;
127+
// If the character to be included within the identifier is the same as that used to quote the identifier itself, then you need to double the character
128+
// https://dev.mysql.com/doc/refman/8.0/en/identifiers.html
129+
return `\`${name.replaceAll('`', '``')}\``;
128130
}
129131

130132
public seriesSql(timeDimension) {

packages/cubejs-schema-compiler/test/unit/base-query.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2240,6 +2240,16 @@ describe('Class unit tests', () => {
22402240
expect(baseQuery.cubeAlias('CamelCaseCube.id')).toEqual('"camel_case_cube__id"');
22412241
expect(baseQuery.cubeAlias('CamelCaseCube.description')).toEqual('"camel_case_cube__description"');
22422242
expect(baseQuery.cubeAlias('CamelCaseCube.grant_total')).toEqual('"camel_case_cube__grant_total"');
2243+
2244+
// aliasName + memberToAlias
2245+
const memberAliasQuery = new BaseQuery(set, {
2246+
memberToAlias: {
2247+
'CamelCaseCube.id': 'alias("id")'
2248+
}
2249+
});
2250+
expect(memberAliasQuery.aliasName('CamelCaseCube.id', false)).toEqual('alias("id")');
2251+
// Should escape quotes
2252+
expect(memberAliasQuery.newDimension('CamelCaseCube.id').aliasName()).toEqual('"alias(""id"")"');
22432253
});
22442254

22452255
it('Test BaseQuery with aliased cube', async () => {
@@ -2289,6 +2299,17 @@ describe('Class unit tests', () => {
22892299
expect(baseQuery.cubeAlias('CamelCaseCube.id')).toEqual('"t1__id"');
22902300
expect(baseQuery.cubeAlias('CamelCaseCube.description')).toEqual('"t1__description"');
22912301
expect(baseQuery.cubeAlias('CamelCaseCube.grant_total')).toEqual('"t1__grant_total"');
2302+
2303+
// aliasName + memberToAlias
2304+
// Should ignore cube alias
2305+
const memberAliasQuery = new BaseQuery(set, {
2306+
memberToAlias: {
2307+
'CamelCaseCube.id': 'alias("id")'
2308+
}
2309+
});
2310+
expect(memberAliasQuery.aliasName('CamelCaseCube.id', false)).toEqual('alias("id")');
2311+
// Should escape quotes
2312+
expect(memberAliasQuery.newDimension('CamelCaseCube.id').aliasName()).toEqual('"alias(""id"")"');
22922313
});
22932314

22942315
it('Test BaseQuery columns order for the query with the sub-query', async () => {

0 commit comments

Comments
 (0)