ing-bank / scruid

Scala + Druid: Scruid. A library that allows you to compose queries in Scala, and parse the result back into typesafe classes.
Apache License 2.0
115 stars 29 forks source link

Switch from Lists to Iterables #45

Closed anskarl closed 5 years ago

anskarl commented 5 years ago

Scruid API limits to the use of lists for defining aggregations, intervals, dimensions, etc. For example, a TimeSeriesQuery class in DruidQuery.scala

case class TimeSeriesQuery(
    aggregations: List[Aggregation],
    intervals: List[String],
    filter: Option[Filter] = None,
    granularity: Granularity = GranularityType.Week,
    descending: String = "true",
    postAggregations: List[PostAggregation] = List()
)(implicit val config: DruidConfig = DruidConfig.DefaultConfig)
    extends DruidQuery {
  val queryType          = QueryType.Timeseries
  val dataSource: String = config.datasource
}

Instead of using List in aggregations, intervals and postAggregations, I think that it would be more practical to switch to Iterable, in order to avoid of using Lists or converting to Lists. For example, in the code below all Lists have been changed to Iterables:

case class TimeSeriesQuery(
    aggregations: Iterable[Aggregation],
    intervals: Iterable[String],
    filter: Option[Filter] = None,
    granularity: Granularity = GranularityType.Week,
    descending: String = "true",
    postAggregations: Iterable[PostAggregation] = Iterable.empty
)(implicit val config: DruidConfig = DruidConfig.DefaultConfig)
    extends DruidQuery {
  val queryType          = QueryType.Timeseries
  val dataSource: String = config.datasource
}

That kind of improvement applies to multiple locations in scruid (e.g., ing.wbaa.druid.Having, ing.wbaa.druid.dql.expressions.And, etc.), but the changes seem to be trivial. Also, by switching to Iterables the API backward compatibility is maintained.

anskarl commented 5 years ago

I am closing the issue since the corresponding PR has been merged