Skip to content

Added asNumber to OrderByColumnSpec #940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
flowbehappy wants to merge 2 commits into from
Closed

Added asNumber to OrderByColumnSpec #940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ab3d164
Added asNumber to OrderByColumnSpec, supporting dimension sorting in …
flowbehappy Dec 7, 2014
10b41ba
Add testGroupByWithOrderAsNumber, and DefaultLimitSpec code clean up
flowbehappy Jan 14, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions processing/src/main/java/io/druid/query/groupby/GroupByQuery.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,11 @@ public Builder addOrderByColumn(String dimension, String direction)
return addOrderByColumn(dimension, OrderByColumnSpec.determineDirection(direction));
}

public Builder addOrderByColumn(String dimension, String direction, boolean asNumber)
{
return addOrderByColumn(new OrderByColumnSpec(asNumber, dimension, OrderByColumnSpec.determineDirection(direction)));
}

public Builder addOrderByColumn(String dimension, OrderByColumnSpec.Direction direction)
{
return addOrderByColumn(new OrderByColumnSpec(dimension, direction));
Expand Down
136 changes: 103 additions & 33 deletions processing/src/main/java/io/druid/query/groupby/orderby/DefaultLimitSpec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,29 +115,34 @@ public int compare(Row left, Row right)
}
};

Map<String, Ordering<Row>> possibleOrderings = Maps.newTreeMap(String.CASE_INSENSITIVE_ORDER);
for (DimensionSpec spec : dimensions) {
final String dimension = spec.getOutputName();
possibleOrderings.put(dimension, dimensionOrdering(dimension));
Map<String, Function<OrderByColumnSpec, Ordering<Row>>> possibleOrderDims = Maps.newTreeMap(String.CASE_INSENSITIVE_ORDER);
for (final DimensionSpec spec : dimensions) {
possibleOrderDims.put(
spec.getOutputName(),
dimensionOrdering(spec.getOutputName())
);
}

for (final AggregatorFactory agg : aggs) {
final String column = agg.getName();
possibleOrderings.put(column, metricOrdering(column, agg.getComparator()));
possibleOrderDims.put(
agg.getName(),
metricOrdering(agg.getName(), agg.getComparator())
);
}

for (PostAggregator postAgg : postAggs) {
final String column = postAgg.getName();
possibleOrderings.put(column, metricOrdering(column, postAgg.getComparator()));
for (final PostAggregator postAgg : postAggs) {
possibleOrderDims.put(
postAgg.getName(),
metricOrdering(postAgg.getName(), postAgg.getComparator())
);
}

for (OrderByColumnSpec columnSpec : columns) {
Ordering<Row> nextOrdering = possibleOrderings.get(columnSpec.getDimension());

if (nextOrdering == null) {
Function<OrderByColumnSpec, Ordering<Row>> genOrderingFunc = possibleOrderDims.get(columnSpec.getDimension());
if (genOrderingFunc == null) {
throw new ISE("Unknown column in order clause[%s]", columnSpec);
}

Ordering<Row> nextOrdering = genOrderingFunc.apply(columnSpec);
switch (columnSpec.getDirection()) {
case DESCENDING:
nextOrdering = nextOrdering.reverse();
Expand All @@ -149,35 +154,100 @@ public int compare(Row left, Row right)
return ordering;
}

private Ordering<Row> metricOrdering(final String column, final Comparator comparator)
private Function<OrderByColumnSpec, Ordering<Row>> metricOrdering(final String column, final Comparator comparator)
{
return new Ordering<Row>()
return new Function<OrderByColumnSpec, Ordering<Row>>()
{
@SuppressWarnings("unchecked")
@Override
public int compare(Row left, Row right)
public Ordering<Row> apply(OrderByColumnSpec input)
{
return comparator.compare(left.getRaw(column), right.getRaw(column));

return new Ordering<Row>()
{
@SuppressWarnings("unchecked")
@Override
public int compare(Row left, Row right)
{
return comparator.compare(left.getRaw(column), right.getRaw(column));
}
};
}
};
}

private Ordering<Row> dimensionOrdering(final String dimension)
private Function<OrderByColumnSpec, Ordering<Row>> dimensionOrdering(final String dimension)
{
return Ordering.natural()
.nullsFirst()
.onResultOf(
new Function<Row, String>()
{
@Override
public String apply(Row input)
{
// Multi-value dimensions have all been flattened at this point;
final List<String> dimList = input.getDimension(dimension);
return dimList.isEmpty() ? null : dimList.get(0);
}
}
);
return new Function<OrderByColumnSpec, Ordering<Row>>()
{
@Override
public Ordering<Row> apply(OrderByColumnSpec columnSpec)
{
if (columnSpec.asNumber()) {
// Order: null < string < number , and number only means "long" here.
return new Ordering<Comparable>()
{
@Override
public int compare(
Comparable left, Comparable right
)
{
if (left instanceof Long && right instanceof Long) {
return ((Long) left).compareTo((Long) right);
} else if (left instanceof String) {
return -1;
} else if (right instanceof String) {
return 1;
} else {
return ((String) left).compareTo((String) right);
}
}
}.nullsFirst().onResultOf(
new Function<Row, Comparable>()
{
@Override
public Comparable apply(Row input)
{
// Multi-value dimensions have all been flattened at this point;
final List<String> dimList = input.getDimension(dimension.toLowerCase());
if (dimList.isEmpty()) {
return null;
}

String dimValue = dimList.get(0);
// Make sure dimValue is number parsable.
if (dimValue.isEmpty()) {
return dimValue;
}
for (int i = 0; i < dimValue.length(); i++) {
Copy link
Copy Markdown
Contributor

@himanshug himanshug Mar 3, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not seem too hard to make this feature complete by adding support for doubles as well.
We can use a regex here instead to detect if its a double value or long value or something else. Accordingly use Double.parseDouble(..) or Long.parseLong(..) .
And in the compare above add another case to handle Doubles.
Also, you can refactor this code a little bit by having a private method like
private Comparable getDimensionValue() {

}
and calling return getDimensionValue(dimList.get(0));

if (!Character.isDigit(dimValue.charAt(i))) {
return dimValue;
}
}
try {
return Long.parseLong(dimValue);
}
catch (NumberFormatException e) {
return dimList.get(0);
}
}
}
);
} else {
return Ordering.natural().nullsFirst().onResultOf(
new Function<Row, String>()
{
@Override
public String apply(Row input)
{
// Multi-value dimensions have all been flattened at this point;
final List<String> dimList = input.getDimension(dimension.toLowerCase());
return dimList.isEmpty() ? null : dimList.get(0);
}
}
);
}
}
};
}

@Override
Expand Down
27 changes: 25 additions & 2 deletions processing/src/main/java/io/druid/query/groupby/orderby/OrderByColumnSpec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public static enum Direction
stupidEnumMap = bob.build();
}

private final boolean asNumber;
private final String dimension;
private final Direction direction;

Expand All @@ -68,14 +69,20 @@ public static OrderByColumnSpec create(Object obj)
Preconditions.checkNotNull(obj, "Cannot build an OrderByColumnSpec from a null object.");

if (obj instanceof String) {
return new OrderByColumnSpec(obj.toString(), null);
return new OrderByColumnSpec(false, obj.toString(), null);
} else if (obj instanceof Map) {
final Map map = (Map) obj;

final boolean asNumber;
if (map.get("asNumber") instanceof Boolean) {
asNumber = (Boolean) map.get("asNumber");
} else {
asNumber = false;
}
final String dimension = map.get("dimension").toString();
final Direction direction = determineDirection(map.get("direction"));

return new OrderByColumnSpec(dimension, direction);
return new OrderByColumnSpec(asNumber, dimension, direction);
} else {
throw new ISE("Cannot build an OrderByColumnSpec from a %s", obj.getClass());
}
Expand Down Expand Up @@ -126,10 +133,26 @@ public OrderByColumnSpec(
Direction direction
)
{
this(false, dimension, direction);
}

public OrderByColumnSpec(
boolean asNumber,
String dimension,
Direction direction
)
{
this.asNumber = asNumber;
this.dimension = dimension;
this.direction = direction == null ? Direction.ASCENDING : direction;
}

@JsonProperty
public boolean asNumber()
{
return asNumber;
}

@JsonProperty
public String getDimension()
{
Expand Down
1 change: 1 addition & 0 deletions processing/src/test/java/io/druid/query/QueryRunnerTestHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public TableDataSource apply(@Nullable String input)
public static final String qualityDimension = "quality";
public static final String placementDimension = "placement";
public static final String placementishDimension = "placementish";
public static final String idDimension = "id";
public static final String indexMetric = "index";
public static final String uniqueMetric = "uniques";
public static final String addRowsIndexConstantMetric = "addRowsIndexConstant";
Expand Down
40 changes: 39 additions & 1 deletion processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ public boolean isSingleThreaded()
pool
);


Function<Object, Object> function = new Function<Object, Object>()
{
@Override
Expand Down Expand Up @@ -1072,6 +1071,45 @@ public void testGroupByWithOrderLimit4()
TestHelper.assertExpectedObjects(expectedResults, results, "order-limit");
}

@Test
public void testGroupByWithOrderAsNumber() throws Exception
{
GroupByQuery.Builder builder = GroupByQuery
.builder()
.setDataSource(QueryRunnerTestHelper.dataSource)
.setInterval("2011-04-14/2011-04-16")
.setDimensions(Lists.<DimensionSpec>newArrayList(new DefaultDimensionSpec("id", "id")))
.setAggregatorSpecs(
Arrays.<AggregatorFactory>asList(
QueryRunnerTestHelper.rowsCount,
new LongSumAggregatorFactory("idx", "index")
)
)
.addOrderByColumn("id", "desc", true)
.setGranularity(new PeriodGranularity(new Period("P1M"), null, null));

final GroupByQuery query = builder.build();

List<Row> expectedResults = Arrays.asList(
GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "id", "2084", "rows", 1L, "idx", 753L),
GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "id", "100", "rows", 1L, "idx", 1029L),
GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "id", "12", "rows", 2L, "idx", 1083L),
GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "id", "3", "rows", 3L, "idx", 337L),
GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "id", "0", "rows", 1L, "idx", 962L),
GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "id", "default", "rows", 13L, "idx", 3081L),
GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "id", "c1", "rows", 1L, "idx", 94L),
GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "id", "b8", "rows", 1L, "idx", 135L),
GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "id", "23b", "rows", 3L, "idx", 2150L)
);

Map<String, Object> context = Maps.newHashMap();
QueryRunner<Row> mergeRunner = factory.getToolchest().mergeResults(runner);
TestHelper.assertExpectedObjects(expectedResults, mergeRunner.run(query, context), "no-limit");
TestHelper.assertExpectedObjects(
Iterables.limit(expectedResults, 5), mergeRunner.run(builder.limit(5).build(), context), "limited"
);
}

@Test
public void testHavingSpec()
{
Expand Down
4 changes: 1 addition & 3 deletions processing/src/test/java/io/druid/query/metadata/SegmentMetadataQueryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import io.druid.jackson.DefaultObjectMapper;
import io.druid.query.Druids;
import io.druid.query.Query;
import io.druid.query.QueryConfig;
import io.druid.query.QueryRunner;
import io.druid.query.QueryRunnerFactory;
import io.druid.query.QueryRunnerTestHelper;
Expand All @@ -42,7 +41,6 @@

import java.util.Arrays;
import java.util.HashMap;
import java.util.List;

public class SegmentMetadataQueryTest
{
Expand Down Expand Up @@ -82,7 +80,7 @@ public void testSegmentMetadataQuery()
);
SegmentAnalysis val = results.iterator().next();
Assert.assertEquals("testSegment", val.getId());
Assert.assertEquals(69843, val.getSize());
Assert.assertEquals(79452, val.getSize());
Assert.assertEquals(
Arrays.asList(new Interval("2011-01-12T00:00:00.000Z/2011-04-15T00:00:00.001Z")),
val.getIntervals()
Expand Down
1 change: 1 addition & 0 deletions processing/src/test/java/io/druid/query/search/SearchQueryRunnerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public void testSearch()
);
expectedResults.put(QueryRunnerTestHelper.marketDimension, Sets.newHashSet("total_market"));
expectedResults.put(QueryRunnerTestHelper.placementishDimension, Sets.newHashSet("a"));
expectedResults.put(QueryRunnerTestHelper.idDimension, Sets.newHashSet("default"));

checkSearchQuery(searchQuery, expectedResults);
}
Expand Down
4 changes: 3 additions & 1 deletion processing/src/test/java/io/druid/segment/TestIndex.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.common.io.InputSupplier;
import com.google.common.io.LineProcessor;
import com.metamx.common.logger.Logger;
import io.druid.data.input.InputRow;
import io.druid.data.input.impl.DelimitedParseSpec;
import io.druid.data.input.impl.DimensionsSpec;
import io.druid.data.input.impl.StringInputRowParser;
Expand Down Expand Up @@ -57,14 +58,15 @@ public class TestIndex
{
public static final String[] COLUMNS = new String[]{
"ts",
"id",
"market",
"quality",
"placement",
"placementish",
"index",
"quality_uniques"
};
public static final String[] DIMENSIONS = new String[]{"market", "quality", "placement", "placementish"};
public static final String[] DIMENSIONS = new String[]{"id", "market", "quality", "placement", "placementish"};
public static final String[] METRICS = new String[]{"index"};
private static final Logger log = new Logger(TestIndex.class);
private static final Interval DATA_INTERVAL = new Interval("2011-01-12T00:00:00.000Z/2011-05-01T00:00:00.000Z");
Expand Down
Loading