Skip to content

Commit 9f9fa46

Browse files
talorictaloric
committed
fix: handle extra_filters with correct seq
1 parent 57b71af commit 9f9fa46

2 files changed

Lines changed: 127 additions & 76 deletions

File tree

‎server/querier/app/prometheus/service/converters.go‎

Lines changed: 85 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,9 @@ func (p *prometheusReader) parseExtraFilters(filter *extraFilters, db string) (s
386386
if filter.label == "" || filter.operator == "" {
387387
return "", ""
388388
}
389+
if filter.operator == "exist" || filter.operator == "not exist" {
390+
return filter.label, fmt.Sprintf("%s(%s)", filter.operator, filter.label)
391+
}
389392
return filter.label, fmt.Sprintf("%s %s %s", filter.label, filter.operator, escapeSingleQuote(filter.value))
390393
}
391394

@@ -424,26 +427,34 @@ func (p *prometheusReader) parseMatchers(matcher *prompb.LabelMatcher, prefixTyp
424427
}
425428

426429
// parse extra-filters to filters in `where` clause
427-
func (p *prometheusReader) parseExtraFiltersToWhereClause(extraLabelFilters [][]*extraFilters, prefixType prefix, db string, handleTags func(string, bool)) string {
428-
outerFilters := make([]string, 0, len(extraLabelFilters))
429-
for i := 0; i < len(extraLabelFilters); i++ {
430-
innerFilters := make([]string, 0, len(extraLabelFilters[i]))
431-
for j := 0; j < len(extraLabelFilters[i]); j++ {
432-
matcher := extraLabelFilters[i][j]
433-
tagName, newFilter := p.parseExtraFilters(matcher, db)
434-
if newFilter == "" {
435-
continue
436-
}
437-
innerFilters = append(innerFilters, newFilter)
438-
handleTags(tagName, matcher.isTag)
430+
func (p *prometheusReader) parseExtraFiltersToWhereClause(node *filterNode, prefixType prefix, db string, handleTags func(string, bool)) string {
431+
if node == nil {
432+
return ""
433+
}
434+
if node.filter != nil {
435+
// leaf node
436+
tagName, sql := p.parseExtraFilters(node.filter, db)
437+
if sql == "" {
438+
return ""
439439
}
440-
// inside matchers use 'AND' for connected
441-
if len(innerFilters) > 0 {
442-
outerFilters = append(outerFilters, fmt.Sprintf("(%s)", strings.Join(innerFilters, " AND ")))
440+
handleTags(tagName, node.filter.isTag)
441+
return sql
442+
}
443+
// internal AND/OR node: recurse into children
444+
parts := make([]string, 0, len(node.children))
445+
for _, child := range node.children {
446+
s := p.parseExtraFiltersToWhereClause(child, prefixType, db, handleTags)
447+
if s != "" {
448+
parts = append(parts, s)
443449
}
444450
}
445-
// outside matchers use 'OR' for connected
446-
return fmt.Sprintf("(%s)", strings.Join(outerFilters, " OR "))
451+
if len(parts) == 0 {
452+
return ""
453+
}
454+
if len(parts) == 1 {
455+
return parts[0]
456+
}
457+
return fmt.Sprintf("(%s)", strings.Join(parts, " "+node.op+" "))
447458
}
448459

449460
// return: prefixType, metricName, db, table, dataPrecision, metricAlias
@@ -1569,60 +1580,62 @@ type extraFilters struct {
15691580
isTag bool
15701581
}
15711582

1572-
func parseExtraFiltersToMatchers(filters string) ([][]*extraFilters, error) {
1583+
// filterNode is a boolean expression tree of extra filters.
1584+
// Leaf node: filter != nil. Internal node: op is "AND"/"OR", children non-empty.
1585+
type filterNode struct {
1586+
filter *extraFilters
1587+
op string
1588+
children []*filterNode
1589+
}
1590+
1591+
func parseExtraFiltersToMatchers(filters string) (*filterNode, error) {
15731592
fakeSQL := fmt.Sprintf("select 1 from t where %s", filters)
15741593
stmt, err := sqlparser.Parse(fakeSQL)
15751594
if err != nil {
15761595
return nil, err
15771596
}
15781597
selectStmt := stmt.(*sqlparser.Select)
1579-
labelMatchers := make([][]*extraFilters, 0)
1580-
_, err = iterateExprs(selectStmt.Where.Expr, &labelMatchers)
1581-
if err != nil {
1582-
return nil, err
1583-
}
1584-
return labelMatchers, nil
1598+
return iterateExprs(selectStmt.Where.Expr)
15851599
}
15861600

1587-
func iterateExprs(node sqlparser.Expr, labelMatchers *[][]*extraFilters) (sqlparser.Expr, error) {
1601+
// iterateExprs builds a filterNode tree from a SQL expression.
1602+
// AND/OR structure is preserved as-is; no DNF flattening needed.
1603+
func iterateExprs(node sqlparser.Expr) (*filterNode, error) {
15881604
switch node := node.(type) {
15891605
case *sqlparser.AndExpr:
1590-
left, err := iterateExprs(node.Left, labelMatchers)
1606+
left, err := iterateExprs(node.Left)
15911607
if err != nil {
1592-
return left, err
1608+
return nil, err
15931609
}
1594-
right, err := iterateExprs(node.Right, labelMatchers)
1610+
right, err := iterateExprs(node.Right)
15951611
if err != nil {
1596-
return right, err
1612+
return nil, err
15971613
}
15981614
if left == nil {
15991615
return right, nil
1600-
} else if right == nil {
1616+
}
1617+
if right == nil {
16011618
return left, nil
16021619
}
1603-
return node, nil
1620+
return &filterNode{op: "AND", children: []*filterNode{left, right}}, nil
16041621
case *sqlparser.OrExpr:
1605-
left, err := iterateExprs(node.Left, labelMatchers)
1622+
left, err := iterateExprs(node.Left)
16061623
if err != nil {
1607-
return left, err
1624+
return nil, err
16081625
}
1609-
right, err := iterateExprs(node.Right, labelMatchers)
1626+
right, err := iterateExprs(node.Right)
16101627
if err != nil {
1611-
return right, err
1628+
return nil, err
16121629
}
16131630
if left == nil {
16141631
return right, nil
1615-
} else if right == nil {
1632+
}
1633+
if right == nil {
16161634
return left, nil
16171635
}
1618-
return node, nil
1636+
return &filterNode{op: "OR", children: []*filterNode{left, right}}, nil
16191637
case *sqlparser.ParenExpr:
1620-
(*labelMatchers) = append((*labelMatchers), []*extraFilters{})
1621-
expr, err := iterateExprs(node.Expr, labelMatchers)
1622-
if err != nil {
1623-
return expr, err
1624-
}
1625-
return expr, nil
1638+
return iterateExprs(node.Expr)
16261639
case *sqlparser.ComparisonExpr:
16271640
var comparExpr sqlparser.Expr
16281641
if parenExpr, ok := node.Left.(*sqlparser.ParenExpr); ok {
@@ -1644,22 +1657,41 @@ func iterateExprs(node sqlparser.Expr, labelMatchers *[][]*extraFilters) (sqlpar
16441657
op = node.Operator
16451658
istag = true
16461659
}
1647-
lastIndex := len(*labelMatchers) - 1
1648-
if lastIndex < 0 {
1649-
(*labelMatchers) = append((*labelMatchers), []*extraFilters{})
1650-
lastIndex = len(*labelMatchers) - 1
1651-
}
1652-
1653-
(*labelMatchers)[lastIndex] = append((*labelMatchers)[lastIndex], &extraFilters{
1660+
return &filterNode{filter: &extraFilters{
16541661
operator: op,
16551662
// some tag will escape by sqlparser
16561663
// https://github.com/xwb1989/sqlparser/blob/master/token.go#L85
16571664
label: removeEscapeQuote(colName, "`"),
16581665
value: removeEscapeQuote(colValue, "'"),
16591666
isTag: istag,
1660-
})
1661-
return node, nil
1667+
}}, nil
1668+
case *sqlparser.NotExpr:
1669+
if funcExpr, ok := node.Expr.(*sqlparser.FuncExpr); ok &&
1670+
strings.ToLower(funcExpr.Name.String()) == "exist" &&
1671+
len(funcExpr.Exprs) == 1 {
1672+
if aliasedExpr, ok := funcExpr.Exprs[0].(*sqlparser.AliasedExpr); ok {
1673+
tagName := removeEscapeQuote(sqlparser.String(aliasedExpr.Expr), "`")
1674+
return &filterNode{filter: &extraFilters{
1675+
operator: "not exist",
1676+
label: tagName,
1677+
isTag: true,
1678+
}}, nil
1679+
}
1680+
}
1681+
return nil, nil
1682+
case *sqlparser.FuncExpr:
1683+
if strings.ToLower(node.Name.String()) == "exist" && len(node.Exprs) == 1 {
1684+
if aliasedExpr, ok := node.Exprs[0].(*sqlparser.AliasedExpr); ok {
1685+
tagName := removeEscapeQuote(sqlparser.String(aliasedExpr.Expr), "`")
1686+
return &filterNode{filter: &extraFilters{
1687+
operator: "exist",
1688+
label: tagName,
1689+
isTag: true,
1690+
}}, nil
1691+
}
1692+
}
1693+
return nil, nil
16621694
default:
1663-
return node, nil
1695+
return nil, nil
16641696
}
16651697
}

‎server/querier/app/prometheus/service/converters_test.go‎

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -918,33 +918,52 @@ func TestParseQueryRequestToSQL(t *testing.T) {
918918
}
919919

920920
func TestParseExFilters(t *testing.T) {
921+
p := &prometheusReader{}
922+
noopTags := func(string, bool) {}
923+
921924
var testCases = []struct {
922-
input string
923-
output [][]*prompb.LabelMatcher
924-
}{{
925-
input: "(pod=1 and pod_ns=2) or (pod=1 and pod_ns=3) or (pod=5 and pod_ns=3)",
926-
output: [][]*prompb.LabelMatcher{
927-
{{Name: "pod", Type: prompb.LabelMatcher_EQ, Value: "1"}, {Name: "pod_ns", Type: prompb.LabelMatcher_EQ, Value: "2"}},
928-
{{Name: "pod", Type: prompb.LabelMatcher_EQ, Value: "1"}, {Name: "pod_ns", Type: prompb.LabelMatcher_EQ, Value: "3"}},
929-
{{Name: "pod", Type: prompb.LabelMatcher_EQ, Value: "5"}, {Name: "pod_ns", Type: prompb.LabelMatcher_EQ, Value: "3"}},
930-
},
931-
}}
925+
input string
926+
wantSQL string
927+
}{
928+
{
929+
// AND/OR tree preserved: A AND B AND (C OR D) stays as-is, no DNF expansion
930+
input: "((pod_cluster_id in (2) AND vpc_id IN (1) AND (chost_id IN (1,2,3,4,5,6) OR exist(chost_id))))",
931+
wantSQL: "((pod_cluster_id in (2) AND vpc_id in (1)) AND (chost_id in (1, 2, 3, 4, 5, 6) OR exist(chost_id)))",
932+
},
933+
{
934+
input: "((pod_cluster_id in (2) AND vpc_id IN (1) AND (chost_id IN (1,2,3,4,5,6) OR not exist(chost_id))))",
935+
wantSQL: "((pod_cluster_id in (2) AND vpc_id in (1)) AND (chost_id in (1, 2, 3, 4, 5, 6) OR not exist(chost_id)))",
936+
},
937+
{
938+
// explicit parentheses with OR
939+
input: "(pod_id=1 and pod_ns_id=2) or (pod_id=1 and pod_ns_id=3) or (pod_id=5 and pod_ns_id=3)",
940+
wantSQL: "(((pod_id = 1 AND pod_ns_id = 2) OR (pod_id = 1 AND pod_ns_id = 3)) OR (pod_id = 5 AND pod_ns_id = 3))",
941+
},
942+
{
943+
// AND binds tighter than OR without parentheses
944+
input: "pod_id=1 and pod_ns_id=2 or pod_id=5 and pod_ns_id=3",
945+
wantSQL: "((pod_id = 1 AND pod_ns_id = 2) OR (pod_id = 5 AND pod_ns_id = 3))",
946+
},
947+
{
948+
// simple OR chain
949+
input: "pod_id=1 or pod_id=2 or pod_id=3",
950+
wantSQL: "((pod_id = 1 OR pod_id = 2) OR pod_id = 3)",
951+
},
952+
{
953+
// left side with parens, right side plain
954+
input: "(pod_id=1 and pod_ns_id=2) or pod_id=5",
955+
wantSQL: "((pod_id = 1 AND pod_ns_id = 2) OR pod_id = 5)",
956+
},
957+
}
932958
t.Run("ParseExFilters", func(t *testing.T) {
933-
for i := 0; i < len(testCases); i++ {
934-
output, err := parseExtraFiltersToMatchers(testCases[i].input)
959+
for i, tc := range testCases {
960+
tree, err := parseExtraFiltersToMatchers(tc.input)
935961
if err != nil {
936-
t.Errorf("Expected output: %v, got: %v", testCases[i].output, err)
937-
} else {
938-
outerIndex := len(testCases[i].output)
939-
for j := 0; j < outerIndex; j++ {
940-
innerIndex := len(testCases[i].output[j])
941-
for k := 0; k < innerIndex; k++ {
942-
assert.Equal(t, testCases[i].output[j][k].Name, output[j][k].Name)
943-
assert.Equal(t, testCases[i].output[j][k].Value, output[j][k].Value)
944-
assert.Equal(t, testCases[i].output[j][k].Type, output[j][k].Type)
945-
}
946-
}
962+
t.Errorf("case %d: unexpected error: %v", i, err)
963+
continue
947964
}
965+
got := p.parseExtraFiltersToWhereClause(tree, prefixNone, "", noopTags)
966+
assert.Equal(t, tc.wantSQL, got, "case %d", i)
948967
}
949968
})
950969
}

0 commit comments

Comments
 (0)