Merge pull request #645 from ishan8/fix-filter-aggregated-fact

enable filtering on aggregated facts not included in output columns
yahoo · May 14, 2020 · d6b48f2 · d6b48f2
2 parents f5be886 + 258dbad
commit d6b48f2
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 2 deletions.
diff --git a/core/src/main/scala/com/yahoo/maha/core/query/druid/DruidQueryGenerator.scala b/core/src/main/scala/com/yahoo/maha/core/query/druid/DruidQueryGenerator.scala
@@ -1140,8 +1140,7 @@ class DruidQueryGenerator(queryOptimizer: DruidQueryOptimizer
     }
 
     val factCols = queryContext.factBestCandidate.factColMapping.toList.collect {
-      case (nonFkCol, alias) if queryContext.factBestCandidate.requestCols(nonFkCol) =>
-        (fact.columnsByNameMap(nonFkCol), alias)
+      case (nonFkCol, alias) => (fact.columnsByNameMap(nonFkCol), alias)
     }
 
     //render derived columns last

diff --git a/core/src/test/scala/com/yahoo/maha/core/query/druid/DruidQueryGeneratorTest.scala b/core/src/test/scala/com/yahoo/maha/core/query/druid/DruidQueryGeneratorTest.scala
@@ -3327,5 +3327,32 @@ class DruidQueryGeneratorTest extends BaseDruidQueryGeneratorTest {
     val json = """\{"queryType":"groupBy","dataSource":\{"type":"table","name":"fact1"\},"intervals":\{"type":"intervals","intervals":\[".*"\]\},"virtualColumns":\[\],"filter":\{"type":"and","fields":\[\{"type":"selector","dimension":"statsDate","value":".*"\},\{"type":"selector","dimension":"advertiser_id","value":"12345"\}\]\},"granularity":\{"type":"all"\},"dimensions":\[\{"type":"extraction","dimension":"__time","outputName":"Week Start","outputType":"STRING","extractionFn":\{"type":"timeFormat","format":"yyyy-MM-dd","timeZone":"UTC","granularity":"WEEK","asMillis":false\}\},\{"type":"default","dimension":"id","outputName":"Keyword ID","outputType":"STRING"\}\],"aggregations":\[\{"type":"longSum","name":"Impressions","fieldName":"impressions"\}\],"postAggregations":\[\],"limitSpec":\{"type":"default","columns":\[\],"limit":120\},"context":\{"applyLimitPushDown":"false","userId":"someUser","uncoveredIntervalsLimit":1,"groupByIsSingleThreaded":true,"timeout":5000,"queryId":"abc123"\},"descending":false\}"""
     result should fullyMatch regex json
   }
+
+  test("Successfully generate a filter on an aggregated fact col even when the fact is not in the selected fields") {
+    val jsonString = s"""{
+                      "cube": "k_stats",
+                      "selectFields": [
+                        {"field": "Keyword ID"},
+                        {"field": "Keyword Value"},
+                        {"field": "Clicks"}
+                      ],
+                      "filterExpressions": [
+                        {"field": "Day", "operator": "=", "value": "$fromDate"},
+                        {"field": "Advertiser ID", "operator": "=", "value": "12345"},
+                        {"field": "Impressions", "operator": ">", "value": "5"}
+                      ],
+                      "paginationStartIndex":20,
+                      "rowsPerPage":100
+                    }"""
+
+    val request: ReportingRequest = getReportingRequestSyncWithAdditionalParameters(jsonString, RequestContext("abc123", "someUser"))
+    val requestModel = RequestModel.from(request, defaultRegistry)
+    val queryPipelineTry = generatePipeline(requestModel.toOption.get)
+    assert(queryPipelineTry.isSuccess, queryPipelineTry.errorMessage("Fail to get the query pipeline"))
+    val result = queryPipelineTry.toOption.get.queryChain.drivingQuery.asInstanceOf[DruidQuery[_]].asString
+    println(result)
+    val json = """\{"queryType":"groupBy","dataSource":\{"type":"table","name":"fact1"\},"intervals":\{"type":"intervals","intervals":\[".*"\]\},"virtualColumns":\[\],"filter":\{"type":"and","fields":\[\{"type":"selector","dimension":"statsDate","value":".*"\},\{"type":"selector","dimension":"advertiser_id","value":"12345"\}\]\},"granularity":\{"type":"all"\},"dimensions":\[\{"type":"default","dimension":"id","outputName":"Keyword ID","outputType":"STRING"\}\],"aggregations":\[\{"type":"longSum","name":"Clicks","fieldName":"clicks"\},\{"type":"longSum","name":"Impressions","fieldName":"impressions"\}\],"postAggregations":\[\],"having":\{"type":"and","havingSpecs":\[\{"type":"greaterThan","aggregation":"Impressions","value":5\}\]\},"limitSpec":\{"type":"default","columns":\[\],"limit":120\},"context":\{"applyLimitPushDown":"false","userId":"someUser","uncoveredIntervalsLimit":1,"groupByIsSingleThreaded":true,"timeout":5000,"queryId":"abc123"\},"descending":false\}"""
+    result should fullyMatch regex json
+  }
 }