Jackie-Jiang commented on a change in pull request #6409: URL: https://github.com/apache/incubator-pinot/pull/6409#discussion_r557020612
########## File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/H3IndexFilterOperator.java ########## @@ -50,94 +46,164 @@ public class H3IndexFilterOperator extends BaseFilterOperator { private static final String OPERATOR_NAME = "H3IndexFilterOperator"; + private final IndexSegment _segment; private final Predicate _predicate; - private final DataSource _dataSource; private final int _numDocs; - private final H3Core _h3Core; - private final IndexSegment _segment; private final H3IndexReader _h3IndexReader; - private final H3IndexResolution _resolution; - final private Geometry _geometry; - final private double _distance; + private final long _h3Id; + private final double _edgeLength; + private final double _lowerBound; + private final double _upperBound; - public H3IndexFilterOperator(Predicate predicate, IndexSegment indexSegment, int numDocs) { + public H3IndexFilterOperator(IndexSegment segment, Predicate predicate, int numDocs) { + _segment = segment; _predicate = predicate; - _segment = indexSegment; - FunctionContext function = predicate.getLhs().getFunction(); - String columnName; - - // TODO: handle composite function that contains ST_DISTANCE - if (function.getArguments().get(0).getType() == ExpressionContext.Type.IDENTIFIER) { - columnName = function.getArguments().get(0).getIdentifier(); - byte[] bytes = BytesUtils.toBytes(function.getArguments().get(1).getLiteral()); - _geometry = GeometrySerializer.deserialize(bytes); - } else if (function.getArguments().get(1).getType() == ExpressionContext.Type.IDENTIFIER) { - columnName = function.getArguments().get(1).getIdentifier(); - byte[] bytes = BytesUtils.toBytes(function.getArguments().get(0).getLiteral()); - _geometry = GeometrySerializer.deserialize(bytes); + _numDocs = numDocs; + + List<ExpressionContext> arguments = predicate.getLhs().getFunction().getArguments(); + Coordinate coordinate; + if (arguments.get(0).getType() == ExpressionContext.Type.IDENTIFIER) { + _h3IndexReader = segment.getDataSource(arguments.get(0).getIdentifier()).getH3Index(); + coordinate = GeometrySerializer.deserialize(BytesUtils.toBytes(arguments.get(1).getLiteral())).getCoordinate(); } else { - throw new RuntimeException("Expecting one of the arguments of ST_DISTANCE to be an identifier"); + _h3IndexReader = segment.getDataSource(arguments.get(1).getIdentifier()).getH3Index(); + coordinate = GeometrySerializer.deserialize(BytesUtils.toBytes(arguments.get(0).getLiteral())).getCoordinate(); } - _dataSource = indexSegment.getDataSource(columnName); - _h3IndexReader = _dataSource.getH3Index(); - _resolution = _h3IndexReader.getH3IndexResolution(); - Preconditions.checkArgument(predicate instanceof RangePredicate, - String.format("H3 index does not support predicate type %s", predicate.getType())); + assert _h3IndexReader != null; + int resolution = _h3IndexReader.getH3IndexResolution().getLowestResolution(); + _h3Id = H3Utils.H3_CORE.geoToH3(coordinate.y, coordinate.x, resolution); + _edgeLength = H3Utils.H3_CORE.edgeLength(resolution, LengthUnit.m); + RangePredicate rangePredicate = (RangePredicate) predicate; - _distance = Double.parseDouble(rangePredicate.getUpperBound()); - _numDocs = numDocs; - try { - _h3Core = H3Core.newInstance(); - } catch (IOException e) { - throw new RuntimeException("Unable to instantiate H3 instance", e); + if (!rangePredicate.getLowerBound().equals(RangePredicate.UNBOUNDED)) { + _lowerBound = Double.parseDouble(rangePredicate.getLowerBound()); + } else { + _lowerBound = Double.NaN; + } + if (!rangePredicate.getUpperBound().equals(RangePredicate.UNBOUNDED)) { + _upperBound = Double.parseDouble(rangePredicate.getUpperBound()); + } else { + _upperBound = Double.NaN; } } @Override protected FilterBlock getNextBlock() { - int resolution = _resolution.getLowestResolution(); - long h3Id = _h3Core.geoToH3(_geometry.getCoordinate().x, _geometry.getCoordinate().y, resolution); - assert _h3IndexReader != null; + if (_upperBound < 0 || _lowerBound > _upperBound) { + // Invalid upper bound - // find the number of rings based on distance for full match - // use the edge of the hexagon to determine the rings are within the distance. This is calculated by (1) divide the - // distance by edge length of the resolution to get the number of contained rings (2) use the (floor of number - 1) - // for fetching the rings since ring0 is the original hexagon - double edgeLength = _h3Core.edgeLength(resolution, LengthUnit.m); - int numFullMatchedRings = (int) Math.floor(_distance / edgeLength); - MutableRoaringBitmap fullMatchedDocIds = new MutableRoaringBitmap(); - List<Long> fullMatchRings = new ArrayList<>(); - if (numFullMatchedRings > 0) { - fullMatchRings = _h3Core.kRing(h3Id, numFullMatchedRings - 1); - for (long id : fullMatchRings) { - ImmutableRoaringBitmap docIds = _h3IndexReader.getDocIds(id); - fullMatchedDocIds.or(docIds); + return new FilterBlock(EmptyDocIdSet.getInstance()); + } + + if (Double.isNaN(_lowerBound) || _lowerBound < 0) { Review comment: We need to handle the case for distance > 0, and not return the same point ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org