Skip to content

Commit

Permalink
Merge pull request #549 from Altinity/issue-409
Browse files Browse the repository at this point in the history 
add support ORDER BY ... WITH FILL  into $columns macros
  • Loading branch information
@Slach
Slach authored May 3, 2024
2 parents 0749b8c + 75667f4 commit c993b67
Show file tree
Hide file tree
Showing 7 changed files with 350 additions and 23 deletions.
148 changes: 148 additions & 0 deletions docker/grafana/dashboards/columns_order_by_with_fill_issue_409.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
{
"annotations": {
"list": [
{
"builtIn": 1,
"datasource": {
"type": "grafana",
"uid": "-- Grafana --"
},
"enable": true,
"hide": true,
"iconColor": "rgba(0, 211, 255, 1)",
"name": "Annotations & Alerts",
"type": "dashboard"
}
]
},
"description": "https://github.com/Altinity/clickhouse-grafana/issues/409",
"editable": true,
"fiscalYearStartMonth": 0,
"graphTooltip": 0,
"id": 36,
"links": [],
"panels": [
{
"datasource": {
"type": "vertamedia-clickhouse-datasource",
"uid": "P7E099F39B84EA795"
},
"fieldConfig": {
"defaults": {
"color": {
"mode": "palette-classic"
},
"custom": {
"axisBorderShow": false,
"axisCenteredZero": false,
"axisColorMode": "text",
"axisLabel": "",
"axisPlacement": "auto",
"barAlignment": 0,
"drawStyle": "line",
"fillOpacity": 0,
"gradientMode": "none",
"hideFrom": {
"legend": false,
"tooltip": false,
"viz": false
},
"insertNulls": false,
"lineInterpolation": "linear",
"lineWidth": 1,
"pointSize": 5,
"scaleDistribution": {
"type": "linear"
},
"showPoints": "auto",
"spanNulls": false,
"stacking": {
"group": "A",
"mode": "none"
},
"thresholdsStyle": {
"mode": "off"
}
},
"mappings": [],
"thresholds": {
"mode": "absolute",
"steps": [
{
"color": "green",
"value": null
},
{
"color": "red",
"value": 80
}
]
}
},
"overrides": []
},
"gridPos": {
"h": 8,
"w": 12,
"x": 0,
"y": 0
},
"id": 1,
"options": {
"legend": {
"calcs": [],
"displayMode": "list",
"placement": "bottom",
"showLegend": true
},
"tooltip": {
"mode": "single",
"sort": "none"
}
},
"targets": [
{
"add_metadata": false,
"database": "default",
"datasource": {
"type": "vertamedia-clickhouse-datasource",
"uid": "P7E099F39B84EA795"
},
"dateColDataType": "",
"dateTimeColDataType": "event_time",
"dateTimeType": "DATETIME",
"editorMode": "builder",
"extrapolate": true,
"format": "time_series",
"formattedQuery": "SELECT $timeSeries as t, count() FROM $table WHERE $timeFilter GROUP BY t ORDER BY t",
"interval": "",
"intervalFactor": 1,
"query": "$columns(\r\n service_name,\r\n count() as sum_req\r\n) FROM $table \r\nWHERE\r\n $timeFilter\r\n AND service_name != 'deprecated'\r\nGROUP BY t, service_name\r\nORDER BY service_name, t WITH FILL STEP 6000\r\n",
"rawQuery": "SELECT t, groupArray((service_name, sum_req)) AS groupArr FROM ( SELECT (intDiv(toUInt32(event_time), 30) * 30) * 1000 AS t, service_name, count() as sum_req FROM default.test_grafana \r\nWHERE event_time >= toDateTime(1714722990) AND event_time <= toDateTime(1714744590) AND\r\n event_time >= toDateTime(1714722990) AND event_time <= toDateTime(1714744590)\r\n AND service_name != 'deprecated'\r GROUP BY t, status_code\r ORDER BY t WITH FILL STEP 6000) GROUP BY t ORDER BY t",
"refId": "A",
"round": "0s",
"showFormattedSQL": true,
"skip_comments": true,
"table": "test_grafana"
}
],
"title": "columns + ORDER BY WITH FILL",
"type": "timeseries"
}
],
"schemaVersion": 39,
"tags": [],
"templating": {
"list": []
},
"time": {
"from": "now-6h",
"to": "now"
},
"timepicker": {},
"timezone": "browser",
"title": "$columns + ORDER BY WITH FILL",
"uid": "adklfil16opa8e",
"version": 1,
"weekStart": ""
}
4 changes: 2 additions & 2 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 34 additions & 8 deletions pkg/eval_query.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,12 +342,38 @@ func (q *EvalQuery) _columns(key, value, beforeMacrosQuery, fromQuery string) (s
var keyAlias = keySplit[len(keySplit)-1]
var valueSplit = strings.Split(strings.Trim(value, " \xA0\t\r\n"), " ")
var valueAlias = valueSplit[len(valueSplit)-1]
var havingIndex = strings.Index(strings.ToLower(fromQuery), "having")
var having = ""
var groupByQuery = " GROUP BY t, " + keyAlias
var orderByQuery = " ORDER BY t, " + keyAlias
var havingQuery = ""
if matched, err := regexp.MatchString(`(?mi)^\s*FROM\s*\(`, fromQuery); err == nil && !matched {
var groupByIndex = strings.Index(strings.ToLower(fromQuery), "group by")
var havingIndex = strings.Index(strings.ToLower(fromQuery), "having")
var orderByIndex = strings.Index(strings.ToLower(fromQuery), "order by")

if havingIndex >= 0 && orderByIndex >= 0 && havingIndex >= orderByIndex {
return "", fmt.Errorf("ORDER BY clause shall be before HAVING")
}

if havingIndex != -1 {
having = " " + fromQuery[havingIndex:]
fromQuery = fromQuery[0 : havingIndex-1]
if groupByIndex >= 0 && orderByIndex >= 0 && groupByIndex >= orderByIndex {
return "", fmt.Errorf("GROUP BY clause shall be before ORDER BY")
}

if groupByIndex >= 0 && havingIndex >= 0 && groupByIndex >= havingIndex {
return "", fmt.Errorf("GROUP BY clause shall be before HAVING")
}

if orderByIndex != -1 {
orderByQuery = " " + fromQuery[orderByIndex:]
fromQuery = fromQuery[0 : orderByIndex-1]
}
if havingIndex != -1 {
havingQuery = " " + fromQuery[havingIndex:]
fromQuery = fromQuery[0 : havingIndex-1]
}
if groupByIndex != -1 {
groupByQuery = " " + fromQuery[groupByIndex:]
fromQuery = fromQuery[0 : groupByIndex-1]
}
}
fromQuery = q._applyTimeFilter(fromQuery)

Expand All @@ -359,9 +385,9 @@ func (q *EvalQuery) _columns(key, value, beforeMacrosQuery, fromQuery string) (s
", " + key +
", " + value + " " +
fromQuery +
" GROUP BY t, " + keyAlias +
having +
" ORDER BY t, " + keyAlias +
groupByQuery +
havingQuery +
orderByQuery +
")" +
" GROUP BY t" +
" ORDER BY t", nil
Expand Down
73 changes: 71 additions & 2 deletions pkg/eval_query_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ func TestCommentsAndRateMacrosWithFromKeywordInFieldName(t *testing.T) {
r.NoError(err)
actual, err := q.applyMacros(query, ast)
r.NoError(err)
r.Equal(actual, expQuery, "gets replaced with right FROM query")
r.Equal(expQuery, actual, "gets replaced with right FROM query")
}

/*
Expand Down Expand Up @@ -414,7 +414,40 @@ func TestColumnsMacrosWithUnionAllAndWithKeyword(t *testing.T) {
r.NoError(err)
actual, err := q.applyMacros(query, ast)
r.NoError(err)
r.Equal(actual, expQuery, "gets replaced with right FROM query")
r.Equal(expQuery, actual, "gets replaced with right FROM query")
}

/*
columns + ORDER BY WITH FILL
fix https://github.com/Altinity/clickhouse-grafana/issues/409
*/
func TestColumnsMacrosWithGroupWithFill(t *testing.T) {
const query = "$columns(\n" +
" status_code,\n" +
" sum(request) as sum_req" +
"\n)\n" +
"FROM $table\n" +
"WHERE\n" +
" $timeFilter\n" +
" AND status_code != 201 AND status_code != 0\n" +
"GROUP BY t, status_code\n" +
"ORDER BY t WITH FILL STEP 60000"
const expQuery = "SELECT t, groupArray((status_code, sum_req)) AS groupArr FROM ( " +
"SELECT $timeSeries AS t, status_code, sum(request) as sum_req FROM $table\n" +
"WHERE $timeFilter AND\n" +
" $timeFilter\n" +
" AND status_code != 201 AND status_code != 0" +
" GROUP BY t, status_code" +
" ORDER BY t WITH FILL STEP 60000" +
") GROUP BY t ORDER BY t"
r := require.New(t)
q := EvalQuery{}
scanner := newScanner(query)
ast, err := scanner.toAST()
r.NoError(err)
actual, err := q.applyMacros(query, ast)
r.NoError(err)
r.Equal(expQuery, actual, "gets replaced with right FROM query")
}

type astTestCase struct {
Expand Down Expand Up @@ -1432,6 +1465,42 @@ func TestScannerAST(t *testing.T) {
}},
}},
),
/* fix https://github.com/Altinity/clickhouse-grafana/issues/409 */
newASTTestCase(
"AST case 28 $columns + ORDER BY ... WITH FILL",
"$columns(\n"+
" service_name, \n"+
" sum(agg_value) as value\n"+
")\n"+
"FROM $table\n"+
"WHERE service_name='mysql'\n"+
"GROUP BY t, service_name\n"+
"HAVING value>100\n"+
"ORDER BY t, service_name WITH FILL 60000",
&EvalAST{Obj: map[string]interface{}{
"root": newEvalAST(false),
"select": newEvalAST(false),
"$columns": &EvalAST{Arr: []interface{}{
"service_name",
"sum(agg_value) as value",
}},
"from": &EvalAST{Arr: []interface{}{
"$table",
}},
"where": &EvalAST{Arr: []interface{}{
"service_name = 'mysql'",
}},
"having": &EvalAST{Arr: []interface{}{
"value > 100",
}},
"group by": &EvalAST{Arr: []interface{}{
"t", "service_name",
}},
"order by": &EvalAST{Arr: []interface{}{
"t", "service_name WITH FILL 60000",
}},
}},
),
}

r := require.New(t)
Expand Down
51 changes: 40 additions & 11 deletions src/datasource/sql-query/sql-query-macros.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,13 +322,44 @@ export default class SqlQueryMacros {
}

let keyAlias = key.trim().split(' ').pop(),
valueAlias = value.trim().split(' ').pop(),
havingIndex = fromQuery.toLowerCase().indexOf('having'),
having = '';
valueAlias = value.trim().split(' ').pop();

let groupByQuery = ' GROUP BY t, ' + keyAlias;
let havingQuery = '';
let orderByQuery = ' ORDER BY t, ' + keyAlias;
const fromRe = /^\s*FROM\s*\(/mi;
if (!fromRe.test(fromQuery)) {
const groupByIndex = fromQuery.toLowerCase().indexOf('group by');
const havingIndex = fromQuery.toLowerCase().indexOf('having');
const orderByIndex = fromQuery.toLowerCase().indexOf('order by');

if (havingIndex >= 0 && orderByIndex >= 0 && havingIndex >= orderByIndex) {
throw {message: "ORDER BY clause shall be before HAVING"};
}

if (havingIndex !== -1) {
having = ' ' + fromQuery.slice(havingIndex, fromQuery.length);
fromQuery = fromQuery.slice(0, havingIndex - 1);
if (groupByIndex >= 0 && orderByIndex >= 0 && groupByIndex >= orderByIndex) {
throw {message: "GROUP BY clause shall be before ORDER BY"};
}

if (groupByIndex >= 0 && havingIndex >= 0 && groupByIndex >= havingIndex) {
throw {message: "GROUP BY clause shall be before HAVING"};
}


if (orderByIndex !== -1) {
orderByQuery = ' ' + fromQuery.slice(orderByIndex, fromQuery.length);
fromQuery = fromQuery.slice(0, orderByIndex - 1);
}

if (havingIndex !== -1) {
havingQuery = ' ' + fromQuery.slice(havingIndex, fromQuery.length);
fromQuery = fromQuery.slice(0, havingIndex - 1);
}

if (groupByIndex !== -1) {
groupByQuery = ' ' + fromQuery.slice(groupByIndex, fromQuery.length);
fromQuery = fromQuery.slice(0, groupByIndex - 1);
}
}
fromQuery = SqlQueryMacros._applyTimeFilter(fromQuery);

Expand All @@ -349,11 +380,9 @@ export default class SqlQueryMacros {
value +
' ' +
fromQuery +
' GROUP BY t, ' +
keyAlias +
having +
' ORDER BY t, ' +
keyAlias +
groupByQuery +
havingQuery +
orderByQuery +
')' +
' GROUP BY t' +
' ORDER BY t'
Expand Down
Loading

0 comments on commit c993b67

Please sign in to comment.