From 01334fbbb15abc50def257a62b95e0b647b546da Mon Sep 17 00:00:00 2001 From: Enrico Risa Date: Fri, 10 Jan 2020 13:06:34 +0100 Subject: [PATCH] Fixed https://github.com/orientechnologies/orientdb/issues/9105 --- .../lucene/collections/OLuceneResultSet.java | 31 +++--- .../collections/OLuceneResultSetEmpty.java | 59 ++++++++++ .../engine/OLuceneGeoSpatialIndexEngine.java | 23 ++-- .../OSpatialFunctionAbstractIndexable.java | 59 +++++++--- .../strategy/SpatialQueryBuilderAbstract.java | 32 +++--- .../LuceneSpatialQueryIntegrationTest.java | 104 ++++++++++++++++++ 6 files changed, 255 insertions(+), 53 deletions(-) create mode 100755 lucene/src/main/java/com/orientechnologies/lucene/collections/OLuceneResultSetEmpty.java create mode 100755 lucene/src/test/java/com/orientechnologies/spatial/LuceneSpatialQueryIntegrationTest.java diff --git a/lucene/src/main/java/com/orientechnologies/lucene/collections/OLuceneResultSet.java b/lucene/src/main/java/com/orientechnologies/lucene/collections/OLuceneResultSet.java index ce370d3c95c..ecaf533cffc 100755 --- a/lucene/src/main/java/com/orientechnologies/lucene/collections/OLuceneResultSet.java +++ b/lucene/src/main/java/com/orientechnologies/lucene/collections/OLuceneResultSet.java @@ -13,7 +13,7 @@ * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * * See the License for the specific language governing permissions and * * limitations under the License. - * + * */ package com.orientechnologies.lucene.collections; @@ -48,18 +48,23 @@ */ public class OLuceneResultSet implements Set { - private static Integer PAGE_SIZE = 10000; - private final Query query; - private final OLuceneIndexEngine engine; - private final OLuceneQueryContext queryContext; - private final String indexName; - private final Highlighter highlighter; - private final List highlighted; - private final int maxNumFragments; - private TopDocs topDocs; - private long deletedMatchCount = 0; + private static Integer PAGE_SIZE = 10000; + private Query query; + private OLuceneIndexEngine engine; + private OLuceneQueryContext queryContext; + private String indexName; + private Highlighter highlighter; + private List highlighted; + private int maxNumFragments; + private TopDocs topDocs; + private long deletedMatchCount = 0; boolean closed = false; + + protected OLuceneResultSet() { + + } + public OLuceneResultSet(OLuceneIndexEngine engine, OLuceneQueryContext queryContext, ODocument metadata) { this.engine = engine; this.queryContext = queryContext; @@ -177,7 +182,7 @@ private class OLuceneResultSetIteratorTx implements Iterator { private ScoreDoc[] scoreDocs; private int index; private int localIndex; - private long totalHits; + private long totalHits; public OLuceneResultSetIteratorTx() { totalHits = topDocs.totalHits; @@ -194,7 +199,7 @@ public boolean hasNext() { final IndexSearcher searcher = queryContext.getSearcher(); if (searcher.getIndexReader().getRefCount() > 1) { engine.release(searcher); - closed= true; + closed = true; } } return hasNext; diff --git a/lucene/src/main/java/com/orientechnologies/lucene/collections/OLuceneResultSetEmpty.java b/lucene/src/main/java/com/orientechnologies/lucene/collections/OLuceneResultSetEmpty.java new file mode 100755 index 00000000000..b6b83eb397b --- /dev/null +++ b/lucene/src/main/java/com/orientechnologies/lucene/collections/OLuceneResultSetEmpty.java @@ -0,0 +1,59 @@ +/* + * + * * Copyright 2010-2016 OrientDB LTD (http://orientdb.com) + * * + * * Licensed under the Apache License, Version 2.0 (the "License"); + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ + +package com.orientechnologies.lucene.collections; + +import com.orientechnologies.common.exception.OException; +import com.orientechnologies.common.log.OLogManager; +import com.orientechnologies.lucene.engine.OLuceneIndexEngine; +import com.orientechnologies.lucene.engine.OLuceneIndexEngineAbstract; +import com.orientechnologies.lucene.engine.OLuceneIndexEngineUtils; +import com.orientechnologies.lucene.exception.OLuceneIndexException; +import com.orientechnologies.lucene.query.OLuceneQueryContext; +import com.orientechnologies.lucene.tx.OLuceneTxChangesAbstract; +import com.orientechnologies.orient.core.command.OCommandContext; +import com.orientechnologies.orient.core.db.record.OIdentifiable; +import com.orientechnologies.orient.core.id.OContextualRecordId; +import com.orientechnologies.orient.core.record.impl.ODocument; +import org.apache.lucene.analysis.TokenStream; +import org.apache.lucene.document.Document; +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.ScoreDoc; +import org.apache.lucene.search.TopDocs; +import org.apache.lucene.search.highlight.Formatter; +import org.apache.lucene.search.highlight.*; + +import java.io.IOException; +import java.util.*; + +/** + * Created by Enrico Risa on 16/09/15. + */ +public class OLuceneResultSetEmpty extends OLuceneResultSet { + + public OLuceneResultSetEmpty() { + super(); + } + + @Override + public Iterator iterator() { + return Collections.emptyIterator(); + } +} diff --git a/lucene/src/main/java/com/orientechnologies/spatial/engine/OLuceneGeoSpatialIndexEngine.java b/lucene/src/main/java/com/orientechnologies/spatial/engine/OLuceneGeoSpatialIndexEngine.java index 8030d7fa7d2..688897eba27 100755 --- a/lucene/src/main/java/com/orientechnologies/spatial/engine/OLuceneGeoSpatialIndexEngine.java +++ b/lucene/src/main/java/com/orientechnologies/spatial/engine/OLuceneGeoSpatialIndexEngine.java @@ -1,30 +1,30 @@ /** * Copyright 2010-2016 OrientDB LTD (http://orientdb.com) *

- * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. * You may obtain a copy of the License at *

* http://www.apache.org/licenses/LICENSE-2.0 *

- * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" + * BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. *

* For more information: http://www.orientdb.com */ package com.orientechnologies.spatial.engine; +import com.orientechnologies.common.exception.OException; import com.orientechnologies.common.log.OLogManager; import com.orientechnologies.lucene.collections.OLuceneResultSet; +import com.orientechnologies.lucene.collections.OLuceneResultSetEmpty; import com.orientechnologies.lucene.query.OLuceneQueryContext; import com.orientechnologies.lucene.tx.OLuceneTxChanges; import com.orientechnologies.orient.core.db.record.OIdentifiable; import com.orientechnologies.orient.core.id.OContextualRecordId; import com.orientechnologies.orient.core.id.ORID; import com.orientechnologies.orient.core.index.OIndexDefinition; +import com.orientechnologies.orient.core.index.OIndexEngineException; import com.orientechnologies.orient.core.index.OIndexKeyUpdater; import com.orientechnologies.orient.core.record.impl.ODocument; import com.orientechnologies.orient.core.storage.OStorage; @@ -73,10 +73,15 @@ public Set getInTx(Object key, OLuceneTxChanges changes) { } } catch (Exception e) { - OLogManager.instance().error(this, "Error on getting entry against Lucene index", e); + if (e instanceof OException) { + OException forward = (OException) e; + throw forward; + } else { + throw OException.wrapException(new OIndexEngineException("Error parsing lucene query"), e); + } } - return null; + return new OLuceneResultSetEmpty(); } private Set newGeoSearch(Map key, OLuceneTxChanges changes) throws Exception { diff --git a/lucene/src/main/java/com/orientechnologies/spatial/functions/OSpatialFunctionAbstractIndexable.java b/lucene/src/main/java/com/orientechnologies/spatial/functions/OSpatialFunctionAbstractIndexable.java index c00b1b4d369..e0b30956959 100755 --- a/lucene/src/main/java/com/orientechnologies/spatial/functions/OSpatialFunctionAbstractIndexable.java +++ b/lucene/src/main/java/com/orientechnologies/spatial/functions/OSpatialFunctionAbstractIndexable.java @@ -1,36 +1,38 @@ /** * Copyright 2010-2016 OrientDB LTD (http://orientdb.com) *

- * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. * You may obtain a copy of the License at *

* http://www.apache.org/licenses/LICENSE-2.0 *

- * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" + * BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. *

* For more information: http://www.orientdb.com */ package com.orientechnologies.spatial.functions; import com.orientechnologies.lucene.collections.OLuceneResultSet; +import com.orientechnologies.lucene.collections.OLuceneResultSetEmpty; import com.orientechnologies.orient.core.command.OCommandContext; import com.orientechnologies.orient.core.db.ODatabaseDocumentInternal; import com.orientechnologies.orient.core.db.ODatabaseRecordThreadLocal; import com.orientechnologies.orient.core.db.record.OIdentifiable; +import com.orientechnologies.orient.core.exception.OCommandExecutionException; import com.orientechnologies.orient.core.index.OIndex; import com.orientechnologies.orient.core.metadata.OMetadata; import com.orientechnologies.orient.core.record.impl.ODocument; +import com.orientechnologies.orient.core.sql.executor.OResult; +import com.orientechnologies.orient.core.sql.executor.OResultInternal; import com.orientechnologies.orient.core.sql.functions.OIndexableSQLFunction; import com.orientechnologies.orient.core.sql.parser.*; import com.orientechnologies.spatial.index.OLuceneSpatialIndex; import com.orientechnologies.spatial.shape.OShapeFactory; import com.orientechnologies.spatial.strategy.SpatialQueryBuilderAbstract; +import java.text.CollationKey; import java.util.*; import java.util.stream.Collectors; @@ -53,15 +55,9 @@ protected OLuceneSpatialIndex searchForIndex(OFromClause target, OExpression[] a String fieldName = args[0].toString(); String className = identifier.getStringValue(); - List indices = dbMetadata - .getSchema() - .getClass(className) - .getIndexes() - .stream() - .filter(idx -> idx instanceof OLuceneSpatialIndex) - .map(idx -> (OLuceneSpatialIndex) idx) - .filter(idx -> intersect(idx.getDefinition().getFields(), Arrays.asList(fieldName))) - .collect(Collectors.toList()); + List indices = dbMetadata.getSchema().getClass(className).getIndexes().stream() + .filter(idx -> idx instanceof OLuceneSpatialIndex).map(idx -> (OLuceneSpatialIndex) idx) + .filter(idx -> intersect(idx.getDefinition().getFields(), Arrays.asList(fieldName))).collect(Collectors.toList()); if (indices.size() > 1) { throw new IllegalArgumentException("too many indices matching given field name: " + String.join(",", fieldName)); @@ -110,6 +106,37 @@ protected OLuceneResultSet results(OFromClause target, OExpression[] args, OComm } else { shape = args[1].execute((OIdentifiable) null, ctx); } + + if (shape instanceof Collection) { + int size = ((Collection) shape).size(); + + if (size == 0) { + return new OLuceneResultSetEmpty(); + } + if (size == 1) { + + Object next = ((Collection) shape).iterator().next(); + + if (next instanceof OResult) { + OResult inner = (OResult) next; + Set propertyNames = inner.getPropertyNames(); + if (propertyNames.size() == 1) { + Object property = inner.getProperty(propertyNames.iterator().next()); + if (property instanceof OResult) { + shape = ((OResult) property).toElement(); + } + } else { + return new OLuceneResultSetEmpty(); + } + } + } else { + throw new OCommandExecutionException("The collection in input cannot be major than 1"); + } + } + + if (shape instanceof OResultInternal) { + shape = ((OResultInternal) shape).toElement(); + } queryParams.put(SpatialQueryBuilderAbstract.SHAPE, shape); onAfterParsing(queryParams, args, ctx, rightValue); diff --git a/lucene/src/main/java/com/orientechnologies/spatial/strategy/SpatialQueryBuilderAbstract.java b/lucene/src/main/java/com/orientechnologies/spatial/strategy/SpatialQueryBuilderAbstract.java index 74c523d2cdb..058dd756e9f 100644 --- a/lucene/src/main/java/com/orientechnologies/spatial/strategy/SpatialQueryBuilderAbstract.java +++ b/lucene/src/main/java/com/orientechnologies/spatial/strategy/SpatialQueryBuilderAbstract.java @@ -1,17 +1,14 @@ /** * Copyright 2010-2016 OrientDB LTD (http://orientdb.com) *

- * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. * You may obtain a copy of the License at *

* http://www.apache.org/licenses/LICENSE-2.0 *

- * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" + * BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. *

* For more information: http://www.orientdb.com */ @@ -32,13 +29,13 @@ */ public abstract class SpatialQueryBuilderAbstract { - public final static String GEO_FILTER = "geo_filter"; - public final static String SHAPE = "shape"; - public final static String SHAPE_TYPE = "type"; - public final static String COORDINATES = "coordinates"; - public final static String MAX_DISTANCE = "maxDistance"; - protected OLuceneSpatialIndexContainer manager; - protected OShapeBuilder factory; + public final static String GEO_FILTER = "geo_filter"; + public final static String SHAPE = "shape"; + public final static String SHAPE_TYPE = "type"; + public final static String COORDINATES = "coordinates"; + public final static String MAX_DISTANCE = "maxDistance"; + protected OLuceneSpatialIndexContainer manager; + protected OShapeBuilder factory; public SpatialQueryBuilderAbstract(OLuceneSpatialIndexContainer manager, OShapeBuilder factory) { this.manager = manager; @@ -54,7 +51,12 @@ protected Shape parseShape(Map query) { if (geometry == null) { throw new OIndexEngineException("Invalid spatial query. Missing shape field " + query, null); } - return factory.fromObject(geometry); + Shape parsed = factory.fromObject(geometry); + + if (parsed == null) { + throw new OIndexEngineException("Invalid spatial query. Invalid shape field. Found: " + geometry.getClass().getName(), null); + } + return parsed; } protected boolean isOnlyBB(SpatialStrategy spatialStrategy) { diff --git a/lucene/src/test/java/com/orientechnologies/spatial/LuceneSpatialQueryIntegrationTest.java b/lucene/src/test/java/com/orientechnologies/spatial/LuceneSpatialQueryIntegrationTest.java new file mode 100755 index 00000000000..3efa97bc4e5 --- /dev/null +++ b/lucene/src/test/java/com/orientechnologies/spatial/LuceneSpatialQueryIntegrationTest.java @@ -0,0 +1,104 @@ +/** + * Copyright 2010-2016 OrientDB LTD (http://orientdb.com) + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" + * BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + *

+ * For more information: http://www.orientdb.com + */ +package com.orientechnologies.spatial; + +import com.orientechnologies.common.log.OLogManager; +import com.orientechnologies.lucene.test.BaseLuceneTest; +import com.orientechnologies.orient.core.Orient; +import com.orientechnologies.orient.core.exception.OCommandExecutionException; +import com.orientechnologies.orient.core.intent.OIntentMassiveInsert; +import com.orientechnologies.orient.core.metadata.schema.OClass; +import com.orientechnologies.orient.core.metadata.schema.OSchema; +import com.orientechnologies.orient.core.metadata.schema.OType; +import com.orientechnologies.orient.core.record.impl.ODocument; +import com.orientechnologies.orient.core.sql.OCommandSQL; +import com.orientechnologies.orient.core.sql.executor.OResultSet; +import com.orientechnologies.orient.core.sql.query.OSQLSynchQuery; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Test; + +import java.io.File; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.LineNumberReader; +import java.util.Enumeration; +import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Created by Enrico Risa on 02/10/14. + */ +public class LuceneSpatialQueryIntegrationTest extends BaseLuceneTest { + + @Test + public void testIssueGH9105() { + + db.command("create class Country extends V").close(); + db.command("create property Country.name STRING").close(); + db.command("create property Country.geometry EMBEDDED OMultiPolygon").close(); + db.command("create class POI extends V").close(); + db.command("create property POI.name STRING").close(); + db.command("create property POI.location EMBEDDED OPoint").close(); + db.command("insert into POI(name, location) values(\"zeropoint\", St_GeomFromText(\"Point(0 0)\"))").close(); + db.command( + "insert into Country(name, geometry) values(\"zeroland\", St_GeomFromText(\"MultiPolygon(((1 1, 1 -1, -1 -1, -1 1, 1 1)))\"))") + .close(); + db.command("CREATE INDEX POI.location ON POI(location) SPATIAL ENGINE LUCENE"); + db.command("CREATE INDEX Country.geometry ON Country(geometry) SPATIAL ENGINE LUCENE;"); + + try (OResultSet resultSet = db.query( + "select name from Country let locations = (select from Poi) where ST_Contains(geometry, $locations[0].location) = true")) { + + assertThat(resultSet.stream().count()).isEqualTo(1); + } + + try (OResultSet resultSet = db + .query("select name from Country where ST_Contains(geometry, (select location from POI)) = true;")) { + + assertThat(resultSet.stream().count()).isEqualTo(1); + } + + try (OResultSet resultSet = db + .query("select name from Country where ST_Contains(geometry, (select name,location from POI)) = true;")) { + + assertThat(resultSet.stream().count()).isEqualTo(0); + } + + db.command("insert into POI(name, location) values(\"zeropoint\", St_GeomFromText(\"Point(0 0)\"))").close(); + + try (OResultSet resultSet = db + .query("select name from Country where ST_Contains(geometry, (select location from POI)) = true;")) { + + Assert.fail("It should throw an exception"); + } catch (Exception e) { + Assert.assertTrue(e instanceof OCommandExecutionException); + } + + db.command("delete vertex Poi").close(); + + try (OResultSet resultSet = db + .query("select name from Country where ST_Contains(geometry, (select location from POI)) = true;")) { + + assertThat(resultSet.stream().count()).isEqualTo(0); + } + + } +}