fix(query): Ignoring query-filter labels with .* regex in the HQE hig…

…her level checks (#1892)

* fix(query): Ignoring query-filter labels with .* regex in the HQE higher level checks

coordinator/src/test/scala/filodb.coordinator/queryplanner/LogicalPlanParserSpec.scala

@@ -762,6 +762,59 @@ class LogicalPlanParserSpec extends AnyFunSpec with Matchers {
     )
   }
 
+  it ("LogicalPlan update for hierarchical queries should ignore .* regex label values in lp update checks") {
+    val t = TimeStepParams(700, 1000, 10000)
+    val nextLevelAggregatedMetricSuffix = "agg_2"
+
+    // with includeTags
+    var nextLevelAggregationTags = Set("aggTag1", "aggTag2", "aggTag3")
+    val includeAggRule = IncludeAggRule(nextLevelAggregatedMetricSuffix, nextLevelAggregationTags)
+    val includeParams = HierarchicalQueryExperienceParams(":::", Map("agg" -> includeAggRule))
+
+    // CASE 1: Should update the metric name since aggTag1/2 is part of include tags and aggTag4 is a .* regex
+    var query = "sum(sum(my_counter:::agg{aggTag1=\"spark\", aggTag2=\"app\", aggTag4=~\".*\"}) by (aggTag1, aggTag2))"
+    var lp = Parser.queryToLogicalPlan(query, t.start, t.step)
+    var lpUpdated = lp.useHigherLevelAggregatedMetric(includeParams)
+    var filterGroups = getColumnFilterGroup(lpUpdated)
+    filterGroups.foreach(
+      filterSet => filterSet.filter(x => x.column == "__name__").head.filter.valuesStrings.head.asInstanceOf[String]
+        .shouldEqual("my_counter:::agg_2")
+    )
+    // CASE 2: Should NOT update since aggTag4 is used in by clause which is not part of include rule
+    query = "sum(sum(my_counter:::agg{aggTag1=\"spark\", aggTag2=\"app\", aggTag4=~\".*\"}) by (aggTag1, aggTag4))"
+    lp = Parser.queryToLogicalPlan(query, t.start, t.step)
+    lpUpdated = lp.useHigherLevelAggregatedMetric(includeParams)
+    filterGroups = getColumnFilterGroup(lpUpdated)
+    filterGroups.foreach(
+      filterSet => filterSet.filter(x => x.column == "__name__").head.filter.valuesStrings.head.asInstanceOf[String]
+        .shouldEqual("my_counter:::agg")
+    )
+
+    // with excludeTags
+    nextLevelAggregationTags = Set("excludeAggTag1", "excludeAggTag2")
+    val excludeAggRule = ExcludeAggRule(nextLevelAggregatedMetricSuffix, nextLevelAggregationTags)
+    val excludeParams = HierarchicalQueryExperienceParams(":::", Map("agg" -> excludeAggRule))
+
+    // CASE 3: should update since excludeTags are only used with .* regex
+    query = "sum by (aggTag1, aggTag2) (sum by (aggTag1, aggTag2) (my_gauge:::agg{aggTag1=\"a\", aggTag2=\"b\", excludeAggTag2=~\".*\"}))"
+    lp = Parser.queryRangeToLogicalPlan(query, t)
+    lpUpdated = lp.useHigherLevelAggregatedMetric(excludeParams)
+    filterGroups = getColumnFilterGroup(lpUpdated)
+    filterGroups.foreach(
+      filterSet => filterSet.filter(x => x.column == "__name__").head.filter.valuesStrings.head.asInstanceOf[String]
+        .shouldEqual("my_gauge:::agg_2")
+    )
+    // CASE 4: should NOT update since excludeTags are only in by clause
+    query = "sum by (aggTag1, excludeAggTag2) (sum by (aggTag1, aggTag2) (my_gauge:::agg{aggTag1=\"a\", aggTag2=\"b\", excludeAggTag2=~\".*\"}))"
+    lp = Parser.queryRangeToLogicalPlan(query, t)
+    lpUpdated = lp.useHigherLevelAggregatedMetric(excludeParams)
+    filterGroups = getColumnFilterGroup(lpUpdated)
+    filterGroups.foreach(
+      filterSet => filterSet.filter(x => x.column == "__name__").head.filter.valuesStrings.head.asInstanceOf[String]
+        .shouldEqual("my_gauge:::agg")
+    )
+  }
+
   it("LogicalPlan update for hierarchical nested aggregation queries") {
     // common parameters using include tags
     val t = TimeStepParams(700, 1000, 10000)

query/src/main/scala/filodb/query/util/HierarchicalQueryExperience.scala

@@ -6,7 +6,7 @@ import scala.jdk.CollectionConverters.asScalaBufferConverter
 
 import filodb.core.GlobalConfig
 import filodb.core.query.ColumnFilter
-import filodb.core.query.Filter.Equals
+import filodb.core.query.Filter.{Equals, EqualsRegex}
 import filodb.query.{AggregateClause, AggregationOperator, LogicalPlan, TsCardinalities}
 
 /**
@@ -236,6 +236,19 @@ object HierarchicalQueryExperience extends StrictLogging {
       case None => false
     }
 
+  /**
+   * @param filters - Seq[ColumnFilter] - label filters of the query/lp
+   * @return - Seq[String] - List of filter tags/labels after filtering out the .* regex. We want to optimize the
+   *         aggregation queries which have .* regex in the filters as they are selecting all the relevant time-series
+   *         and hence not applicable for filter check in the aggregation rules.
+   */
+  def getColumnsAfterFilteringOutDotStarRegexFilters(filters: Seq[ColumnFilter]): Seq[String] = {
+    filters.filter {
+      case ColumnFilter(_, EqualsRegex(value)) if value.toString == ".*" => false
+      case _ => true
+    }.map(x => x.column)
+  }
+
   /**
    * Updates the metric column filter if higher level aggregation is applicable
    * @param params - HierarchicalQueryExperienceParams - Contains metricDelimiter and aggRules
@@ -244,7 +257,7 @@ object HierarchicalQueryExperience extends StrictLogging {
    */
   def upsertMetricColumnFilterIfHigherLevelAggregationApplicable(params: HierarchicalQueryExperienceParams,
                                                                  filters: Seq[ColumnFilter]): Seq[ColumnFilter] = {
-    val filterTags = filters.map(x => x.column)
+    val filterTags = getColumnsAfterFilteringOutDotStarRegexFilters(filters)
     val metricColumnFilter = getMetricColumnFilterTag(filterTags, GlobalConfig.datasetOptions.get.metricColumn)
     val currentMetricName = getMetricName(metricColumnFilter, filters)
     if (currentMetricName.isDefined) {

query/src/test/scala/filodb/query/util/HierarchicalQueryExperienceSpec.scala

@@ -4,9 +4,8 @@ import org.scalatest.funspec.AnyFunSpec
 import org.scalatest.matchers.should.Matchers
 import kamon.Kamon
 import kamon.testkit.InstrumentInspection.Syntax.counterInstrumentInspection
-
 import filodb.core.query.ColumnFilter
-import filodb.core.query.Filter.Equals
+import filodb.core.query.Filter.{Equals, EqualsRegex}
 
 class HierarchicalQueryExperienceSpec extends AnyFunSpec with Matchers {
 
@@ -83,6 +82,36 @@ class HierarchicalQueryExperienceSpec extends AnyFunSpec with Matchers {
       ExcludeAggRule("agg_2", Set("tag3", "tag4")), Seq("tag1", "tag2", "_ws_", "_ns_", "_metric_")) shouldEqual true
   }
 
+  it("getColumnsAfterFilteringOutDotStarRegexFilters should return as expected") {
+    var filters = Seq(
+      ColumnFilter("__name__", Equals("metric1")),
+      ColumnFilter("tag1", Equals("value1")),
+      ColumnFilter("tag2", EqualsRegex("value2.*")),
+      ColumnFilter("tag3", Equals("value3")),
+      ColumnFilter("tag4", EqualsRegex(".*")))
+
+    // should ignore tag4
+    HierarchicalQueryExperience.getColumnsAfterFilteringOutDotStarRegexFilters(filters) shouldEqual
+      Seq("__name__", "tag1", "tag2", "tag3")
+
+    filters = Seq(
+      ColumnFilter("tag1", Equals("value1")),
+      ColumnFilter("tag2", Equals("value2")),
+      ColumnFilter("tag3", EqualsRegex(".*abc")))
+
+    // should not ignore any tags
+    HierarchicalQueryExperience.getColumnsAfterFilteringOutDotStarRegexFilters(filters) shouldEqual
+      Seq("tag1", "tag2", "tag3")
+
+    filters = Seq(
+      ColumnFilter("tag1", EqualsRegex(".*")),
+      ColumnFilter("tag2", EqualsRegex(".*")),
+      ColumnFilter("tag3", EqualsRegex(".*")))
+
+    // should ignore all tags
+    HierarchicalQueryExperience.getColumnsAfterFilteringOutDotStarRegexFilters(filters) shouldEqual Seq()
+  }
+
   it("checkAggregateQueryEligibleForHigherLevelAggregatedMetric should increment counter if metric updated") {
     val excludeRule = ExcludeAggRule("agg_2", Set("notAggTag1", "notAggTag2"))
     val params = HierarchicalQueryExperienceParams(":::", Map("agg" -> excludeRule))