diff --git a/janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java b/janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java index 19c897ea39..ce98558f6d 100644 --- a/janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java +++ b/janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java @@ -4434,4 +4434,31 @@ public void testGetIndexInfo() throws DecoderException { assertEquals(1, indexInfo.getCompositeIndexType().getInlineFieldKeys().length); assertEquals("id", indexInfo.getCompositeIndexType().getInlineFieldKeys()[0]); } + + @Test + public void testOrderingListProperty() { + PropertyKey strSingle = mgmt.makePropertyKey("strSingle").dataType(String.class).cardinality(Cardinality.SINGLE) + .make(); + PropertyKey strList = mgmt.makePropertyKey("strList").dataType(String.class).cardinality(Cardinality.LIST) + .make(); + + mgmt.buildIndex("mixedStrSingle", Vertex.class).addKey(strSingle, Mapping.STRING.asParameter()) + .buildMixedIndex(INDEX); + mgmt.buildIndex("mixedStrList", Vertex.class).addKey( + strList, Mapping.STRING.asParameter()) + .buildMixedIndex(INDEX); + finishSchema(); + + tx.addVertex("strSingle", "val1", "strList", "val1"); + tx.addVertex("strSingle", "val2"); + tx.addVertex("strList", "val3"); + tx.addVertex("strSingle", "val4", "strList", "val4", "strList", "val5", "strList", "val6"); + tx.addVertex("strSingle", "val7", "strList", "val7"); + tx.commit(); + + clopen(option(FORCE_INDEX_USAGE), false); + + assertEquals(4, tx.traversal().V().has("strSingle").order().by("strSingle").count().next()); + assertEquals(4, tx.traversal().V().has("strList").order().by("strList").count().next()); + } } diff --git a/janusgraph-core/src/main/java/org/janusgraph/graphdb/query/graph/GraphCentricQueryBuilder.java b/janusgraph-core/src/main/java/org/janusgraph/graphdb/query/graph/GraphCentricQueryBuilder.java index bfc51f394b..f5f4243e5f 100644 --- a/janusgraph-core/src/main/java/org/janusgraph/graphdb/query/graph/GraphCentricQueryBuilder.java +++ b/janusgraph-core/src/main/java/org/janusgraph/graphdb/query/graph/GraphCentricQueryBuilder.java @@ -15,7 +15,7 @@ package org.janusgraph.graphdb.query.graph; import com.google.common.base.Preconditions; -import org.janusgraph.core.Cardinality; +//import org.janusgraph.core.Cardinality; import org.janusgraph.core.JanusGraphEdge; import org.janusgraph.core.JanusGraphElement; import org.janusgraph.core.JanusGraphQuery; @@ -201,8 +201,8 @@ public GraphCentricQueryBuilder orderBy(String keyName, org.apache.tinkerpop.gr Preconditions.checkArgument(key!=null && order!=null,"Need to specify and key and an order"); Preconditions.checkArgument(Comparable.class.isAssignableFrom(key.dataType()), "Can only order on keys with comparable data type. [%s] has datatype [%s]", key.name(), key.dataType()); - Preconditions.checkArgument(key.cardinality()== Cardinality.SINGLE, - "Ordering is undefined on multi-valued key [%s]", key.name()); + //Preconditions.checkArgument(key.cardinality()== Cardinality.SINGLE, + // "Ordering is undefined on multi-valued key [%s]", key.name()); Preconditions.checkArgument(!orders.containsKey(key), "orders [%s] already contains key [%s]", orders, key); orders.add(key, Order.convert(order)); return this; diff --git a/janusgraph-core/src/main/java/org/janusgraph/graphdb/tinkerpop/optimize/step/HasStepFolder.java b/janusgraph-core/src/main/java/org/janusgraph/graphdb/tinkerpop/optimize/step/HasStepFolder.java index bcf6d78444..09fb72d8b9 100644 --- a/janusgraph-core/src/main/java/org/janusgraph/graphdb/tinkerpop/optimize/step/HasStepFolder.java +++ b/janusgraph-core/src/main/java/org/janusgraph/graphdb/tinkerpop/optimize/step/HasStepFolder.java @@ -47,6 +47,7 @@ import java.util.ArrayList; import java.util.Comparator; +import java.util.Iterator; import java.util.List; import java.util.Objects; import java.util.Set; @@ -58,14 +59,17 @@ public interface HasStepFolder extends Step { /** * Optional method which provides a hint to future has containers additions. - * The method will optionally resize the underlying array for future addition to ensure that it can hold additional - * elements. I.e. it will ensure that we can add `additionalSize` containers without any resizes. + * The method will optionally resize the underlying array for future addition to + * ensure that it can hold additional + * elements. I.e. it will ensure that we can add `additionalSize` containers + * without any resizes. */ void ensureAdditionalHasContainersCapacity(int additionalSize); void addHasContainer(final HasContainer hasContainer); - List addLocalHasContainersConvertingAndPContainers(TraversalParent parent, List localHasContainers); + List addLocalHasContainersConvertingAndPContainers(TraversalParent parent, + List localHasContainers); List addLocalHasContainersSplittingAndPContainers(TraversalParent parent, Iterable has); @@ -88,7 +92,7 @@ public interface HasStepFolder extends Step { static boolean validJanusGraphHas(HasContainer has) { if (has.getPredicate() instanceof ConnectiveP) { final List> predicates = ((ConnectiveP) has.getPredicate()).getPredicates(); - return predicates.stream().allMatch(p-> validJanusGraphHas(new HasContainer(has.getKey(), p))); + return predicates.stream().allMatch(p -> validJanusGraphHas(new HasContainer(has.getKey(), p))); } else { return JanusGraphPredicateUtils.supports(has.getBiPredicate()); } @@ -96,32 +100,61 @@ static boolean validJanusGraphHas(HasContainer has) { static boolean validJanusGraphHas(Iterable has) { for (final HasContainer h : has) { - if (!validJanusGraphHas(h)) return false; + if (!validJanusGraphHas(h)) + return false; } return true; } + static boolean graphHasLuceneIndexBackend(JanusGraphTransaction tx) { + Iterator configIt = tx.configuration().getKeys(); + while (configIt.hasNext()) { + String configKey = configIt.next(); + if (configKey.matches("index\\.[^.]+\\.backend")) { + // get the value and check if it is Lucene + String configValue = tx.configuration().getString(configKey); + System.err.println(configValue); + if (configValue != null && configValue.toLowerCase().trim().equals("lucene")) { + return true; + } + } + } + return false; + } + static boolean validJanusGraphOrder(OrderGlobalStep orderGlobalStep, Traversal rootTraversal, - boolean isVertexOrder) { + boolean isVertexOrder) { final List> comparators = orderGlobalStep.getComparators(); - for(final Pair comp : comparators) { + for (final Pair comp : comparators) { final String key; if (comp.getValue0() instanceof ValueTraversal && - comp.getValue1() instanceof Order) { + comp.getValue1() instanceof Order) { key = ((ValueTraversal) comp.getValue0()).getPropertyKey(); } else if (comp.getValue1() instanceof ElementValueComparator) { final ElementValueComparator evc = (ElementValueComparator) comp.getValue1(); - if (!(evc.getValueComparator() instanceof Order)) return false; + if (!(evc.getValueComparator() instanceof Order)) + return false; key = evc.getPropertyKey(); } else { - // do not fold comparators that include nested traversals that are not simple ElementValues + // do not fold comparators that include nested traversals that are not simple + // ElementValues return false; } final JanusGraphTransaction tx = JanusGraphTraversalUtil.getTx(rootTraversal.asAdmin()); + + // check if any indexing backend is Lucene + // in case yes, let's not allow ordering via index backend for LIST properties + // in LuceneIndex.java:buildIndexFields() methods for some reason + // "isPossibleSortIndex" variable is set only for Single variables + // if isPossibleSortIndex is changed to "true" all the time, a lot of tests + // start failing! + // for Solr and ElasticSearch, LIST property ordering is supported + boolean graphHasLuceneIndexBackend = graphHasLuceneIndexBackend(tx); + final PropertyKey pKey = tx.getPropertyKey(key); if (pKey == null - || !(Comparable.class.isAssignableFrom(pKey.dataType())) - || (isVertexOrder && pKey.cardinality() != Cardinality.SINGLE)) { + || !(Comparable.class.isAssignableFrom(pKey.dataType())) + || (isVertexOrder && graphHasLuceneIndexBackend && pKey.cardinality() != Cardinality.SINGLE)) { return false; } } @@ -134,15 +167,18 @@ static void foldInIds(final HasStepFolder janusgraphStep, final Traversal.Admin< if (currentStep instanceof HasContainerHolder) { final HasContainerHolder hasContainerHolder = (HasContainerHolder) currentStep; final GraphStep graphStep = (GraphStep) janusgraphStep; - // HasContainer collection that we get back is immutable so we keep track of which containers - // need to be deleted after they've been folded into the JanusGraphStep and then remove them from their + // HasContainer collection that we get back is immutable so we keep track of + // which containers + // need to be deleted after they've been folded into the JanusGraphStep and then + // remove them from their // step using HasContainer.removeHasContainer final List removableHasContainers = new ArrayList<>(); final Set stepLabels = currentStep.getLabels(); hasContainerHolder.getHasContainers().forEach(hasContainer -> { if (GraphStep.processHasContainerIds(graphStep, hasContainer)) { stepLabels.forEach(janusgraphStep::addLabel); - // this has container is no longer needed because its ids will be folded into the JanusGraphStep + // this has container is no longer needed because its ids will be folded into + // the JanusGraphStep removableHasContainers.add(hasContainer); } }); @@ -154,8 +190,7 @@ static void foldInIds(final HasStepFolder janusgraphStep, final Traversal.Admin< if (hasContainerHolder.getHasContainers().isEmpty()) { traversal.removeStep(currentStep); } - } - else if (currentStep instanceof IdentityStep || currentStep instanceof NoOpBarrierStep) { + } else if (currentStep instanceof IdentityStep || currentStep instanceof NoOpBarrierStep) { // do nothing, has no impact } else { break; @@ -170,17 +205,18 @@ static void foldInHasContainer(final HasStepFolder janusgraphStep, final Travers while (true) { if (currentStep instanceof OrStep && janusgraphStep instanceof JanusGraphStep) { for (final Traversal.Admin child : ((OrStep) currentStep).getLocalChildren()) { - if (!validFoldInHasContainer(child.getStartStep(), false)){ + if (!validFoldInHasContainer(child.getStartStep(), false)) { return; } } - ((OrStep) currentStep).getLocalChildren().forEach(t ->localFoldInHasContainer(janusgraphStep, t.getStartStep(), t, rootTraversal)); + ((OrStep) currentStep).getLocalChildren() + .forEach(t -> localFoldInHasContainer(janusgraphStep, t.getStartStep(), t, rootTraversal)); currentStep.getLabels().forEach(janusgraphStep::addLabel); traversal.removeStep(currentStep); - } else if (currentStep instanceof HasContainerHolder && validFoldInHasContainer(currentStep, true)){ + } else if (currentStep instanceof HasContainerHolder && validFoldInHasContainer(currentStep, true)) { List hasContainersList = ((HasContainerHolder) currentStep).getHasContainers(); janusgraphStep.ensureAdditionalHasContainersCapacity(hasContainersList.size()); - for(HasContainer hasContainer : hasContainersList){ + for (HasContainer hasContainer : hasContainersList) { janusgraphStep.addHasContainer(JanusGraphPredicateUtils.convert(hasContainer)); } currentStep.getLabels().forEach(janusgraphStep::addLabel); @@ -195,16 +231,19 @@ static void foldInHasContainer(final HasStepFolder janusgraphStep, final Travers } } - static void localFoldInHasContainer(final HasStepFolder janusgraphStep, final Step tinkerpopStep, final Traversal.Admin traversal, - final Traversal rootTraversal){ + static void localFoldInHasContainer(final HasStepFolder janusgraphStep, final Step tinkerpopStep, + final Traversal.Admin traversal, + final Traversal rootTraversal) { Step currentStep = tinkerpopStep; while (true) { if (currentStep instanceof HasContainerHolder) { List localHasContainers = janusgraphStep.addLocalHasContainersConvertingAndPContainers( - traversal.getParent(), ((HasContainerHolder) currentStep).getHasContainers()); + traversal.getParent(), ((HasContainerHolder) currentStep).getHasContainers()); currentStep.getLabels().forEach(janusgraphStep::addLabel); traversal.removeStep(currentStep); - currentStep = foldInOrder(janusgraphStep, currentStep, traversal, rootTraversal, janusgraphStep instanceof JanusGraphStep && ((JanusGraphStep)janusgraphStep).returnsVertex(), localHasContainers); + currentStep = foldInOrder(janusgraphStep, currentStep, traversal, rootTraversal, + janusgraphStep instanceof JanusGraphStep && ((JanusGraphStep) janusgraphStep).returnsVertex(), + localHasContainers); foldInRange(janusgraphStep, currentStep, traversal, localHasContainers); } else if (isExistsStep(currentStep)) { currentStep = foldInHasFilter(traversal, (TraversalFilterStep) currentStep); @@ -216,16 +255,18 @@ static void localFoldInHasContainer(final HasStepFolder janusgraphStep, final St } } - static boolean validFoldInHasContainer(final Step tinkerpopStep, final boolean defaultValue){ + static boolean validFoldInHasContainer(final Step tinkerpopStep, final boolean defaultValue) { Step currentStep = tinkerpopStep; Boolean toReturn = null; while (!(currentStep instanceof EmptyStep)) { if (currentStep instanceof HasContainerHolder) { final Iterable containers = ((HasContainerHolder) currentStep).getHasContainers(); - toReturn = toReturn == null ? validJanusGraphHas(containers) : toReturn && validJanusGraphHas(containers); + toReturn = toReturn == null ? validJanusGraphHas(containers) + : toReturn && validJanusGraphHas(containers); } else if (isExistsStep(currentStep)) { toReturn = true; - } else if (!(currentStep instanceof IdentityStep) && !(currentStep instanceof NoOpBarrierStep) && !(currentStep instanceof RangeGlobalStep) && !(currentStep instanceof OrderGlobalStep)) { + } else if (!(currentStep instanceof IdentityStep) && !(currentStep instanceof NoOpBarrierStep) + && !(currentStep instanceof RangeGlobalStep) && !(currentStep instanceof OrderGlobalStep)) { toReturn = toReturn != null && (toReturn && defaultValue); break; } @@ -236,7 +277,9 @@ static boolean validFoldInHasContainer(final Step tinkerpopStep, final bo /** * Check if a given step is an "exists" step, i.e. has(key) step - * Tinkerpop translates has(key) gremlin query into TraversalFilterStep(PropertiesStep) + * Tinkerpop translates has(key) gremlin query into + * TraversalFilterStep(PropertiesStep) + * * @param step * @return true if given step is essentially an "exists" step */ @@ -245,50 +288,56 @@ static boolean isExistsStep(final Step step) { List traversals = ((TraversalFilterStep) step).getLocalChildren(); assert traversals.size() == 1; List steps = traversals.get(0).getSteps(); - if (!(steps.size() == 1 && steps.get(0) instanceof PropertiesStep)) return false; + if (!(steps.size() == 1 && steps.get(0) instanceof PropertiesStep)) + return false; return ((PropertiesStep) steps.get(0)).getPropertyKeys().length == 1; } return false; } /** - * Convert a TraversalFilterStep that is essentially "has(key)" into a normal HasStep - * @param traversal local traversal + * Convert a TraversalFilterStep that is essentially "has(key)" into a normal + * HasStep + * + * @param traversal local traversal * @param currentStep */ static Step foldInHasFilter(final Traversal.Admin traversal, final TraversalFilterStep currentStep) { List> traversals = currentStep.getLocalChildren(); - assert(traversals.size() == 1); + assert (traversals.size() == 1); List steps = traversals.get(0).getSteps(); - assert(steps.size() == 1 && steps.get(0) instanceof PropertiesStep); + assert (steps.size() == 1 && steps.get(0) instanceof PropertiesStep); String[] propertyKeys = ((PropertiesStep) steps.get(0)).getPropertyKeys(); - assert(propertyKeys.length == 1); + assert (propertyKeys.length == 1); HasStep hasStep = new HasStep(traversal, new HasContainer(propertyKeys[0], P.neq(null))); currentStep.getLabels().forEach(hasStep::addLabel); TraversalHelper.replaceStep(currentStep, hasStep, traversal); return hasStep; } - static Step foldInOrder(final HasStepFolder janusgraphStep, final Step tinkerpopStep, final Traversal.Admin traversal, - final Traversal rootTraversal, boolean isVertexOrder, final List hasContainers) { + static Step foldInOrder(final HasStepFolder janusgraphStep, final Step tinkerpopStep, + final Traversal.Admin traversal, + final Traversal rootTraversal, boolean isVertexOrder, final List hasContainers) { Step currentStep = tinkerpopStep; OrderGlobalStep lastOrder = null; while (true) { if (currentStep instanceof OrderGlobalStep) { - if (lastOrder != null) { //Previous orders are rendered irrelevant by next order (since re-ordered) + if (lastOrder != null) { // Previous orders are rendered irrelevant by next order (since re-ordered) lastOrder.getLabels().forEach(janusgraphStep::addLabel); traversal.removeStep(lastOrder); } lastOrder = (OrderGlobalStep) currentStep; - } else if (!(currentStep instanceof IdentityStep) && !(currentStep instanceof HasStep) && !(currentStep instanceof NoOpBarrierStep)) { + } else if (!(currentStep instanceof IdentityStep) && !(currentStep instanceof HasStep) + && !(currentStep instanceof NoOpBarrierStep)) { break; } currentStep = currentStep.getNextStep(); } if (lastOrder != null && validJanusGraphOrder(lastOrder, rootTraversal, isVertexOrder)) { - //Add orders to HasStepFolder - for (final Pair, Comparator> comp : (List, Comparator>>) ((OrderGlobalStep) lastOrder).getComparators()) { + // Add orders to HasStepFolder + for (final Pair, Comparator> comp : (List, Comparator>>) ((OrderGlobalStep) lastOrder) + .getComparators()) { final String key; final Order order; if (comp.getValue0() instanceof ValueTraversal) { @@ -334,12 +383,15 @@ public OrderEntry(String key, Order order) { @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; final OrderEntry that = (OrderEntry) o; - if (!Objects.equals(key, that.key)) return false; + if (!Objects.equals(key, that.key)) + return false; return order == that.order; } @@ -353,29 +405,36 @@ public int hashCode() { @Override public String toString() { return "OrderEntry{" + - "key='" + key + '\'' + - ", order=" + order + - '}'; + "key='" + key + '\'' + + ", order=" + order + + '}'; } } - static void foldInRange(final HasStepFolder janusgraphStep, final Step tinkerpopStep, final Traversal.Admin traversal, final List hasContainers) { - final Step nextStep = tinkerpopStep instanceof IdentityStep ? JanusGraphTraversalUtil.getNextNonIdentityStep(tinkerpopStep): tinkerpopStep; + static void foldInRange(final HasStepFolder janusgraphStep, final Step tinkerpopStep, + final Traversal.Admin traversal, final List hasContainers) { + final Step nextStep = tinkerpopStep instanceof IdentityStep + ? JanusGraphTraversalUtil.getNextNonIdentityStep(tinkerpopStep) + : tinkerpopStep; if (nextStep instanceof RangeGlobalStep) { final RangeGlobalStep range = (RangeGlobalStep) nextStep; int low = 0; if (janusgraphStep instanceof JanusGraphStep) { low = QueryUtil.convertLimit(range.getLowRange()); - low = QueryUtil.mergeLowLimits(low, hasContainers == null ? janusgraphStep.getLowLimit(): janusgraphStep.getLocalLowLimit(traversal.getParent(), hasContainers)); + low = QueryUtil.mergeLowLimits(low, hasContainers == null ? janusgraphStep.getLowLimit() + : janusgraphStep.getLocalLowLimit(traversal.getParent(), hasContainers)); } int high = QueryUtil.convertLimit(range.getHighRange()); - high = QueryUtil.mergeHighLimits(high, hasContainers == null ? janusgraphStep.getHighLimit(): janusgraphStep.getLocalHighLimit(traversal.getParent(), hasContainers)); + high = QueryUtil.mergeHighLimits(high, hasContainers == null ? janusgraphStep.getHighLimit() + : janusgraphStep.getLocalHighLimit(traversal.getParent(), hasContainers)); if (hasContainers == null) { janusgraphStep.setLimit(low, high); } else { janusgraphStep.setLocalLimit(traversal.getParent(), hasContainers, low, high); } - if (janusgraphStep instanceof JanusGraphStep || range.getLowRange() == 0) { //Range can be removed since there is JanusGraphStep or no offset + if (janusgraphStep instanceof JanusGraphStep || range.getLowRange() == 0) { // Range can be removed since + // there is JanusGraphStep or no + // offset nextStep.getLabels().forEach(janusgraphStep::addLabel); traversal.removeStep(nextStep); } @@ -384,7 +443,8 @@ static void foldInRange(final HasStepFolder janusgraphStep, final Step ti /** * @param janusgraphStep The step to test - * @return True if there are 'has' steps following this step and no subsequent range limit step + * @return True if there are 'has' steps following this step and no subsequent + * range limit step */ static boolean foldableHasContainerNoLimit(FlatMapStep janusgraphStep) { boolean foldableHasContainerNoLimit = false; @@ -392,14 +452,14 @@ static boolean foldableHasContainerNoLimit(FlatMapStep janusgraphStep) { while (true) { if (currentStep instanceof OrStep) { for (final Traversal.Admin child : ((OrStep) currentStep).getLocalChildren()) { - if (!validFoldInHasContainer(child.getStartStep(), false)){ + if (!validFoldInHasContainer(child.getStartStep(), false)) { return false; } } - foldableHasContainerNoLimit = true; + foldableHasContainerNoLimit = true; } else if (currentStep instanceof HasContainerHolder) { - if (validFoldInHasContainer(currentStep, true)) { - foldableHasContainerNoLimit = true; + if (validFoldInHasContainer(currentStep, true)) { + foldableHasContainerNoLimit = true; } } else if (currentStep instanceof RangeGlobalStep) { return false; diff --git a/janusgraph-solr/src/test/java/org/janusgraph/diskstorage/solr/JanusGraphSolrContainer.java b/janusgraph-solr/src/test/java/org/janusgraph/diskstorage/solr/JanusGraphSolrContainer.java index d16c3e2b3f..afdbc39adc 100644 --- a/janusgraph-solr/src/test/java/org/janusgraph/diskstorage/solr/JanusGraphSolrContainer.java +++ b/janusgraph-solr/src/test/java/org/janusgraph/diskstorage/solr/JanusGraphSolrContainer.java @@ -30,7 +30,7 @@ public class JanusGraphSolrContainer extends SolrContainer { private static final String DEFAULT_SOLR_VERSION = "8.11.1"; private static final String DEFAULT_SOLR_IMAGE = "solr"; - private static final String COLLECTIONS = "store1 store2 vertex edge namev namee composite psearch esearch vsearch mi mixed index1 index2 index3 ecategory vcategory pcategory theIndex vertices edges booleanIndex dateIndex instantIndex uuidIndex randomMixedIndex collectionIndex nameidx oridx otheridx lengthidx"; + private static final String COLLECTIONS = "store1 store2 vertex edge namev namee composite psearch esearch vsearch mi mixed index1 index2 index3 ecategory vcategory pcategory theIndex vertices edges booleanIndex dateIndex instantIndex uuidIndex randomMixedIndex collectionIndex nameidx oridx otheridx lengthidx mixedStrSingle mixedStrList"; private static String getVersion() { String property = System.getProperty("solr.docker.version");