Skip to content

Commit

Permalink
remove redundant clauses when normalizing (#1505)
Browse files Browse the repository at this point in the history
In some cases with tooling a `:cq` can get applied multiple
times. If the common query condition has an OR clause, then
that will get expanded with the cross product when computing
the DNF. As a result, the normalized query resulting from
applying a `:cq` once would differ from applying the same
`:cq` multiple times. This change removes redundant clauses
from the normalized OR set to avoid the discrepancy. Redundant
means that the removed clause will not change the result
since another branch of the OR would have matched.
  • Loading branch information
brharrington authored Dec 14, 2022
1 parent 8e7368c commit c2dd5c3
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 4 deletions.
29 changes: 25 additions & 4 deletions atlas-webapi/src/main/scala/com/netflix/atlas/webapi/ExprApi.scala
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,24 @@ object ExprApi {
case q => q
}

/**
* For conjunctions that are combined with OR, if a given conjunction is a superset
* of every other conjunction, then it can be removed because an entry would be matched
* based on the other branches of the OR, so the additional conditions do not change the
* outcome.
*/
private def removeRedundantClauses(queries: List[List[Query]]): List[List[Query]] = {
queries match {
case Nil => queries
case _ :: Nil => queries
case _ =>
val sets = queries.map(_.toSet)
queries.filterNot { q =>
sets.forall(_.subsetOf(q.toSet))
}
}
}

/**
* Combines a set of query clauses together using AND. Common query clauses that can
* be applied later using :cq can be added to the exclude set so they will get ignored
Expand All @@ -348,19 +366,22 @@ object ExprApi {
*/
private def sort(query: Query): Query = {
val simplified = Query.simplify(query)
Query
val normalized = Query
.dnfList(simplified)
.map { q =>
Query
.cnfList(q)
.map(normalizeClauses)
.distinct
.sortWith(_.toString < _.toString)
.reduce { (q1, q2) =>
Query.And(q1, q2)
}
}
.distinct
removeRedundantClauses(normalized)
.map { qs =>
qs.reduce { (q1, q2) =>
Query.And(q1, q2)
}
}
.sortWith(_.toString < _.toString) // order OR clauses
.reduce { (q1, q2) =>
Query.Or(q1, q2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,12 @@ class ExprApiSuite extends MUnitRouteSuite {
assertEquals(normalize(expr), List(expected))
}

test("normalize, remove redundant clauses") {
val expr = "name,a,:eq,:sum,b,:has,c,:has,:or,:cq,b,:has,c,:has,:or,:cq"
val expected = "b,:has,name,a,:eq,:and,c,:has,name,a,:eq,:and,:or,:sum"
assertEquals(normalize(expr), List(expected))
}

test("normalize simplify query") {
val input = "app,foo,:eq,name,cpuUser,:eq,:and,:true,:and,:sum"
val expected = "app,foo,:eq,name,cpuUser,:eq,:and,:sum"
Expand Down

0 comments on commit c2dd5c3

Please sign in to comment.