Skip to content

Commit

Permalink
Merge pull request yahoo#683 from ryankwagner/parseNullsDD
Browse files Browse the repository at this point in the history 
Add logic to accept nulls in DrillDown & log
  • Loading branch information
@panditsurabhi
panditsurabhi authored Jul 21, 2020
2 parents 18dfb04 + 34cd98c commit b5a78af
Show file tree
Hide file tree
Showing 21 changed files with 146 additions and 20 deletions.
2 changes: 1 addition & 1 deletion api-example/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<parent>
<artifactId>maha-parent</artifactId>
<groupId>com.yahoo.maha</groupId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
</parent>

<name>maha api-example</name>
Expand Down
2 changes: 1 addition & 1 deletion api-jersey/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<parent>
<groupId>com.yahoo.maha</groupId>
<artifactId>maha-parent</artifactId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
</parent>

<name>maha api-jersey</name>
Expand Down
2 changes: 1 addition & 1 deletion core/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<parent>
<groupId>com.yahoo.maha</groupId>
<artifactId>maha-parent</artifactId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
</parent>

<name>maha core</name>
Expand Down
2 changes: 1 addition & 1 deletion db/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<parent>
<groupId>com.yahoo.maha</groupId>
<artifactId>maha-parent</artifactId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
</parent>

<name>maha db</name>
Expand Down
2 changes: 1 addition & 1 deletion druid-lookups/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<artifactId>maha-parent</artifactId>
<groupId>com.yahoo.maha</groupId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>

Expand Down
2 changes: 1 addition & 1 deletion druid/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<parent>
<groupId>com.yahoo.maha</groupId>
<artifactId>maha-parent</artifactId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
</parent>

<name>maha druid executor</name>
Expand Down
2 changes: 1 addition & 1 deletion job-service/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<parent>
<groupId>com.yahoo.maha</groupId>
<artifactId>maha-parent</artifactId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
</parent>

<name>maha job service</name>
Expand Down
2 changes: 1 addition & 1 deletion oracle/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<parent>
<groupId>com.yahoo.maha</groupId>
<artifactId>maha-parent</artifactId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
</parent>

<name>maha oracle executor</name>
Expand Down
2 changes: 1 addition & 1 deletion par-request-2/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<parent>
<groupId>com.yahoo.maha</groupId>
<artifactId>maha-parent</artifactId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
</parent>


Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>com.yahoo.maha</groupId>
<artifactId>maha-parent</artifactId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
<packaging>pom</packaging>

<name>maha parent</name>
Expand Down
2 changes: 1 addition & 1 deletion postgres/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<parent>
<groupId>com.yahoo.maha</groupId>
<artifactId>maha-parent</artifactId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
</parent>

<name>maha postgres executor</name>
Expand Down
2 changes: 1 addition & 1 deletion presto/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<parent>
<groupId>com.yahoo.maha</groupId>
<artifactId>maha-parent</artifactId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
</parent>

<name>maha presto executor</name>
Expand Down
2 changes: 1 addition & 1 deletion request-log/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<parent>
<groupId>com.yahoo.maha</groupId>
<artifactId>maha-parent</artifactId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
</parent>

<name>maha request log</name>
Expand Down
2 changes: 1 addition & 1 deletion service/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<parent>
<groupId>com.yahoo.maha</groupId>
<artifactId>maha-parent</artifactId>
<version>5.382-SNAPSHOT</version>
<version>5.383-SNAPSHOT</version>
</parent>

<name>maha service</name>
Expand Down
9 changes: 8 additions & 1 deletion service/src/main/scala/com/yahoo/maha/service/curators/DrilldownCurator.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,14 @@ class DrilldownCurator (override val requestModelValidator: CuratorRequestModelV
val rowList = defaultRequestResult.queryPipelineResult.rowList
var values: Set[String] = Set.empty
rowList.foreach {
row => values = values ++ List(row.cols(row.aliasMap(fieldAlias)).toString)
row =>
val aliasPosition = row.aliasMap(fieldAlias)
if (row.cols(aliasPosition) != null) {
values = values ++ List(row.cols(row.aliasMap(fieldAlias)).toString)
}
else {
logger.error(s"Row has null $fieldAlias (position $aliasPosition)! Found row ${row.cols.mkString("[", ",", "]")}")
}
}

val newReportingRequest = implementDrilldownRequestMinimization(
Expand Down
3 changes: 2 additions & 1 deletion service/src/test/scala/com/yahoo/maha/service/BaseMahaServiceTest.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ trait BaseMahaServiceTest extends FunSuite with Logging {
| "Section ID": 100,
| "Student ID": 213,
| "Class ID": "200",
| "Total Marks": 99
| "Total Marks": 99,
| "Remarks": null
| }
|}]""".stripMargin
Ok(response)
Expand Down
2 changes: 1 addition & 1 deletion service/src/test/scala/com/yahoo/maha/service/RequestCoordinatorTest.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1537,7 +1537,7 @@ class RequestCoordinatorTest extends BaseMahaServiceTest with BeforeAndAfterAll
"selectFields": [
{"field": "Student ID"},
{"field": "Total Marks"},
{"field": "Remarks"},
{"field": "Remarks2"},
{"field": "Class ID"},
{"field": "Performance Factor"}
],
Expand Down
91 changes: 91 additions & 0 deletions service/src/test/scala/com/yahoo/maha/service/curators/DrilldownCuratorTest.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
// Licensed under the terms of the Apache License 2.0. Please see LICENSE file in project root for terms.
package com.yahoo.maha.service.curators

import com.yahoo.maha.core.ColumnInfo
import com.yahoo.maha.core.bucketing.{BucketParams, UserInfo}
import com.yahoo.maha.core.query.Row
import com.yahoo.maha.core.request.ReportingRequest
import com.yahoo.maha.jdbc._
import com.yahoo.maha.parrequest2.GeneralError
Expand All @@ -14,6 +16,8 @@ import org.joda.time.DateTime
import org.joda.time.format.DateTimeFormat
import org.scalatest.BeforeAndAfterAll

import scala.collection.mutable.ArrayBuffer


/**
* Created by pranavbhole on 10/04/18.
Expand Down Expand Up @@ -284,4 +288,91 @@ class DrilldownCuratorTest extends BaseMahaServiceTest with BeforeAndAfterAll {
assert(result.isLeft)
assert(result.left.get.message === "fail")
}

test("Should succeed even when DrillDown metric isn't present") {
class CuratorCustomPostProcessor extends CuratorResultPostProcessor {
override def process(mahaRequestContext: MahaRequestContext, requestResult: RequestResult) : Either[GeneralError, RequestResult] = {
val columns: IndexedSeq[ColumnInfo] = requestResult.queryPipelineResult.queryChain.drivingQuery.queryContext.requestModel.requestCols
val aliasMap : Map[String, Int] = columns.map(_.alias).zipWithIndex.toMap
val data = new ArrayBuffer[Any](initialSize = aliasMap.size)
data+="sid"
data+="cid"
data+="sid"
data+=999
data+=null
val row = Row(aliasMap, data)
requestResult.queryPipelineResult.rowList.addRow(row)
new Right(requestResult)
}
}

val jsonRequest =
s"""
|{
| "cube": "student_performance",
| "curators" : {
| "drilldown" : {
| "config": {
| "dimension": "Remarks",
| "enforceFilters": true
| }
| }
| },
| "selectFields": [
| {"field": "Student ID"},
| {"field": "Class ID"},
| {"field": "Section ID"},
| {"field": "Total Marks"},
| {"field": "Remarks"}
| ],
| "filterExpressions": [
| {"field": "Day", "operator": "between", "from": "$fromDate", "to": "$toDate"},
| {"field": "Student ID", "operator": "=", "value": "555"}
| ]
| }
|""".stripMargin

val reportingRequestResult = ReportingRequest.deserializeSyncWithFactBias(jsonRequest.getBytes, schema = StudentSchema)
require(reportingRequestResult.isSuccess)
val reportingRequest = reportingRequestResult.toOption.get

val bucketParams = BucketParams(UserInfo("uid", true), forceRevision = Some(1))


val mahaRequestContext = MahaRequestContext(REGISTRY,
bucketParams,
reportingRequest,
jsonRequest.getBytes,
Map.empty, "rid", "uid")

val mahaRequestLogHelper = MahaRequestLogHelper(mahaRequestContext, mahaServiceConfig.mahaRequestLogWriter)
val curatorMahaRequestLogHelper = CuratorMahaRequestLogHelper(mahaRequestLogHelper)


val ddCurator = new DrilldownCurator()
val defaultCurator = DefaultCurator(curatorResultPostProcessor = new CuratorCustomPostProcessor)

val curatorInjector = new CuratorInjector(2, mahaService, mahaRequestLogHelper, Set("default"))
val defaultParRequest: Either[CuratorError, ParRequest[CuratorResult]] = defaultCurator
.process(Map.empty, mahaRequestContext, mahaService, curatorMahaRequestLogHelper, NoConfig, curatorInjector)

val parseDDConfig = ddCurator.parseConfig(reportingRequest.curatorJsonConfigMap(DrilldownCurator.name))
assert(parseDDConfig.isSuccess, s"failed : $parseDDConfig")
val ddConfig: DrilldownConfig = parseDDConfig.toOption.get.asInstanceOf[DrilldownConfig]
//assert(totalMetricsConfig.forceRevision === Option(0))
//val curatorInjector = new CuratorInjector(2, mahaService, mahaRequestLogHelper, Set.empty)

val ddCuratorResult: Either[CuratorError, ParRequest[CuratorResult]] = ddCurator
.process(Map("default" -> defaultParRequest), mahaRequestContext, mahaService, curatorMahaRequestLogHelper, ddConfig, curatorInjector)

val queryPipelineResult = ddCuratorResult
.right.get.get().right.get.parRequestResultOption.get.prodRun.get().right.get.queryPipelineResult
var rowCount = 0
queryPipelineResult.rowList.foreach {
row=>
rowCount+=1
}
assert(rowCount == 0)
//Errors here will pan out in Log as: Row has null Remarks (position 4)! Found row
}
}
Loading

0 comments on commit b5a78af

Please sign in to comment.