Canonicalize features

This allows us to find the type of features referenced in
ranking expressions regardless of the form they are written in.

11 files changed, 308 insertions, 144 deletions

diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
index cd65c6ef761..136d09304da 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
@@ -16,6 +16,7 @@ import com.yahoo.searchdefinition.parser.ParseException;
 import com.yahoo.searchlib.rankingexpression.ExpressionFunction;
 import com.yahoo.searchlib.rankingexpression.FeatureList;
 import com.yahoo.searchlib.rankingexpression.RankingExpression;
+import com.yahoo.searchlib.rankingexpression.evaluation.FeatureNames;
 import com.yahoo.searchlib.rankingexpression.evaluation.TensorValue;
 import com.yahoo.searchlib.rankingexpression.evaluation.TypeMapContext;
 import com.yahoo.searchlib.rankingexpression.evaluation.Value;
@@ -746,11 +747,11 @@ public class RankProfile implements Serializable, Cloneable {
         TypeMapContext context = new TypeMapContext();
 
         // Add constants
-        getConstants().forEach((k, v) -> context.setType(asConstantFeature(k), v.type()));
+        getConstants().forEach((k, v) -> context.setType(FeatureNames.asConstantFeature(k), v.type()));
 
         // Add attributes
         for (SDField field : getSearch().allConcreteFields())
-            field.getAttributes().forEach((k, a) -> context.setType(asAttributeFeature(k), a.tensorType().orElse(TensorType.empty)));
+            field.getAttributes().forEach((k, a) -> context.setType(FeatureNames.asAttributeFeature(k), a.tensorType().orElse(TensorType.empty)));
 
         // Add query features from rank profile types reached from the "default" profile
         QueryProfile profile = queryProfilesOf(getSearch().sourceApplication()).getComponent("default");
@@ -761,7 +762,7 @@ public class RankProfile implements Serializable, Cloneable {
                     if (field.getType() instanceof TensorFieldType)
                         type = ((TensorFieldType)field.getType()).type().get();
 
-                    String feature = asQueryFeature(prefix.append(field.getName()).toString());
+                    String feature = FeatureNames.asQueryFeature(prefix.append(field.getName()).toString());
                     context.setType(feature, type);
                 }
             });
@@ -776,7 +777,7 @@ public class RankProfile implements Serializable, Cloneable {
         try {
             queryProfileFiles = applicationPackage.getQueryProfileFiles();
             queryProfileTypeFiles = applicationPackage.getQueryProfileTypeFiles();
-            return new QueryProfileXMLReader().read(queryProfileFiles, queryProfileTypeFiles);
+            return new QueryProfileXMLReader().read(queryProfileTypeFiles, queryProfileFiles);
         }
         finally {
             NamedReader.closeAll(queryProfileFiles);
@@ -784,18 +785,6 @@ public class RankProfile implements Serializable, Cloneable {
         }
     }
 
-    private String asConstantFeature(String constantName) {
-        return "constant(\"" + constantName + "\")";
-    }
-
-    private String asAttributeFeature(String constantName) {
-        return "attribute(\"" + constantName + "\")";
-    }
-
-    private String asQueryFeature(String constantName) {
-        return "query(\"" + constantName + "\")";
-    }
-
     /**
      * A rank setting. The identity of a rank setting is its field name and type (not value).
      * A rank setting is immutable.
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java
index 9c7fd7d9f0a..da85e9f65ec 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/expressiontransforms/TensorFlowFeatureConverter.java
@@ -185,6 +185,11 @@ public class TensorFlowFeatureConverter extends ExpressionTransformer<RankProfil
                                                    "' of type " + requiredType + " but this macro is not present in " +
                                                    profile);
             TensorType actualType = macro.getRankingExpression().getRoot().type(profile.typeContext());
+            if ( actualType == null)
+                throw new IllegalArgumentException("Model refers Placeholder '" + macroName +
+                                                   "' of type " + requiredType +
+                                                   " which must be produced by a macro in the rank profile, but " +
+                                                   "this macro references a feature which is not declared");
             if ( ! actualType.isAssignableTo(requiredType))
                 throw new IllegalArgumentException("Model refers Placeholder '" + macroName +
                                                    "' of type " + requiredType +
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java
index 82d0d66a82a..598ed04e657 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/RankingExpressionWithTensorFlowTestCase.java
@@ -6,6 +6,7 @@ import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.model.test.MockApplicationPackage;
 import com.yahoo.io.GrowableByteBuffer;
 import com.yahoo.io.IOUtils;
+import com.yahoo.io.reader.NamedReader;
 import com.yahoo.path.Path;
 import com.yahoo.searchdefinition.RankingConstant;
 import com.yahoo.searchdefinition.parser.ParseException;
@@ -22,6 +23,7 @@ import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Reader;
+import java.io.StringReader;
 import java.io.UncheckedIOException;
 import java.util.Arrays;
 import java.util.Collections;
@@ -48,7 +50,7 @@ public class RankingExpressionWithTensorFlowTestCase {
     }
 
     @Test
-    public void testMinimalTensorFlowReference() throws ParseException {
+    public void testTensorFlowReference() throws ParseException {
         RankProfileSearchFixture search = fixtureWith("tensor(d0[2],d1[784])(0.0)",
                                                       "tensorflow('mnist_softmax/saved')");
         search.assertFirstPhaseExpression(vespaExpression, "my_profile");
@@ -57,6 +59,23 @@ public class RankingExpressionWithTensorFlowTestCase {
     }
 
     @Test
+    public void testTensorFlowReferenceWithQueryFeature() throws ParseException {
+        String queryProfile = "<query-profile id='default' type='root'/>";
+        String queryProfileType = "<query-profile-type id='root'>" +
+                                  "  <field name='mytensor' type='tensor(d0[3],d1[784])'/>" +
+                                  "</query-profile-type>";
+        StoringApplicationPackage application = new StoringApplicationPackage(applicationDir,
+                                                                              queryProfile,
+                                                                              queryProfileType);
+        RankProfileSearchFixture search = fixtureWith("query(mytensor)",
+                                                      "tensorflow('mnist_softmax/saved')",
+                                                      application);
+        search.assertFirstPhaseExpression(vespaExpression, "my_profile");
+        assertConstant("Variable_1", search, Optional.of(10L));
+        assertConstant("Variable", search, Optional.of(7840L));
+    }
+
+    @Test
     public void testNestedTensorFlowReference() throws ParseException {
         RankProfileSearchFixture search = fixtureWith("tensor(d0[2],d1[784])(0.0)",
                                                       "5 + sum(tensorflow('mnist_softmax/saved'))");
@@ -233,14 +252,19 @@ public class RankingExpressionWithTensorFlowTestCase {
 
         private final File root;
 
+        /** The content of the single query profile and type present in this, or null if none */
+        private final String queryProfile, queryProfileType;
+
         StoringApplicationPackage(Path applicationPackageWritableRoot) {
-            this(applicationPackageWritableRoot.toFile());
+            this(applicationPackageWritableRoot, null, null);
         }
 
-        StoringApplicationPackage(File applicationPackageWritableRoot) {
+        StoringApplicationPackage(Path applicationPackageWritableRoot, String queryProfile, String queryProfileType) {
             super(null, null, Collections.emptyList(), null,
                   null, null, false);
-            this.root = applicationPackageWritableRoot;
+            this.root = new File(applicationPackageWritableRoot.toString());
+            this.queryProfile = queryProfile;
+            this.queryProfileType = queryProfileType;
         }
 
         @Override
@@ -253,6 +277,18 @@ public class RankingExpressionWithTensorFlowTestCase {
             return new StoringApplicationPackageFile(file, Path.fromString(root.toString()));
         }
 
+        @Override
+        public List<NamedReader> getQueryProfileFiles() {
+            if (queryProfile == null) return Collections.emptyList();
+            return Collections.singletonList(new NamedReader("default.xml", new StringReader(queryProfile)));
+        }
+
+        @Override
+        public List<NamedReader> getQueryProfileTypeFiles() {
+            if (queryProfileType == null) return Collections.emptyList();
+            return Collections.singletonList(new NamedReader("root.xml", new StringReader(queryProfileType)));
+        }
+
     }
 
     private static class StoringApplicationPackageFile extends ApplicationFile {
diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java b/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java
index 095d83d2887..eb4a0ad6be4 100644
--- a/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java
+++ b/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java
@@ -21,7 +21,6 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
-import java.util.logging.Logger;
 
 /**
  * A class which imports query profiles and types from XML files
@@ -30,8 +29,6 @@ import java.util.logging.Logger;
  */
 public class QueryProfileXMLReader {
 
-    private static Logger logger=Logger.getLogger(QueryProfileXMLReader.class.getName());
-
     /**
      * Reads all query profile xml files in a given directory,
      * and all type xml files from the immediate subdirectory "types/" (if any)
@@ -42,8 +39,8 @@ public class QueryProfileXMLReader {
         List<NamedReader> queryProfileReaders = new ArrayList<>();
         List<NamedReader> queryProfileTypeReaders = new ArrayList<>();
         try {
-            File dir=new File(directory);
-            if ( !dir.isDirectory() ) throw new IllegalArgumentException("Could not read query profiles: '" +
+            File dir = new File(directory);
+            if ( ! dir.isDirectory() ) throw new IllegalArgumentException("Could not read query profiles: '" +
                                                                          directory + "' is not a valid directory.");
 
             for (File file : sortFiles(dir)) {
@@ -100,21 +97,20 @@ public class QueryProfileXMLReader {
     }
 
     public List<Element> createQueryProfileTypes(List<NamedReader> queryProfileTypeReaders, QueryProfileTypeRegistry registry) {
-        List<Element> queryProfileTypeElements=new ArrayList<>(queryProfileTypeReaders.size());
+        List<Element> queryProfileTypeElements = new ArrayList<>(queryProfileTypeReaders.size());
         for (NamedReader reader : queryProfileTypeReaders) {
-            Element root=XML.getDocument(reader).getDocumentElement();
+            Element root = XML.getDocument(reader).getDocumentElement();
             if ( ! root.getNodeName().equals("query-profile-type")) {
-                logger.info("Ignoring '" + reader.getName() +
-                            "': Expected XML root element 'query-profile-type' but was '" + root.getNodeName() + "'");
-                continue;
+                throw new IllegalArgumentException("Root tag in '" + reader.getName() +
+                                                   "' must be 'query-profile-type', not '" + root.getNodeName() + "'");
             }
 
             String idString=root.getAttribute("id");
-            if (idString==null || idString.equals(""))
+            if (idString == null || idString.equals(""))
                 throw new IllegalArgumentException("'" + reader.getName() + "' has no 'id' attribute in the root element");
-            ComponentId id=new ComponentId(idString);
+            ComponentId id = new ComponentId(idString);
             validateFileNameToId(reader.getName(),id,"query profile type");
-            QueryProfileType type=new QueryProfileType(id);
+            QueryProfileType type = new QueryProfileType(id);
             type.setMatchAsPath(XML.getChild(root,"match") != null);
             type.setStrict(XML.getChild(root,"strict") != null);
             registry.register(type);
@@ -124,32 +120,33 @@ public class QueryProfileXMLReader {
     }
 
     public List<Element> createQueryProfiles(List<NamedReader> queryProfileReaders, QueryProfileRegistry registry) {
-        List<Element> queryProfileElements=new ArrayList<>(queryProfileReaders.size());
+        List<Element> queryProfileElements = new ArrayList<>(queryProfileReaders.size());
         for (NamedReader reader : queryProfileReaders) {
-            Element root=XML.getDocument(reader).getDocumentElement();
+            Element root = XML.getDocument(reader).getDocumentElement();
             if ( ! root.getNodeName().equals("query-profile")) {
-                logger.info("Ignoring '" + reader.getName() +
-                            "': Expected XML root element 'query-profile' but was '" + root.getNodeName() + "'");
-                continue;
+                throw new IllegalArgumentException("Root tag in '" + reader.getName() +
+                                                   "' must be 'query-profile', not '" + root.getNodeName() + "'");
             }
 
-            String idString=root.getAttribute("id");
-            if (idString==null || idString.equals(""))
-                throw new IllegalArgumentException("Query profile '" + reader.getName() + "' has no 'id' attribute in the root element");
-            ComponentId id=new ComponentId(idString);
-            validateFileNameToId(reader.getName(),id,"query profile");
-
-            QueryProfile queryProfile=new QueryProfile(id);
-            String typeId=root.getAttribute("type");
-            if (typeId!=null && ! typeId.equals("")) {
-                QueryProfileType type=registry.getType(typeId);
-                if (type==null)
-                    throw new IllegalArgumentException("Query profile '" + reader.getName() + "': Type id '" + typeId + "' can not be resolved");
+            String idString = root.getAttribute("id");
+            if (idString == null || idString.equals(""))
+                throw new IllegalArgumentException("Query profile '" + reader.getName() +
+                                                   "' has no 'id' attribute in the root element");
+            ComponentId id = new ComponentId(idString);
+            validateFileNameToId(reader.getName(), id, "query profile");
+
+            QueryProfile queryProfile = new QueryProfile(id);
+            String typeId = root.getAttribute("type");
+            if (typeId != null && ! typeId.equals("")) {
+                QueryProfileType type = registry.getType(typeId);
+                if (type == null)
+                    throw new IllegalArgumentException("Query profile '" + reader.getName() +
+                                                       "': Type id '" + typeId + "' can not be resolved");
                 queryProfile.setType(type);
             }
 
-            Element dimensions=XML.getChild(root,"dimensions");
-            if (dimensions!=null)
+            Element dimensions = XML.getChild(root,"dimensions");
+            if (dimensions != null)
                 queryProfile.setDimensions(toArray(XML.getValue(dimensions)));
 
             registry.register(queryProfile);
@@ -159,66 +156,72 @@ public class QueryProfileXMLReader {
     }
 
     /** Throws an exception if the name is not corresponding to the id */
-    private void validateFileNameToId(final String actualName,ComponentId id,String artifactName) {
-        String expectedCanonicalFileName=id.toFileName();
-        String expectedAlternativeFileName=id.stringValue().replace(":","-").replace("/","_"); // legacy
-        String fileName=new File(actualName).getName();
-        fileName=stripXmlEnding(fileName);
-        String canonicalFileName=ComponentId.fromFileName(fileName).toFileName();
+    private void validateFileNameToId(String actualName, ComponentId id, String artifactName) {
+        String expectedCanonicalFileName = id.toFileName();
+        String expectedAlternativeFileName = id.stringValue().replace(":", "-").replace("/", "_"); // legacy
+        String fileName = new File(actualName).getName();
+        fileName = stripXmlEnding(fileName);
+        String canonicalFileName = ComponentId.fromFileName(fileName).toFileName();
         if ( ! canonicalFileName.equals(expectedCanonicalFileName) && ! canonicalFileName.equals(expectedAlternativeFileName))
             throw new IllegalArgumentException("The file name of " + artifactName + " '" + id +
-                                               "' must be '" + expectedCanonicalFileName + ".xml' but was '" + actualName + "'");
+                                               "' must be '" + expectedCanonicalFileName + ".xml' but was '" +
+                                               actualName + "'");
     }
 
     private String stripXmlEnding(String fileName) {
-        if (!fileName.endsWith(".xml"))
+        if ( ! fileName.endsWith(".xml"))
             throw new IllegalArgumentException("'" + fileName + "' should have a .xml ending");
         else
             return fileName.substring(0,fileName.length()-4);
     }
 
     private String[] toArray(String csv) {
-        String[] array=csv.split(",");
-        for (int i=0; i<array.length; i++)
-            array[i]=array[i].trim();
+        String[] array = csv.split(",");
+        for (int i = 0; i < array.length; i++)
+            array[i] = array[i].trim();
         return array;
     }
 
     public void fillQueryProfileTypes(List<Element> queryProfileTypeElements, QueryProfileTypeRegistry registry) {
         for (Element element : queryProfileTypeElements) {
-            QueryProfileType type=registry.getComponent(new ComponentSpecification(element.getAttribute("id")).toId());
+            QueryProfileType type = registry.getComponent(new ComponentSpecification(element.getAttribute("id")).toId());
             try {
-                readInheritedTypes(element,type,registry);
-                readFieldDefinitions(element,type,registry);
+                readInheritedTypes(element, type, registry);
+                readFieldDefinitions(element, type, registry);
             }
             catch (RuntimeException e) {
-                throw new IllegalArgumentException("Error reading " + type,e);
+                throw new IllegalArgumentException("Error reading " + type, e);
             }
         }
     }
 
-    private void readInheritedTypes(Element element,QueryProfileType type,QueryProfileTypeRegistry registry) {
-        String inheritedString=element.getAttribute("inherits");
-        if (inheritedString==null || inheritedString.equals("")) return;
+    private void readInheritedTypes(Element element,QueryProfileType type, QueryProfileTypeRegistry registry) {
+        String inheritedString = element.getAttribute("inherits");
+        if (inheritedString == null || inheritedString.equals("")) return;
         for (String inheritedId : inheritedString.split(" ")) {
-            inheritedId=inheritedId.trim();
+            inheritedId = inheritedId.trim();
             if (inheritedId.equals("")) continue;
-            QueryProfileType inheritedType=registry.getComponent(inheritedId);
-            if (inheritedType==null) throw new IllegalArgumentException("Could not resolve inherited query profile type '" + inheritedId);
+            QueryProfileType inheritedType = registry.getComponent(inheritedId);
+            if (inheritedType == null)
+                throw new IllegalArgumentException("Could not resolve inherited query profile type '" + inheritedId);
             type.inherited().add(inheritedType);
         }
     }
 
-    private void readFieldDefinitions(Element element,QueryProfileType type,QueryProfileTypeRegistry registry) {
+    private void readFieldDefinitions(Element element, QueryProfileType type, QueryProfileTypeRegistry registry) {
         for (Element field : XML.getChildren(element,"field")) {
-            String name=field.getAttribute("name");
-            if (name==null || name.equals("")) throw new IllegalArgumentException("A field has no 'name' attribute");
+            String name = field.getAttribute("name");
+            if (name == null || name.equals("")) throw new IllegalArgumentException("A field has no 'name' attribute");
             try {
-                String fieldTypeName=field.getAttribute("type");
-                if (fieldTypeName==null) throw new IllegalArgumentException("Field '" + field + "' has no 'type' attribute");
+                String fieldTypeName = field.getAttribute("type");
+                if (fieldTypeName == null) throw new IllegalArgumentException("Field '" + field + "' has no 'type' attribute");
                 FieldType fieldType=FieldType.fromString(fieldTypeName,registry);
-                type.addField(new FieldDescription(name,fieldType,field.getAttribute("alias"),
-                              getBooleanAttribute("mandatory",false,field),getBooleanAttribute("overridable",true,field)), registry);
+                type.addField(new FieldDescription(name,
+                                                   fieldType,
+                                                   field.getAttribute("alias"),
+                                                   getBooleanAttribute("mandatory", false, field),
+                                                   getBooleanAttribute("overridable", true, field)),
+                              registry);
             }
             catch(RuntimeException e) {
                 throw new IllegalArgumentException("Invalid field '" + name + "'",e);
@@ -229,46 +232,51 @@ public class QueryProfileXMLReader {
     public void fillQueryProfiles(List<Element> queryProfileElements, QueryProfileRegistry registry) {
         for (Element element : queryProfileElements) {
             // Lookup by exact id
-            QueryProfile profile=registry.getComponent(new ComponentSpecification(element.getAttribute("id")).toId());
+            QueryProfile profile = registry.getComponent(new ComponentSpecification(element.getAttribute("id")).toId());
             try {
-                readInherited(element,profile,registry,null,profile.toString());
-                readFields(element,profile,registry,null,profile.toString());
-                readVariants(element,profile,registry);
+                readInherited(element, profile, registry,null, profile.toString());
+                readFields(element, profile, registry,null, profile.toString());
+                readVariants(element, profile, registry);
             }
             catch (RuntimeException e) {
-                throw new IllegalArgumentException("Error reading " + profile,e);
+                throw new IllegalArgumentException("Error reading " + profile, e);
             }
         }
     }
 
-    private void readInherited(Element element,QueryProfile profile,QueryProfileRegistry registry,DimensionValues dimensionValues,String sourceDescription) {
-        String inheritedString=element.getAttribute("inherits");
-        if (inheritedString==null || inheritedString.equals("")) return;
+    private void readInherited(Element element, QueryProfile profile, QueryProfileRegistry registry,
+                               DimensionValues dimensionValues, String sourceDescription) {
+        String inheritedString = element.getAttribute("inherits");
+        if (inheritedString == null || inheritedString.equals("")) return;
         for (String inheritedId : inheritedString.split(" ")) {
-            inheritedId=inheritedId.trim();
+            inheritedId = inheritedId.trim();
             if (inheritedId.equals("")) continue;
-            QueryProfile inheritedProfile=registry.getComponent(inheritedId);
-            if (inheritedProfile==null) throw new IllegalArgumentException("Could not resolve inherited query profile '" + inheritedId + "' in " + sourceDescription);
+            QueryProfile inheritedProfile = registry.getComponent(inheritedId);
+            if (inheritedProfile == null)
+                throw new IllegalArgumentException("Could not resolve inherited query profile '" +
+                                                   inheritedId + "' in " + sourceDescription);
             profile.addInherited(inheritedProfile,dimensionValues);
         }
     }
 
-    private void readFields(Element element,QueryProfile profile,QueryProfileRegistry registry,DimensionValues dimensionValues,String sourceDescription) {
-        List<KeyValue> references=new ArrayList<>();
-        List<KeyValue> properties=new ArrayList<>();
+    private void readFields(Element element,QueryProfile profile, QueryProfileRegistry registry,
+                            DimensionValues dimensionValues, String sourceDescription) {
+        List<KeyValue> references = new ArrayList<>();
+        List<KeyValue> properties = new ArrayList<>();
         for (Element field : XML.getChildren(element,"field")) {
-            String name=field.getAttribute("name");
-            if (name==null || name.equals("")) throw new IllegalArgumentException("A field in " + sourceDescription + " has no 'name' attribute");
+            String name = field.getAttribute("name");
+            if (name == null || name.equals(""))
+                throw new IllegalArgumentException("A field in " + sourceDescription + " has no 'name' attribute");
             try {
-                Boolean overridable=getBooleanAttribute("overridable",null,field);
-                if (overridable!=null)
-                    profile.setOverridable(name,overridable,null);
+                Boolean overridable = getBooleanAttribute("overridable",null,field);
+                if (overridable != null)
+                    profile.setOverridable(name, overridable, null);
 
-                Object fieldValue=readFieldValue(field,name,sourceDescription,registry);
+                Object fieldValue = readFieldValue(field, name, sourceDescription, registry);
                 if (fieldValue instanceof QueryProfile)
-                    references.add(new KeyValue(name,fieldValue));
+                    references.add(new KeyValue(name, fieldValue));
                 else
-                    properties.add(new KeyValue(name,fieldValue));
+                    properties.add(new KeyValue(name, fieldValue));
             }
             catch (IllegalArgumentException e) {
                 throw new IllegalArgumentException("Invalid field '" + name + "' in " + sourceDescription,e);
@@ -276,18 +284,18 @@ public class QueryProfileXMLReader {
         }
         // Must set references before properties
         for (KeyValue keyValue : references)
-            profile.set(keyValue.getKey() ,keyValue.getValue(), dimensionValues, registry);
+            profile.set(keyValue.getKey(), keyValue.getValue(), dimensionValues, registry);
         for (KeyValue keyValue : properties)
             profile.set(keyValue.getKey(), keyValue.getValue(), dimensionValues, registry);
 
     }
 
-    private Object readFieldValue(Element field,String name,String targetDescription,QueryProfileRegistry registry) {
-        Element ref=XML.getChild(field,"ref");
-        if (ref!=null) {
-            String referencedName=XML.getValue(ref);
-            QueryProfile referenced=registry.getComponent(referencedName);
-            if (referenced==null)
+    private Object readFieldValue(Element field, String name, String targetDescription, QueryProfileRegistry registry) {
+        Element ref = XML.getChild(field,"ref");
+        if (ref != null) {
+            String referencedName = XML.getValue(ref);
+            QueryProfile referenced = registry.getComponent(referencedName);
+            if (referenced == null)
                 throw new IllegalArgumentException("Could not find query profile '" + referencedName + "' referenced as '" +
                                                    name + "' in " + targetDescription);
             return referenced;
@@ -297,40 +305,41 @@ public class QueryProfileXMLReader {
         }
     }
 
-    private void readVariants(Element element,QueryProfile profile,QueryProfileRegistry registry) {
+    private void readVariants(Element element, QueryProfile profile, QueryProfileRegistry registry) {
         for (Element queryProfileVariantElement : XML.getChildren(element,"query-profile")) { // A "virtual" query profile contained inside another
-            List<String> dimensions=profile.getDimensions();
-            if (dimensions==null)
+            List<String> dimensions = profile.getDimensions();
+            if (dimensions == null)
                 throw new IllegalArgumentException("Cannot create a query profile variant in " + profile +
                                                    ", as it has not declared any variable dimensions");
-            String dimensionString=queryProfileVariantElement.getAttribute("for");
-            String[] dimensionValueArray=makeStarsNull(toArray(dimensionString));
+            String dimensionString = queryProfileVariantElement.getAttribute("for");
+            String[] dimensionValueArray = makeStarsNull(toArray(dimensionString));
             if (dimensions.size()<dimensionValueArray.length)
                 throw new IllegalArgumentException("Cannot create a query profile variant for '" + dimensionString +
                                                    "' as only " + dimensions.size() + " dimensions has been defined");
-            DimensionValues dimensionValues=DimensionValues.createFrom(dimensionValueArray);
+            DimensionValues dimensionValues = DimensionValues.createFrom(dimensionValueArray);
 
-            String description="variant '" + dimensionString + "' in " + profile.toString();
-            readInherited(queryProfileVariantElement,profile,registry,dimensionValues,description);
-            readFields(queryProfileVariantElement,profile,registry,dimensionValues,description);
+            String description = "variant '" + dimensionString + "' in " + profile.toString();
+            readInherited(queryProfileVariantElement, profile, registry, dimensionValues, description);
+            readFields(queryProfileVariantElement, profile, registry, dimensionValues, description);
         }
     }
 
     private String[] makeStarsNull(String[] strings) {
-        for (int i=0; i<strings.length; i++)
+        for (int i = 0; i < strings.length; i++)
             if (strings[i].equals("*"))
-                strings[i]=null;
+                strings[i] = null;
         return strings;
     }
 
     /**
-     * Returns true if the string is "true".<br>
-     * Returns false if the string is "false".<br>
-     * Returns <code>default</code> if the string is null or empty (this parameter may be null)<br>
+     * Returns true if the string is "true".
+     * Returns false if the string is "false".
+     * Returns <code>default</code> if the string is null or empty (this parameter may be null).
+     *
      * @throws IllegalArgumentException if the string has any other value
      */
     private Boolean asBoolean(String s,Boolean defaultValue) {
-        if (s==null) return defaultValue;
+        if (s == null) return defaultValue;
         if (s.isEmpty()) return defaultValue;
         if ("true".equals(s)) return true;
         if ("false".equals(s)) return false;
@@ -338,12 +347,12 @@ public class QueryProfileXMLReader {
     }
 
     /** Returns the given attribute as a boolean, using the semantics of {@link #asBoolean} */
-    private Boolean getBooleanAttribute(String attributeName,Boolean defaultValue,Element from) {
+    private Boolean getBooleanAttribute(String attributeName, Boolean defaultValue, Element from) {
         try {
-            return asBoolean(from.getAttribute(attributeName),defaultValue);
+            return asBoolean(from.getAttribute(attributeName), defaultValue);
         }
         catch (IllegalArgumentException e) {
-            throw new IllegalArgumentException("Attribute '" + attributeName,e);
+            throw new IllegalArgumentException("Attribute '" + attributeName, e);
         }
     }
 
@@ -352,9 +361,9 @@ public class QueryProfileXMLReader {
         private String key;
         private Object value;
 
-        public KeyValue(String key,Object value) {
-            this.key=key;
-            this.value=value;
+        public KeyValue(String key, Object value) {
+            this.key = key;
+            this.value = value;
         }
 
         public String getKey() { return key; }
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/FeatureList.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/FeatureList.java
index 1f58dbe9f9d..49466f1974d 100755
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/FeatureList.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/FeatureList.java
@@ -15,7 +15,7 @@ import java.util.List;
 /**
  * Encapsulates the production rule 'featureList()' int the RankingExpressionParser.
  *
- * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a>
+ * @author Simon Thoresen
  */
 @Beta
 public class FeatureList implements Iterable<ReferenceNode> {
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNames.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNames.java
new file mode 100644
index 00000000000..3788a252d76
--- /dev/null
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNames.java
@@ -0,0 +1,76 @@
+package com.yahoo.searchlib.rankingexpression.evaluation;// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+/**
+ * Utility methods for working with rank feature names
+ *
+ * @author bratseth
+ */
+public class FeatureNames {
+
+    private static final Pattern identifierRegexp = Pattern.compile("[A-Za-z0-9_][A-Za-z0-9_-]*");
+
+    /**
+     * Returns the given feature in canonical form.
+     * A feature name consists of a feature shortname, followed by zero or more arguments enclosed in quotes
+     * and an optional output prefixed by a dot: shortname[(argument-ist)][.output]
+     * Arguments may be identifiers or any strings single or double quoted.
+     *
+     * Argument string values may not contain comma, single quote nor double quote characters.
+     *
+     * <i>The canonical form use no quotes for arguments which are identifiers, and double quotes otherwise.</i>
+     */
+    public static String canonicalize(String feature) {
+        int startParenthesis = feature.indexOf('(');
+        int endParenthesis = feature.lastIndexOf(')');
+        if (startParenthesis < 1) return feature; // No arguments
+        if (endParenthesis < startParenthesis)
+            throw new IllegalArgumentException("A feature name must be on the form shortname[(argument-ist)][.output], " +
+                                               "but was '" + feature + "'");
+        String argumentString = feature.substring(startParenthesis + 1, endParenthesis);
+        List<String> canonicalizedArguments =
+                Arrays.stream(argumentString.split(","))
+                      .map(FeatureNames::canonicalizeArgument)
+                      .collect(Collectors.toList());
+        return feature.substring(0, startParenthesis + 1) +
+               canonicalizedArguments.stream().collect(Collectors.joining(",")) +
+               feature.substring(endParenthesis);
+    }
+
+    /** Canomicalizes a single argument */
+    private static String canonicalizeArgument(String argument) {
+        if (argument.startsWith("'")) {
+            if ( ! argument.endsWith("'"))
+                throw new IllegalArgumentException("Feature arguments starting by a single quote " +
+                                                   "must end by a single quote, but was \"" + argument + "\"");
+            argument = argument.substring(1, argument.length() - 1);
+        }
+        if (argument.startsWith("\"")) {
+            if ( ! argument.endsWith("\""))
+                throw new IllegalArgumentException("Feature arguments starting by a double quote " +
+                                                   "must end by a double quote, but was '" + argument + "'");
+            argument = argument.substring(1, argument.length() - 1);
+        }
+        if (identifierRegexp.matcher(argument).matches())
+            return argument;
+        else
+            return "\"" + argument + "\"";
+    }
+
+    public static String asConstantFeature(String constantName) {
+        return canonicalize("constant(\"" + constantName + "\")");
+    }
+
+    public static String asAttributeFeature(String attributeName) {
+        return canonicalize("attribute(\"" + attributeName + "\")");
+    }
+
+    public static String asQueryFeature(String propertyName) {
+        return canonicalize("query(\"" + propertyName + "\")");
+    }
+
+}
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/MapContext.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/MapContext.java
index 39efe641f26..333af529cb9 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/MapContext.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/MapContext.java
@@ -24,13 +24,10 @@ public class MapContext extends Context {
 
     /**
      * Creates a map context from a map.
-     * The ownership of the map is transferred to this - it cannot be further modified by the caller.
      * All the Values of the map will be frozen.
      */
     public MapContext(Map<String,Value> bindings) {
-        this.bindings = bindings;
-        for (Value boundValue : bindings.values())
-            boundValue.freeze();
+        bindings.forEach((k, v) -> this.bindings.put(FeatureNames.canonicalize(k), v.freeze()));
     }
 
     /**
@@ -46,7 +43,7 @@ public class MapContext extends Context {
     /** Returns the type of the given value key, or null if it is not bound. */
     @Override
     public TensorType getType(String key) {
-        Value value = bindings.get(key);
+        Value value = bindings.get(FeatureNames.canonicalize(key));
         if (value == null) return null;
         return value.type();
     }
@@ -54,19 +51,19 @@ public class MapContext extends Context {
     /** Returns the value of a key. 0 is returned if the given key is not bound in this. */
     @Override
     public Value get(String key) {
-        return bindings.getOrDefault(key, DoubleValue.zero);
+        return bindings.getOrDefault(FeatureNames.canonicalize(key), DoubleValue.zero);
     }
 
     /**
-     * Sets the value of a key.The value is frozen by this.
+     * Sets the value of a key. The value is frozen by this.
      */
     @Override
     public void put(String key,Value value) {
-        bindings.put(key,value.freeze());
+        bindings.put(FeatureNames.canonicalize(key), value.freeze());
     }
 
     /** Returns an immutable view of the bindings of this. */
-    public Map<String,Value> bindings() {
+    public Map<String, Value> bindings() {
         if (frozen) return bindings;
         return Collections.unmodifiableMap(bindings);
     }
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeMapContext.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeMapContext.java
index a018aae0c3e..0335ead4420 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeMapContext.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeMapContext.java
@@ -4,6 +4,7 @@ package com.yahoo.searchlib.rankingexpression.evaluation;// Copyright 2018 Yahoo
 import com.yahoo.tensor.TensorType;
 import com.yahoo.tensor.evaluation.TypeContext;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -17,12 +18,15 @@ public class TypeMapContext implements TypeContext {
     private final Map<String, TensorType> featureTypes = new HashMap<>();
 
     public void setType(String name, TensorType type) {
-        featureTypes.put(name, type);
+        featureTypes.put(FeatureNames.canonicalize(name), type);
     }
 
     @Override
     public TensorType getType(String name) {
-        return featureTypes.get(name);
+        return featureTypes.get(FeatureNames.canonicalize(name));
     }
 
+    /** Returns an unmodifiable map of the bindings in this */
+    public Map<String, TensorType> bindings() { return Collections.unmodifiableMap(featureTypes); }
+
 }
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java
index f79297f7773..05a6773c5cb 100755
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java
@@ -108,7 +108,7 @@ public final class ReferenceNode extends CompositeNode {
     @Override
     public TensorType type(TypeContext context) {
         // Don't support outputs of different type, for simplicity
-        return context.getType(name);
+        return context.getType(toString());
     }
 
     @Override
diff --git a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNamesTestCase.java b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNamesTestCase.java
new file mode 100644
index 00000000000..6ea980b4d5b
--- /dev/null
+++ b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/FeatureNamesTestCase.java
@@ -0,0 +1,48 @@
+package com.yahoo.searchlib.rankingexpression.evaluation;// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Tests rank feature names.
+ *
+ * @author bratseth
+ */
+public class FeatureNamesTestCase {
+
+    @Test
+    public void testCanonicalization() {
+        assertEquals("foo", FeatureNames.canonicalize("foo"));
+        assertEquals("foo.out", FeatureNames.canonicalize("foo.out"));
+        assertEquals("foo(bar)", FeatureNames.canonicalize("foo(bar)"));
+        assertEquals("foo(bar)", FeatureNames.canonicalize("foo('bar')"));
+        assertEquals("foo(bar)", FeatureNames.canonicalize("foo(\"bar\")"));
+        assertEquals("foo(bar).out", FeatureNames.canonicalize("foo(bar).out"));
+        assertEquals("foo(bar).out", FeatureNames.canonicalize("foo('bar').out"));
+        assertEquals("foo(bar).out", FeatureNames.canonicalize("foo(\"bar\").out"));
+        assertEquals("foo(\"ba.r\")", FeatureNames.canonicalize("foo(ba.r)"));
+        assertEquals("foo(\"ba.r\")", FeatureNames.canonicalize("foo('ba.r')"));
+        assertEquals("foo(\"ba.r\")", FeatureNames.canonicalize("foo(\"ba.r\")"));
+        assertEquals("foo(bar1,\"b.ar2\",\"ba/r3\").out",
+                     FeatureNames.canonicalize("foo(bar1,b.ar2,ba/r3).out"));
+        assertEquals("foo(bar1,\"b.ar2\",\"ba/r3\").out",
+                     FeatureNames.canonicalize("foo(bar1,'b.ar2',\"ba/r3\").out"));
+    }
+
+    @Test
+    public void testConstantFeature() {
+        assertEquals("constant(\"foo/bar\")", FeatureNames.asConstantFeature("foo/bar"));
+    }
+
+    @Test
+    public void testAttributeFeature() {
+        assertEquals("attribute(foo)", FeatureNames.asAttributeFeature("foo"));
+    }
+
+    @Test
+    public void testQueryFeature() {
+        assertEquals("query(\"foo.bar\")", FeatureNames.asQueryFeature("foo.bar"));
+    }
+
+}
diff --git a/vespajlib/src/main/java/com/yahoo/tensor/evaluation/TypeContext.java b/vespajlib/src/main/java/com/yahoo/tensor/evaluation/TypeContext.java
index 4d3bd04c3d4..760a225efdf 100644
--- a/vespajlib/src/main/java/com/yahoo/tensor/evaluation/TypeContext.java
+++ b/vespajlib/src/main/java/com/yahoo/tensor/evaluation/TypeContext.java
@@ -11,7 +11,7 @@ import com.yahoo.tensor.TensorType;
 public interface TypeContext {
 
     /**
-     * Returns tye type of the tensor with this name.
+     * Returns the type of the tensor with this name.
      *
      * @return returns the type of the tensor which will be returned by calling getTensor(name)
      *         or null if getTensor will return null.