From 2b5ce895543f9782399162b7ce137107adf95bc2 Mon Sep 17 00:00:00 2001 From: fjy Date: Mon, 29 Sep 2014 12:07:27 -0700 Subject: [PATCH 1/2] fix spatial indexing with multi spatial dims --- .../SpatialDimensionRowFormatter.java | 61 +++++------- .../segment/filter/SpatialFilterTest.java | 94 ++++++++++++++++++- 2 files changed, 118 insertions(+), 37 deletions(-) diff --git a/processing/src/main/java/io/druid/segment/incremental/SpatialDimensionRowFormatter.java b/processing/src/main/java/io/druid/segment/incremental/SpatialDimensionRowFormatter.java index a9bec20d29c..1d624dfb75d 100644 --- a/processing/src/main/java/io/druid/segment/incremental/SpatialDimensionRowFormatter.java +++ b/processing/src/main/java/io/druid/segment/incremental/SpatialDimensionRowFormatter.java @@ -49,26 +49,15 @@ public class SpatialDimensionRowFormatter private static final Joiner JOINER = Joiner.on(","); private static final Splitter SPLITTER = Splitter.on(","); - private final List spatialDimensions; - private final Set spatialDimNames; + private final Map spatialDimensionMap; private final Set spatialPartialDimNames; public SpatialDimensionRowFormatter(List spatialDimensions) { - this.spatialDimensions = spatialDimensions; - this.spatialDimNames = Sets.newHashSet( - Lists.transform( - spatialDimensions, - new Function() - { - @Override - public String apply(SpatialDimensionSchema input) - { - return input.getDimName(); - } - } - ) - ); + this.spatialDimensionMap = Maps.newHashMap(); + for (SpatialDimensionSchema spatialDimension : spatialDimensions) { + this.spatialDimensionMap.put(spatialDimension.getDimName(), spatialDimension); + } this.spatialPartialDimNames = Sets.newHashSet( Iterables.concat( Lists.transform( @@ -110,7 +99,7 @@ public class SpatialDimensionRowFormatter @Override public boolean apply(String input) { - return !spatialDimNames.contains(input) && !spatialPartialDimNames.contains(input); + return !spatialDimensionMap.containsKey(input) && !spatialPartialDimNames.contains(input); } } ) @@ -173,32 +162,32 @@ public class SpatialDimensionRowFormatter } }; - if (!spatialPartialDimNames.isEmpty()) { - for (SpatialDimensionSchema spatialDimension : spatialDimensions) { - List spatialDimVals = Lists.newArrayList(); + for (Map.Entry entry : spatialDimensionMap.entrySet()) { + final String spatialDimName = entry.getKey(); + final SpatialDimensionSchema spatialDim = entry.getValue(); - for (String partialSpatialDim : spatialDimension.getDims()) { - List dimVals = row.getDimension(partialSpatialDim); - if (isSpatialDimValsValid(dimVals)) { - spatialDimVals.addAll(dimVals); - } - } - - if (spatialDimVals.size() == spatialPartialDimNames.size()) { - spatialLookup.put(spatialDimension.getDimName(), Arrays.asList(JOINER.join(spatialDimVals))); - finalDims.add(spatialDimension.getDimName()); - } - } - } else { - for (String spatialDimName : spatialDimNames) { - List dimVals = row.getDimension(spatialDimName); + List dimVals = row.getDimension(spatialDimName); + if (dimVals != null && !dimVals.isEmpty()) { if (dimVals.size() != 1) { - throw new ISE("Cannot have a spatial dimension value with size[%d]", dimVals.size()); + throw new ISE("Spatial dimension value must be in an array!"); } if (isJoinedSpatialDimValValid(dimVals.get(0))) { spatialLookup.put(spatialDimName, dimVals); finalDims.add(spatialDimName); } + } else { + List spatialDimVals = Lists.newArrayList(); + for (String dim : spatialDim.getDims()) { + List partialDimVals = row.getDimension(dim); + if (isSpatialDimValsValid(partialDimVals)) { + spatialDimVals.addAll(partialDimVals); + } + } + + if (spatialDimVals.size() == spatialDim.getDims().size()) { + spatialLookup.put(spatialDimName, Arrays.asList(JOINER.join(spatialDimVals))); + finalDims.add(spatialDimName); + } } } diff --git a/processing/src/test/java/io/druid/segment/filter/SpatialFilterTest.java b/processing/src/test/java/io/druid/segment/filter/SpatialFilterTest.java index 84df58a260d..75fc85f5587 100644 --- a/processing/src/test/java/io/druid/segment/filter/SpatialFilterTest.java +++ b/processing/src/test/java/io/druid/segment/filter/SpatialFilterTest.java @@ -76,7 +76,7 @@ public class SpatialFilterTest new LongSumAggregatorFactory("val", "val") }; - private static List DIMS = Lists.newArrayList("dim", "lat", "long"); + private static List DIMS = Lists.newArrayList("dim", "lat", "long", "lat2", "long2"); @Parameterized.Parameters public static Collection constructorFeeder() throws IOException @@ -110,6 +110,10 @@ public class SpatialFilterTest new SpatialDimensionSchema( "dim.geo", Arrays.asList("lat", "long") + ), + new SpatialDimensionSchema( + "spatialIsRad", + Arrays.asList("lat2", "long2") ) ) ).build() @@ -204,6 +208,18 @@ public class SpatialFilterTest ) ) ); + theIndex.add( + new MapBasedInputRow( + new DateTime("2013-01-05").getMillis(), + DIMS, + ImmutableMap.of( + "timestamp", new DateTime("2013-01-05").toString(), + "lat2", 0.0f, + "long2", 0.0f, + "val", 13l + ) + ) + ); // Add a bunch of random points Random rand = new Random(); @@ -250,6 +266,10 @@ public class SpatialFilterTest new SpatialDimensionSchema( "dim.geo", Arrays.asList("lat", "long") + ), + new SpatialDimensionSchema( + "spatialIsRad", + Arrays.asList("lat2", "long2") ) ) ).build() @@ -263,6 +283,10 @@ public class SpatialFilterTest new SpatialDimensionSchema( "dim.geo", Arrays.asList("lat", "long") + ), + new SpatialDimensionSchema( + "spatialIsRad", + Arrays.asList("lat2", "long2") ) ) ).build() @@ -276,6 +300,10 @@ public class SpatialFilterTest new SpatialDimensionSchema( "dim.geo", Arrays.asList("lat", "long") + ), + new SpatialDimensionSchema( + "spatialIsRad", + Arrays.asList("lat2", "long2") ) ) ).build() @@ -372,6 +400,18 @@ public class SpatialFilterTest ) ) ); + second.add( + new MapBasedInputRow( + new DateTime("2013-01-05").getMillis(), + DIMS, + ImmutableMap.of( + "timestamp", new DateTime("2013-01-05").toString(), + "lat2", 0.0f, + "long2", 0.0f, + "val", 13l + ) + ) + ); // Add a bunch of random points Random rand = new Random(); @@ -486,6 +526,58 @@ public class SpatialFilterTest } } + + @Test + public void testSpatialQueryWithOtherSpatialDim() + { + TimeseriesQuery query = Druids.newTimeseriesQueryBuilder() + .dataSource("test") + .granularity(QueryGranularity.ALL) + .intervals(Arrays.asList(new Interval("2013-01-01/2013-01-07"))) + .filters( + new SpatialDimFilter( + "spatialIsRad", + new RadiusBound(new float[]{0.0f, 0.0f}, 5) + ) + ) + .aggregators( + Arrays.asList( + new CountAggregatorFactory("rows"), + new LongSumAggregatorFactory("val", "val") + ) + ) + .build(); + + List> expectedResults = Arrays.asList( + new Result( + new DateTime("2013-01-01T00:00:00.000Z"), + new TimeseriesResultValue( + ImmutableMap.builder() + .put("rows", 1L) + .put("val", 13l) + .build() + ) + ) + ); + try { + TimeseriesQueryRunnerFactory factory = new TimeseriesQueryRunnerFactory( + new TimeseriesQueryQueryToolChest(new QueryConfig()), + new TimeseriesQueryEngine(), + QueryRunnerTestHelper.NOOP_QUERYWATCHER + ); + + QueryRunner runner = new FinalizeResultsQueryRunner( + factory.createRunner(segment), + factory.getToolchest() + ); + + TestHelper.assertExpectedResults(expectedResults, runner.run(query)); + } + catch (Exception e) { + throw Throwables.propagate(e); + } + } + @Test public void testSpatialQueryMorePoints() { From e1c1e8997a5c3059fc96b0b6e478d2b94c90efaa Mon Sep 17 00:00:00 2001 From: fjy Date: Mon, 29 Sep 2014 17:22:58 -0700 Subject: [PATCH 2/2] address cr --- .../segment/incremental/SpatialDimensionRowFormatter.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/processing/src/main/java/io/druid/segment/incremental/SpatialDimensionRowFormatter.java b/processing/src/main/java/io/druid/segment/incremental/SpatialDimensionRowFormatter.java index 1d624dfb75d..b9b4ce9cc74 100644 --- a/processing/src/main/java/io/druid/segment/incremental/SpatialDimensionRowFormatter.java +++ b/processing/src/main/java/io/druid/segment/incremental/SpatialDimensionRowFormatter.java @@ -56,7 +56,9 @@ public class SpatialDimensionRowFormatter { this.spatialDimensionMap = Maps.newHashMap(); for (SpatialDimensionSchema spatialDimension : spatialDimensions) { - this.spatialDimensionMap.put(spatialDimension.getDimName(), spatialDimension); + if (this.spatialDimensionMap.put(spatialDimension.getDimName(), spatialDimension) != null) { + throw new ISE("Duplicate spatial dimension names found! Check your schema yo!"); + } } this.spatialPartialDimNames = Sets.newHashSet( Iterables.concat(