Skip to content

Commit

Permalink
fix: quote column name in prom query filter (#1083)
Browse files Browse the repository at this point in the history
## Rationale
PromQL query should be case-sensitive for  column name

## Detailed Changes
- Use `ident` when construct filter.

## Test Plan
Update integration tests and UT
  • Loading branch information
jiacai2050 authored Jul 20, 2023
1 parent 37d2dda commit aadfa0e
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 14 deletions.
18 changes: 11 additions & 7 deletions integration_tests/prom/remote-query.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,23 @@ def prepare_data(ts):
CREATE TABLE if not exists `{}` (
`t` timestamp NOT NULL,
`tag1` string TAG,
`tag2` string TAG,
`TAG2` string TAG,
`value` double NOT NULL,
`VALUE2` double NOT NULL,
timestamp KEY (t)
);
""".format(t))

execute_sql("""
insert into {}(t, tag1, tag2, value, VALUE2)
insert into {}(t, tag1, TAG2, value, VALUE2)
values
({}, "v1", "v2", 1, 2),
({}, "v1", "v2", 11, 22)
;
""".format(table, ts-5000, ts))

execute_sql("""
insert into {}(t, tag1, tag2, value, VALUE2)
insert into {}(t, tag1, TAG2, value, VALUE2)
values
({}, "v1", "v2", 10, 20),
({}, "v1", "v2", 110, 220)
Expand All @@ -60,11 +60,15 @@ def remote_query(ts):

r = execute_pql(table + '{tag1="v1"}[5m]')
result = r['data']['result']
assert result == [{'metric': {'__name__': table, 'tag1': 'v1', 'tag2': 'v2'}, 'values': [[ts-5, '1'], [ts, '11']]}]
assert result == [{'metric': {'__name__': table, 'tag1': 'v1', 'TAG2': 'v2'}, 'values': [[ts-5, '1'], [ts, '11']]}]

r = execute_pql(table + '{TAG2="v2"}[5m]')
result = r['data']['result']
assert result == [{'metric': {'__name__': table, 'tag1': 'v1', 'TAG2': 'v2'}, 'values': [[ts-5, '1'], [ts, '11']]}]

r = execute_pql(table + '{tag1=~"v1"}[5m]')
result = r['data']['result']
assert result == [{'metric': {'__name__': table, 'tag1': 'v1', 'tag2': 'v2'}, 'values': [[ts-5, '1'], [ts, '11']]}]
assert result == [{'metric': {'__name__': table, 'tag1': 'v1', 'TAG2': 'v2'}, 'values': [[ts-5, '1'], [ts, '11']]}]

r = execute_pql(table + '{tag1!="v1"}[5m]')
result = r['data']['result']
Expand All @@ -77,12 +81,12 @@ def remote_query(ts):
# uppercase field
r = execute_pql(table + '{tag1="v1",__ceresdb_field__="VALUE2"}[5m]')
result = r['data']['result']
assert result == [{'metric': {'__name__': table, 'tag1': 'v1', 'tag2': 'v2'}, 'values': [[ts-5, '2'], [ts, '22']]}]
assert result == [{'metric': {'__name__': table, 'tag1': 'v1', 'TAG2': 'v2'}, 'values': [[ts-5, '2'], [ts, '22']]}]

# uppercase table
r = execute_pql(table2 + '{tag1="v1"}[5m]')
result = r['data']['result']
assert result == [{'metric': {'__name__': table2, 'tag1': 'v1', 'tag2': 'v2'}, 'values': [[ts-5, '10'], [ts, '110']]}]
assert result == [{'metric': {'__name__': table2, 'tag1': 'v1', 'TAG2': 'v2'}, 'values': [[ts-5, '10'], [ts, '110']]}]

def main():
ts = now()
Expand Down
15 changes: 8 additions & 7 deletions query_frontend/src/promql/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use common_types::{schema::Schema, time::TimeRange};
use datafusion::{
logical_expr::LogicalPlanBuilder,
optimizer::utils::conjunction,
prelude::{col, lit, regexp_match, Expr},
prelude::{ident, lit, regexp_match, Expr},
sql::{planner::ContextProvider, TableReference},
};
use prom_remote_api::types::{label_matcher, LabelMatcher, Query};
Expand Down Expand Up @@ -95,16 +95,17 @@ fn normalize_matchers(matchers: Vec<LabelMatcher>) -> Result<(String, String, Ve
NAME_LABEL => metric = Some(m.value),
FIELD_LABEL => field = Some(m.value),
_ => {
let col_name = ident(&m.name);
let expr = match m.r#type() {
label_matcher::Type::Eq => col(m.name).eq(lit(m.value)),
label_matcher::Type::Neq => col(m.name).not_eq(lit(m.value)),
label_matcher::Type::Eq => col_name.eq(lit(m.value)),
label_matcher::Type::Neq => col_name.not_eq(lit(m.value)),
// https://github.com/prometheus/prometheus/blob/2ce94ac19673a3f7faf164e9e078a79d4d52b767/model/labels/regexp.go#L29
label_matcher::Type::Re => {
regexp_match(vec![col(m.name), lit(format!("^(?:{})$", m.value))])
regexp_match(vec![col_name, lit(format!("^(?:{})$", m.value))])
.is_not_null()
}
label_matcher::Type::Nre => {
regexp_match(vec![col(m.name), lit(format!("^(?:{})$", m.value))]).is_null()
regexp_match(vec![col_name, lit(format!("^(?:{})$", m.value))]).is_null()
}
};

Expand Down Expand Up @@ -215,7 +216,7 @@ Query(QueryPlan { df_plan: Sort: cpu.tsid ASC NULLS FIRST, cpu.time ASC NULLS FI
("a", "1", Type::Eq),
("b", "2", Type::Neq),
("c", "3", Type::Re),
("d", "4", Type::Nre),
("D", "4", Type::Nre),
(NAME_LABEL, "cpu", Type::Eq),
]);

Expand All @@ -226,7 +227,7 @@ Query(QueryPlan { df_plan: Sort: cpu.tsid ASC NULLS FIRST, cpu.time ASC NULLS FI
r#"a = Utf8("1")
b != Utf8("2")
regexpmatch(c, Utf8("^(?:3)$")) IS NOT NULL
regexpmatch(d, Utf8("^(?:4)$")) IS NULL"#,
regexpmatch(D, Utf8("^(?:4)$")) IS NULL"#,
filters
.iter()
.map(|f| f.to_string())
Expand Down

0 comments on commit aadfa0e

Please sign in to comment.