Skip to content

Commit

Permalink
Modified according to mike and rick's comments
Browse files Browse the repository at this point in the history
  • Loading branch information
garyluoex committed Mar 14, 2017
1 parent 1885c3e commit 6e94d2c
Show file tree
Hide file tree
Showing 14 changed files with 52 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class AggregatableDimensionsMatcher implements PhysicalTableMatcher {
/**
* Constructor saves metrics, dimensions, coarsest time grain, and logical table name (for logging).
*
* @param requestConstraints contains the request constraints extracted from DataApiRequest and TemplateDruidQuery
* @param requestConstraints Contains the request constraints extracted from DataApiRequest and TemplateDruidQuery
*/
public AggregatableDimensionsMatcher(QueryPlanningConstraint requestConstraints) {
this.requestConstraints = requestConstraints;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@
package com.yahoo.bard.webservice.table.resolver;

import com.yahoo.bard.webservice.application.MetricRegistryFactory;
import com.yahoo.bard.webservice.data.metric.TemplateDruidQuery;
import com.yahoo.bard.webservice.druid.model.query.Granularity;
import com.yahoo.bard.webservice.table.PhysicalTable;
import com.yahoo.bard.webservice.web.DataApiRequest;
import com.yahoo.bard.webservice.web.ErrorMessageFormat;

import com.codahale.metrics.MetricRegistry;
Expand All @@ -18,7 +15,6 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.BinaryOperator;

/**
Expand All @@ -29,15 +25,6 @@ public abstract class BasePhysicalTableResolver implements PhysicalTableResolver
private static final Logger LOG = LoggerFactory.getLogger(BasePhysicalTableResolver.class);
private static final MetricRegistry REGISTRY = MetricRegistryFactory.getRegistry();

protected final BiFunction<DataApiRequest, TemplateDruidQuery, Granularity> resolveAcceptingGrain;

/**
* Constructor.
*/
public BasePhysicalTableResolver() {
this.resolveAcceptingGrain = new RequestQueryGranularityResolver();
}

/**
* Create a list of matchers based on a request and query.
*
Expand Down Expand Up @@ -101,50 +88,30 @@ public PhysicalTable resolve(
) throws NoMatchFoundException {

// Minimum grain at which the request can be aggregated from
Granularity minimumTableTimeGrain = requestConstraints.getMinimumGranularity();
Set<String> columnNames = requestConstraints.getAllColumnNames();
LOG.trace(
"Resolving Table using TimeGrain: {}, dimension API names: {} and TableGroup: {}",
minimumTableTimeGrain,
columnNames,
requestConstraints.getMinimumGranularity(),
requestConstraints.getAllColumnNames(),
candidateTables
);

try {
Set<PhysicalTable> physicalTables = filter(candidateTables, requestConstraints);

BinaryOperator<PhysicalTable> betterTable = getBetterTableOperator(requestConstraints);
PhysicalTable bestTable = physicalTables.stream().reduce(betterTable).get();
PhysicalTable bestTable = physicalTables.stream()
.reduce(getBetterTableOperator(requestConstraints)).get();

REGISTRY.meter("request.physical.table." + bestTable.getName() + "." + bestTable.getTimeGrain()).mark();
LOG.trace("Found best Table: {}", bestTable);
return bestTable;
} catch (NoMatchFoundException me) {
// Blow up if we couldn't match a table, log and return if we can
logMatchException(requestConstraints, minimumTableTimeGrain);
LOG.error(ErrorMessageFormat.NO_PHYSICAL_TABLE_MATCHED.logFormat(
requestConstraints.getAllDimensionNames(),
requestConstraints.getLogicalMetricNames(),
requestConstraints.getMinimumGranularity()
));
throw me;
}
}

/**
* Log out inability to find a matching table.
*
* @param requestConstraints contains the request constraints extracted from DataApiRequest and TemplateDruidQuery
* @param minimumTableTimeGrain Minimum grain that we needed to meet
*/
public void logMatchException(
QueryPlanningConstraint requestConstraints,
Granularity minimumTableTimeGrain
) {
// Get the dimensions and metrics as lists of names
Set<String> requestDimensionNames = requestConstraints.getAllDimensionNames();
Set<String> requestMetricNames = requestConstraints.getLogicalMetricNames();

String msg = ErrorMessageFormat.NO_PHYSICAL_TABLE_MATCHED.logFormat(
requestDimensionNames,
requestMetricNames,
minimumTableTimeGrain
);
LOG.error(msg);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.yahoo.bard.webservice.web.ApiFilter;
import com.yahoo.bard.webservice.web.DataApiRequest;

import java.util.Collections;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
Expand All @@ -23,6 +24,9 @@ public class DataSourceConstraint {
private final Set<Dimension> metricDimensions;
private final Set<String> metricNames;
private final Map<Dimension, Set<ApiFilter>> apiFilters;
private final Set<Dimension> allDimensions;
private final Set<String> allDimensionNames;
private final Set<String> allColumnNames;

/**
* Constructor.
Expand All @@ -31,21 +35,25 @@ public class DataSourceConstraint {
* @param templateDruidQuery Query containing metric constraint information.
*/
public DataSourceConstraint(DataApiRequest dataApiRequest, DruidAggregationQuery<?> templateDruidQuery) {
this.requestDimensions = dataApiRequest.getDimensions();
this.filterDimensions = dataApiRequest.getFilterDimensions();
this.metricDimensions = templateDruidQuery.getMetricDimensions();
this.metricNames = templateDruidQuery.getDependentFieldNames();
this.apiFilters = dataApiRequest.getFilters();
this.requestDimensions = Collections.unmodifiableSet(dataApiRequest.getDimensions());
this.filterDimensions = Collections.unmodifiableSet(dataApiRequest.getFilterDimensions());
this.metricDimensions = Collections.unmodifiableSet(templateDruidQuery.getMetricDimensions());
this.metricNames = Collections.unmodifiableSet(templateDruidQuery.getDependentFieldNames());
this.apiFilters = Collections.unmodifiableMap(dataApiRequest.getFilters());
this.allDimensions = Stream.of(
getRequestDimensions().stream(),
getFilterDimensions().stream(),
getMetricDimensions().stream()
).flatMap(Function.identity()).collect(Collectors.toSet());
this.allDimensionNames = allDimensions.stream().map(Dimension::getApiName).collect(Collectors.toSet());
this.allColumnNames = Stream.concat(allDimensionNames.stream(), metricNames.stream())
.collect(Collectors.toSet());
}

public Set<Dimension> getRequestDimensions() {
return requestDimensions;
}

public Set<String> getRequestDimensionNames() {
return getRequestDimensions().stream().map(Dimension::getApiName).collect(Collectors.toSet());
}

public Set<Dimension> getFilterDimensions() {
return filterDimensions;
}
Expand All @@ -59,22 +67,15 @@ public Set<String> getMetricNames() {
}

public Set<Dimension> getAllDimensions() {
return Stream.of(
getRequestDimensions().stream(),
getFilterDimensions().stream(),
getMetricDimensions().stream()
).flatMap(Function.identity()).collect(Collectors.toSet());
return allDimensions;
}

public Set<String> getAllDimensionNames() {
return getAllDimensions().stream().map(Dimension::getApiName).collect(Collectors.toSet());
return allDimensionNames;
}

public Set<String> getAllColumnNames() {
return Stream.of(
getAllDimensionNames().stream(),
getMetricNames().stream()
).flatMap(Function.identity()).collect(Collectors.toSet());
return allColumnNames;
}

public Map<Dimension, Set<ApiFilter>> getApiFilters() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,11 @@ public DefaultPhysicalTableResolver(

@Override
public List<PhysicalTableMatcher> getMatchers(QueryPlanningConstraint requestConstraints) {
SchemaPhysicalTableMatcher schemaMatcher =
new SchemaPhysicalTableMatcher(requestConstraints);

TimeAlignmentPhysicalTableMatcher timeAlignmentMatcher =
new TimeAlignmentPhysicalTableMatcher(requestConstraints);

AggregatableDimensionsMatcher aggregatabilityMatcher =
new AggregatableDimensionsMatcher(requestConstraints);

return Arrays.asList(schemaMatcher, aggregatabilityMatcher, timeAlignmentMatcher);
return Arrays.asList(
new SchemaPhysicalTableMatcher(requestConstraints),
new AggregatableDimensionsMatcher(requestConstraints),
new TimeAlignmentPhysicalTableMatcher(requestConstraints)
);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public interface PhysicalTableResolver {
* Choose the best fit Physical Table from a table group.
*
* @param candidateTables The tables being considered for match
* @param requestConstraints contains the request constraints extracted from DataApiRequest and TemplateDruidQuery
* @param requestConstraints Contains the request constraints extracted from DataApiRequest and TemplateDruidQuery
*
* @return The table, if any, that satisfies all criteria and best matches the query
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.joda.time.Interval;

import java.util.Collections;
import java.util.Set;
import java.util.stream.Collectors;

Expand All @@ -23,6 +24,7 @@ public class QueryPlanningConstraint extends DataSourceConstraint {
private final Set<LogicalMetric> logicalMetrics;
private final Granularity minimumGranularity;
private final Granularity requestGranularity;
private final Set<String> logicalMetricNames;

/**
* Constructor.
Expand All @@ -34,10 +36,11 @@ public QueryPlanningConstraint(DataApiRequest dataApiRequest, TemplateDruidQuery
super(dataApiRequest, templateDruidQuery);

this.logicalTable = dataApiRequest.getTable();
this.intervals = dataApiRequest.getIntervals();
this.intervals = Collections.unmodifiableSet(dataApiRequest.getIntervals());
this.logicalMetrics = Collections.unmodifiableSet(dataApiRequest.getLogicalMetrics());
this.minimumGranularity = new RequestQueryGranularityResolver().apply(dataApiRequest, templateDruidQuery);
this.requestGranularity = dataApiRequest.getGranularity();
this.logicalMetrics = dataApiRequest.getLogicalMetrics();
this.logicalMetricNames = logicalMetrics.stream().map(LogicalMetric::getName).collect(Collectors.toSet());
}

public LogicalTable getLogicalTable() {
Expand All @@ -53,7 +56,7 @@ public Set<LogicalMetric> getLogicalMetrics() {
}

public Set<String> getLogicalMetricNames() {
return getLogicalMetrics().stream().map(LogicalMetric::getName).collect(Collectors.toSet());
return logicalMetricNames;
}

public Granularity getMinimumGranularity() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public boolean test(PhysicalTable table) {
public NoMatchFoundException noneFoundException() {
String logicalTableName = requestConstraints.getLogicalTable().getName();
Set<String> logicalMetrics = requestConstraints.getLogicalMetricNames();
Set<String> dimensions = requestConstraints.getRequestDimensionNames();
Set<String> dimensions = requestConstraints.getAllDimensionNames();
String grainName = requestConstraints.getMinimumGranularity().getName();
LOG.error(
MESSAGE_FORMAT.logFormat(logicalTableName, dimensions, logicalMetrics, grainName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public class TimeAlignmentPhysicalTableMatcher implements PhysicalTableMatcher {
* Stores the request table name and intervals and creates a predicate to test a physical table based on request
* intervals.
*
* @param requestConstraints contains the request constraints extracted from DataApiRequest and TemplateDruidQuery
* @param requestConstraints Contains the request constraints extracted from DataApiRequest and TemplateDruidQuery
*/
public TimeAlignmentPhysicalTableMatcher(QueryPlanningConstraint requestConstraints) {
if (requestConstraints.getIntervals().isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class VolatileTimeComparator implements Comparator<PhysicalTable> {
/**
* Builds a table comparator that compares tables based on how much data there is in their volatile intervals.
*
* @param requestConstraints contains the request constraints extracted from DataApiRequest and TemplateDruidQuery
* @param requestConstraints Contains the request constraints extracted from DataApiRequest and TemplateDruidQuery
* @param partialDataHandler A service for computing partial data information
* @param volatileIntervalsService A service to extract the intervals in a query that are volatile with respect
* to a given table
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class DimensionToDefaultDimensionSpecSpec extends Specification {
apiRequest.getTopN() >> OptionalInt.empty()
apiRequest.getSorts() >> ([])
apiRequest.getCount() >> OptionalInt.empty()
apiRequest.getFilters() >> Collections.emptyMap()
}

def "Serialize to apiName when apiName and physicalName of a dimension is the same"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class LookupDimensionToDimensionSpecSpec extends Specification{
apiRequest.getTopN() >> OptionalInt.empty()
apiRequest.getSorts() >> ([])
apiRequest.getCount() >> OptionalInt.empty()
apiRequest.getFilters() >> Collections.emptyMap()
}

def "Given lookup dimension with no namespace serialize using dimension serializer"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class RegisteredLookupDimensionToDimensionSpecSpec extends Specification{
apiRequest.getTopN() >> OptionalInt.empty()
apiRequest.getSorts() >> ([])
apiRequest.getCount() >> OptionalInt.empty()
apiRequest.getFilters() >> Collections.emptyMap()
}

def "Given registered lookup dimension with no lookup serialize using dimension serializer"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ class DefaultPhysicalTableResolverSpec extends Specification {
apiRequest.granularity >> granularity
apiRequest.filterDimensions >> prototype['filterDimensions']
apiRequest.logicalMetrics >> prototype['logicalMetrics']
apiRequest.getFilters() >> Collections.emptyMap()
return apiRequest
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ class SchemaPhysicalTableMatcherSpec extends Specification {
query.getDependentFieldNames() >> ([] as Set)
request.getFilterDimensions() >> []
request.getDimensions() >> (dimSet)
request.getFilters() >> Collections.emptyMap()
request.getIntervals() >> []
request.getLogicalMetrics() >> []

dimensionDictionary = new DimensionDictionary(dimSet)
schemaPhysicalTableMatcher = new SchemaPhysicalTableMatcher(
Expand Down

0 comments on commit 6e94d2c

Please sign in to comment.