Multiple labels in CREATE and MATCH (#130)

* Move Cypher extensions post-condition to translation writer * Allow enabling multiple translator features * Translate CREATE and MATCH with multiple labels * Workaround for multi-label bug in Neptune * Enable multiple labels translation for Neptune tests * Fix rewriters to support multi-labels Signed-off-by: Dimitry Solovyov <[email protected]>
opencypher · Jul 4, 2018 · 3d909c7 · 3d909c7
1 parent 83cb6ff
commit 3d909c7
Show file tree

Hide file tree

Showing 21 changed files with 265 additions and 67 deletions.
diff --git a/testware/integration-tests/src/test/java/org/opencypher/gremlin/queries/ProcedureTest.java b/testware/integration-tests/src/test/java/org/opencypher/gremlin/queries/ProcedureTest.java
@@ -63,6 +63,7 @@ private List<Map<String, Object>> submitAndGet(String cypher, Map<String, Object
         DefaultGraphTraversal g = new DefaultGraphTraversal(gts);
         Translator<GraphTraversal, P> translator = Translator.builder()
             .traversal(g)
+            .enableCypherExtensions()
             .build();
         CypherAst ast = CypherAst.parse(cypher, parameters);
         Seq<GremlinStep> ir = ast.translate(flavor, procedureContext);

diff --git a/...ware-common/src/main/java/org/opencypher/gremlin/rules/GremlinServerExternalResource.java b/...ware-common/src/main/java/org/opencypher/gremlin/rules/GremlinServerExternalResource.java
@@ -77,7 +77,7 @@ private CypherGremlinClient configuredCypherGremlinClient() {
             case "gremlin":
                 return CypherGremlinClient.translating(gremlinClient, () -> Translator.builder()
                     .gremlinGroovy()
-                    .allowCypherExtensions()
+                    .enableCypherExtensions()
                     .build());
             case "vanilla":
                 return CypherGremlinClient.translating(gremlinClient, () -> Translator.builder()
@@ -86,7 +86,7 @@ private CypherGremlinClient configuredCypherGremlinClient() {
             case "bytecode":
                 return CypherGremlinClient.bytecode(gremlinClient.alias("g"), () -> Translator.builder()
                     .bytecode()
-                    .allowCypherExtensions()
+                    .enableCypherExtensions()
                     .build());
             case "cosmosdb":
                 return CypherGremlinClient.translating(gremlinClient, () -> Translator.builder()
@@ -96,6 +96,7 @@ private CypherGremlinClient configuredCypherGremlinClient() {
                 return CypherGremlinClient.translating(gremlinClient, () -> Translator.builder()
                     .gremlinGroovy()
                     .inlineParameters()
+                    .enableMultipleLabels()
                     .build(TranslatorFlavor.neptune()));
             default:
                 throw new IllegalArgumentException("Unknown name: " + clientName);

diff --git a/...sole-plugin/src/main/java/org/opencypher/gremlin/console/jsr223/CypherRemoteAcceptor.java b/...sole-plugin/src/main/java/org/opencypher/gremlin/console/jsr223/CypherRemoteAcceptor.java
@@ -88,15 +88,15 @@ private static Translator<String, GroovyPredicate> translatorByName(List<String>
             case "cosmosdb":
                 return builder.build(TranslatorFlavor.cosmosDb());
             case "cosmosdb+extensions":
-                return builder.allowCypherExtensions().build(TranslatorFlavor.cosmosDb());
+                return builder.enableCypherExtensions().build(TranslatorFlavor.cosmosDb());
             case "neptune":
                 return builder.build(TranslatorFlavor.neptune());
             case "neptune+extensions":
-                return builder.allowCypherExtensions().build(TranslatorFlavor.neptune());
+                return builder.enableCypherExtensions().build(TranslatorFlavor.neptune());
             case "gremlin":
                 return builder.build(TranslatorFlavor.gremlinServer());
             case "gremlin+extensions":
-                return builder.allowCypherExtensions().build(TranslatorFlavor.gremlinServer());
+                return builder.enableCypherExtensions().build(TranslatorFlavor.gremlinServer());
             case "":
                 return builder.build(TranslatorFlavor.gremlinServer());
             default:

diff --git a/...op/cypher-gremlin-extensions/src/main/java/org/opencypher/gremlin/translation/Tokens.java b/...op/cypher-gremlin-extensions/src/main/java/org/opencypher/gremlin/translation/Tokens.java
@@ -22,6 +22,7 @@ private Tokens() {
     public static final String START = "  cypher.start";
     public static final String NULL = "  cypher.null";
     public static final String UNUSED = "  cypher.unused";
+    public static final String NONEXISTENT = "  cypher.nonexistent";
     public static final String PATH_EDGE = "  cypher.path.edge.";
     public static final String PATH_START = "  cypher.path.start.";
     public static final String MATCH_START = "  cypher.match.start.";

diff --git a/...erver-client/src/main/java/org/opencypher/gremlin/client/InMemoryCypherGremlinClient.java b/...erver-client/src/main/java/org/opencypher/gremlin/client/InMemoryCypherGremlinClient.java
@@ -62,7 +62,7 @@ public CompletableFuture<CypherResultSet> submitAsync(String cypher, Map<String,
         }
 
         DefaultGraphTraversal g = new DefaultGraphTraversal(gts.clone());
-        Translator<GraphTraversal, P> translator = Translator.builder().traversal(g).allowCypherExtensions().build();
+        Translator<GraphTraversal, P> translator = Translator.builder().traversal(g).enableCypherExtensions().build();
         GraphTraversal<?, ?> traversal = ast.buildTranslation(translator);
         ReturnNormalizer returnNormalizer = ReturnNormalizer.create(ast.getReturnTypes());
         List<Result> results = traversal.toStream()

diff --git a/...erver-plugin/src/main/java/org/opencypher/gremlin/server/op/cypher/CypherOpProcessor.java b/...erver-plugin/src/main/java/org/opencypher/gremlin/server/op/cypher/CypherOpProcessor.java
@@ -106,7 +106,7 @@ private void evalCypher(Context context) throws OpProcessorException {
         Translator<String, GroovyPredicate> stringTranslator = Translator.builder()
             .gremlinGroovy()
             .inlineParameters()
-            .allowCypherExtensions()
+            .enableCypherExtensions()
             .build();
 
         String gremlin = TranslationWriter.write(ir, stringTranslator, parameters);
@@ -119,6 +119,7 @@ private void evalCypher(Context context) throws OpProcessorException {
 
         Translator<GraphTraversal, P> traversalTranslator = Translator.builder()
             .traversal(g)
+            .enableCypherExtensions()
             .build();
 
         GraphTraversal<?, ?> traversal = TranslationWriter.write(ir, traversalTranslator, parameters);

diff --git a/translation/src/main/java/org/opencypher/gremlin/translation/translator/Translator.java b/translation/src/main/java/org/opencypher/gremlin/translation/translator/Translator.java
@@ -16,6 +16,8 @@
 package org.opencypher.gremlin.translation.translator;
 
 
+import java.util.EnumSet;
+import java.util.Set;
 import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.DefaultGraphTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
@@ -43,18 +45,18 @@ public final class Translator<T, P> {
     private final GremlinSteps<T, P> steps;
     private final GremlinPredicates<P> predicates;
     private final GremlinBindings bindings;
-    private final boolean cypherExtensions;
+    private final Set<TranslatorFeature> features;
     private final TranslatorFlavor flavor;
 
     private Translator(GremlinSteps<T, P> steps,
                        GremlinPredicates<P> predicates,
                        GremlinBindings bindings,
-                       boolean cypherExtensions,
+                       Set<TranslatorFeature> features,
                        TranslatorFlavor flavor) {
         this.steps = steps;
         this.predicates = predicates;
         this.bindings = bindings;
-        this.cypherExtensions = cypherExtensions;
+        this.features = features;
         this.flavor = flavor;
     }
 
@@ -92,12 +94,12 @@ public GremlinBindings bindings() {
     }
 
     /**
-     * Returns true if this translation assumes the CfoG plugin is installed on target server.
+     * Returns true if a given feature is enabled in this translator.
      *
-     * @return true, if CfoG plugin should be installed, false otherwise
+     * @return true, if the feature is enabled, false otherwise
      */
-    public boolean requiresCypherExtensions() {
-        return cypherExtensions;
+    public boolean isEnabled(TranslatorFeature feature) {
+        return features.contains(feature);
     }
 
     /**
@@ -217,7 +219,7 @@ public static class FlavorBuilder<T, P> {
         private final GremlinSteps<T, P> steps;
         private final GremlinPredicates<P> predicates;
         protected GremlinBindings bindings;
-        protected boolean cypherExtensions = false;
+        private final Set<TranslatorFeature> features = EnumSet.noneOf(TranslatorFeature.class);
 
         private FlavorBuilder(GremlinSteps<T, P> steps,
                               GremlinPredicates<P> predicates,
@@ -228,13 +230,34 @@ private FlavorBuilder(GremlinSteps<T, P> steps,
         }
 
         /**
-         * Builds a {@link Translator} with support for custom functions and predicates
-         * provided by the CfoG Gremlin Server plugin.
+         * Enables a feature in the {@link Translator} that's being built.
          *
          * @return builder for translator
          */
-        public FlavorBuilder<T, P> allowCypherExtensions() {
-            cypherExtensions = true;
+        public FlavorBuilder<T, P> enable(TranslatorFeature feature) {
+            features.add(feature);
+            return this;
+        }
+
+        /**
+         * Enables Cypher extensions in the {@link Translator} that's being built.
+         *
+         * @return builder for translator
+         * @see TranslatorFeature#CYPHER_EXTENSIONS
+         */
+        public FlavorBuilder<T, P> enableCypherExtensions() {
+            features.add(TranslatorFeature.CYPHER_EXTENSIONS);
+            return this;
+        }
+
+        /**
+         * Enables multiple labels translation in the {@link Translator} that's being built.
+         *
+         * @return builder for translator
+         * @see TranslatorFeature#CYPHER_EXTENSIONS
+         */
+        public FlavorBuilder<T, P> enableMultipleLabels() {
+            features.add(TranslatorFeature.MULTIPLE_LABELS);
             return this;
         }
 
@@ -258,7 +281,7 @@ public Translator<T, P> build(TranslatorFlavor flavor) {
                 steps,
                 predicates,
                 bindings,
-                cypherExtensions,
+                features,
                 flavor != null ? flavor : TranslatorFlavor.gremlinServer()
             );
         }

diff --git a/...lation/src/main/java/org/opencypher/gremlin/translation/translator/TranslatorFeature.java b/...lation/src/main/java/org/opencypher/gremlin/translation/translator/TranslatorFeature.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2018 "Neo4j, Inc." [https://neo4j.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 org.opencypher.gremlin.translation.translator;
+
+/**
+ * Translator features are additional behaviors that can be allowed in translation.
+ * These need to be enabled individually when creating a {@link Translator}.
+ */
+public enum TranslatorFeature {
+    /**
+     * Support for custom functions and predicates
+     * provided by the CfoG Gremlin Server plugin.
+     */
+    CYPHER_EXTENSIONS,
+
+    /**
+     * Support for specifying multiple labels for a vertex
+     * and matching by multiple labels.
+     */
+    MULTIPLE_LABELS
+}
diff --git a/translation/src/main/scala/org/opencypher/gremlin/translation/CypherAst.scala b/translation/src/main/scala/org/opencypher/gremlin/translation/CypherAst.scala
@@ -25,7 +25,8 @@ import org.opencypher.gremlin.translation.ir.builder.{IRGremlinBindings, IRGreml
 import org.opencypher.gremlin.translation.ir.model.GremlinStep
 import org.opencypher.gremlin.translation.ir.verify.NoCustomFunctions
 import org.opencypher.gremlin.translation.preparser._
-import org.opencypher.gremlin.translation.translator.{Translator, TranslatorFlavor}
+import org.opencypher.gremlin.translation.translator.TranslatorFeature._
+import org.opencypher.gremlin.translation.translator.{Translator, TranslatorFeature, TranslatorFlavor}
 import org.opencypher.gremlin.translation.walker.StatementWalker
 import org.opencypher.gremlin.traversal.ProcedureContext
 import org.opencypher.v9_0.ast._
@@ -71,7 +72,8 @@ class CypherAst private (
         new IRGremlinPredicates,
         new IRGremlinBindings
       )
-      .allowCypherExtensions()
+      .enableCypherExtensions()
+      .enableMultipleLabels()
       .build()
 
     val context = WalkerContext(dsl, expressionTypes, returnTypes, procedures, parameters)
@@ -97,9 +99,6 @@ class CypherAst private (
     */
   def buildTranslation[T, P](dsl: Translator[T, P]): T = {
     val ir = translate(dsl.flavor(), ProcedureContext.empty())
-    if (!dsl.requiresCypherExtensions) {
-      NoCustomFunctions(ir).foreach(msg => throw new SyntaxException(msg))
-    }
     TranslationWriter.write(ir, dsl, parameters)
   }
 

diff --git a/translation/src/main/scala/org/opencypher/gremlin/translation/ir/TranslationWriter.scala b/translation/src/main/scala/org/opencypher/gremlin/translation/ir/TranslationWriter.scala
@@ -19,8 +19,11 @@ import java.util
 
 import org.apache.tinkerpop.gremlin.process.traversal.Scope
 import org.opencypher.gremlin.translation.GremlinSteps
+import org.opencypher.gremlin.translation.exception.SyntaxException
 import org.opencypher.gremlin.translation.ir.model._
-import org.opencypher.gremlin.translation.translator.Translator
+import org.opencypher.gremlin.translation.ir.verify._
+import org.opencypher.gremlin.translation.translator.{Translator, TranslatorFeature}
+import org.opencypher.gremlin.translation.translator.TranslatorFeature._
 
 import scala.collection.JavaConverters._
 
@@ -44,7 +47,15 @@ object TranslationWriter {
     write(ir, translator, parameters.asScala.toMap)
   }
 
+  private val postConditions: Map[TranslatorFeature, GremlinPostCondition] = Map(
+    CYPHER_EXTENSIONS -> NoCustomFunctions,
+    MULTIPLE_LABELS -> NoMultipleLabels
+  )
+
   def write[T, P](ir: Seq[GremlinStep], translator: Translator[T, P], parameters: Map[String, Any]): T = {
+    for ((feature, postCondition) <- postConditions if !translator.isEnabled(feature);
+         msg <- postCondition(ir)) throw new SyntaxException(msg)
+
     val generator = new TranslationWriter(translator, parameters)
     generator.writeSteps(ir, translator.steps())
     translator.translate()

diff --git a/...ation/src/main/scala/org/opencypher/gremlin/translation/ir/rewrite/GroupStepFilters.scala b/...ation/src/main/scala/org/opencypher/gremlin/translation/ir/rewrite/GroupStepFilters.scala
@@ -91,21 +91,24 @@ object GroupStepFilters extends GremlinRewriter {
   // Extracts "has" steps from a list of WHERE expressions
   private def whereExtractor(traversals: Seq[Seq[GremlinStep]]): Seq[(String, GremlinStep)] = {
     traversals.flatMap {
-      case SelectK(stepLabel) :: (hasLabel: HasLabel) :: Nil =>
-        Some((stepLabel, hasLabel))
       case SelectK(stepLabel) :: Values(propertyKey) :: Is(predicate) :: Nil =>
-        Some((stepLabel, HasP(propertyKey, predicate)))
+        (stepLabel, HasP(propertyKey, predicate)) :: Nil
+      case SelectK(stepLabel) :: rest if rest.forall(_.isInstanceOf[HasLabel]) =>
+        rest.map((stepLabel, _))
       case _ =>
-        None
+        Nil
     }
   }
 
   // Filters out relocated expressions from WHERE
   private def whereFilter(aliases: Set[String])(traversals: Seq[Seq[GremlinStep]]): Option[GremlinStep] = {
     val newTraversals = traversals.flatMap {
-      case SelectK(alias) :: (_: HasLabel) :: Nil if aliases.contains(alias)      => None
-      case SelectK(alias) :: Values(_) :: Is(_) :: Nil if aliases.contains(alias) => None
-      case other                                                                  => Some(other)
+      case SelectK(alias) :: Values(_) :: Is(_) :: Nil if aliases.contains(alias) =>
+        None
+      case SelectK(alias) :: rest if aliases.contains(alias) && rest.forall(_.isInstanceOf[HasLabel]) =>
+        None
+      case other =>
+        Some(other)
     }.toList
     newTraversals match {
       case Nil              => None

diff --git a/translation/src/main/scala/org/opencypher/gremlin/translation/ir/rewrite/NeptuneFlavor.scala b/translation/src/main/scala/org/opencypher/gremlin/translation/ir/rewrite/NeptuneFlavor.scala
@@ -16,6 +16,7 @@
 package org.opencypher.gremlin.translation.ir.rewrite
 
 import org.apache.tinkerpop.gremlin.structure.Column
+import org.opencypher.gremlin.translation.Tokens._
 import org.opencypher.gremlin.translation.ir.TraversalHelper._
 import org.opencypher.gremlin.translation.ir.model.{GremlinStep, _}
 
@@ -28,6 +29,7 @@ object NeptuneFlavor extends GremlinRewriter {
       injectWorkaround(_),
       deleteWorkaround(_),
       limit0Workaround(_),
+      multipleLabelsWorkaround(_),
       traversalRewriters(_)
     ).foldLeft(steps) { (steps, rewriter) =>
       rewriter(steps)
@@ -76,7 +78,16 @@ object NeptuneFlavor extends GremlinRewriter {
   private def limit0Workaround(steps: Seq[GremlinStep]): Seq[GremlinStep] = {
     replace({
       case Barrier :: Limit(0) :: rest =>
-        SelectK("  cypher.empty.result") :: rest
+        SelectK(NONEXISTENT) :: rest
     })(steps)
   }
+
+  private def multipleLabelsWorkaround(steps: Seq[GremlinStep]): Seq[GremlinStep] = {
+    steps match {
+      case Vertex :: (_: HasLabel) :: (_: HasLabel) :: _ =>
+        Vertex +: Is(Neq(NONEXISTENT)) +: steps.drop(1)
+      case _ =>
+        steps
+    }
+  }
 }