-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Clickhouse string ordering and string filtering by UTF8 instead of bytes #6143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
63b935c
1ea8d11
49111c2
153b3fd
b2c4ee9
dd2fd32
f26f4fe
a5b279e
f7a4759
83437e9
600c1fa
eadfa1a
0610d16
8807178
f095335
e76b91a
fdebe6e
be6875b
4fc9ed4
b44e57e
a2616fe
ce3ec56
ee06aee
b649df2
716f24b
79ab001
f660674
beac449
bd29d16
a62eb42
225c19a
8971b45
f712829
d18b953
79373d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,4 +1,6 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { parseSqlInterval } from '@cubejs-backend/shared'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import R from 'ramda'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { getEnv, parseSqlInterval } from '@cubejs-backend/shared'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { BaseQuery } from './BaseQuery'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { BaseFilter } from './BaseFilter'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { UserError } from '../compiler/UserError'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -18,7 +20,7 @@ class ClickHouseFilter extends BaseFilter { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public likeIgnoreCase(column, not, param, type) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const p = (!type || type === 'contains' || type === 'ends') ? '%' : ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const s = (!type || type === 'contains' || type === 'starts') ? '%' : ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return `lower(${column}) ${not ? 'NOT' : ''} LIKE CONCAT('${p}', lower(${this.allocateParam(param)}), '${s}')`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return `${column} ${not ? 'NOT' : ''} ILIKE CONCAT('${p}', ${this.allocateParam(param)}, '${s}')`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public castParameter() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -123,7 +125,7 @@ export class ClickHouseQuery extends BaseQuery { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.join(' AND '); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public getFieldAlias(id) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public getField(id) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const equalIgnoreCase = (a, b) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
typeof a === 'string' && typeof b === 'string' && a.toUpperCase() === b.toUpperCase() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -134,16 +136,34 @@ export class ClickHouseQuery extends BaseQuery { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
d => equalIgnoreCase(d.dimension, id), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!field) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
field = this.measures.find( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
d => equalIgnoreCase(d.measure, id) || equalIgnoreCase(d.expressionName, id), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return field; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public getFieldAlias(id) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const field = this.getField(id); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (field) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return field.aliasName(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
field = this.measures.find( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
d => equalIgnoreCase(d.measure, id) || equalIgnoreCase(d.expressionName, id), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public getFieldType(hash) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!hash || !hash.id) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const field = this.getField(hash.id); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (field) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return field.aliasName(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return field.definition().type; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -168,6 +188,43 @@ export class ClickHouseQuery extends BaseQuery { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return `${fieldAlias} ${direction}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public getCollation() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const useCollation = getEnv('clickhouseUseCollation', { dataSource: this.dataSource }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And the same question here: should it be public? |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (useCollation) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return getEnv('clickhouseSortCollation', { dataSource: this.dataSource }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public override orderBy() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
MazterQyou marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// ClickHouse orders string by bytes, so we need to use COLLATE 'en' to order by string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (R.isEmpty(this.order)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const collation = this.getCollation(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const orderByString = R.pipe( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
R.map((order) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let orderString = this.orderHashToString(order); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (collation && this.getFieldType(order) === 'string') { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
orderString = `${orderString} COLLATE '${collation}'`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return orderString; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
R.reject(R.isNil), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
R.join(', ') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
)(this.order); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+203
to
+219
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't you mind rewriting this in plain JS instead of using Ramda?
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!orderByString) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ` ORDER BY ${orderByString}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public groupByClause() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (this.ungrouped) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ''; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -281,6 +338,22 @@ export class ClickHouseQuery extends BaseQuery { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// ClickHouse intervals have a distinct type for each granularity | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
delete templates.types.interval; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
delete templates.types.binary; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const collation = this.getCollation(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (collation) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
templates.expressions.sort = `${templates.expressions.sort}{% if data_type and data_type == 'string' %} COLLATE '${collation}'{% endif %}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
templates.expressions.order_by = `${templates.expressions.order_by}{% if data_type and data_type == 'string' %} COLLATE '${collation}'{% endif %}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const oldOrderBy = '{% if order_by %}\nORDER BY {{ order_by | map(attribute=\'expr\') | join(\', \') }}{% endif %}'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const newOrderBy = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'{% if order_by %}\nORDER BY {% for item in order_by %}{{ item.expr }}' + | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
`{%- if item.data_type and item.data_type == 'string' %} COLLATE '${collation}'{% endif %}` + | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'{%- if not loop.last %}, {% endif %}{% endfor %}{% endif %}'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+348
to
+353
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest staying away from replacing strings by putting the original I would suggest moving whole |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
templates.statements.select = templates.statements.select.replace(oldOrderBy, newOrderBy); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return templates; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to make it public? Maybe private would be better?