diff options
author | Jon Bratseth <bratseth@gmail.com> | 2024-01-15 13:30:15 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-01-15 13:30:15 +0100 |
commit | c6b547c0c2898a324983356aa677ea3082533f7d (patch) | |
tree | d6eb202d57f779e2a396bbebd829a39b710d9f36 | |
parent | 8c7f8c17ad5e1de5adcc71ee34f2a3c1cd36d6bd (diff) | |
parent | 829debe2d05daefe2444360dd138d79d09249313 (diff) |
Merge pull request #29905 from vespa-engine/revert-29884-bratseth/param-refs-in-embed
Revert "Support parameter references in embed"
8 files changed, 23 insertions, 87 deletions
diff --git a/config-model/src/test/java/com/yahoo/schema/SchemaTestCase.java b/config-model/src/test/java/com/yahoo/schema/SchemaTestCase.java index e920672646f..c959634019d 100644 --- a/config-model/src/test/java/com/yahoo/schema/SchemaTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/SchemaTestCase.java @@ -1,17 +1,11 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.schema; -import com.yahoo.document.Document; import com.yahoo.schema.document.Stemming; import com.yahoo.schema.parser.ParseException; import com.yahoo.schema.processing.ImportedFieldsResolver; import com.yahoo.schema.processing.OnnxModelTypeResolver; import com.yahoo.vespa.documentmodel.DocumentSummary; -import com.yahoo.vespa.indexinglanguage.expressions.AttributeExpression; -import com.yahoo.vespa.indexinglanguage.expressions.Expression; -import com.yahoo.vespa.indexinglanguage.expressions.InputExpression; -import com.yahoo.vespa.indexinglanguage.expressions.ScriptExpression; -import com.yahoo.vespa.indexinglanguage.expressions.StatementExpression; import com.yahoo.vespa.model.test.utils.DeployLoggerStub; import org.junit.jupiter.api.Test; diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/types/ConversionContext.java b/container-search/src/main/java/com/yahoo/search/query/profile/types/ConversionContext.java index 70f6e405a92..bef766e7ef9 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/types/ConversionContext.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/types/ConversionContext.java @@ -15,7 +15,6 @@ public class ConversionContext { private final String destination; private final CompiledQueryProfileRegistry registry; private final Map<String, Embedder> embedders; - private final Map<String, String> contextValues; private final Language language; public ConversionContext(String destination, CompiledQueryProfileRegistry registry, Embedder embedder, @@ -31,7 +30,6 @@ public class ConversionContext { this.embedders = embedders; this.language = context.containsKey("language") ? Language.fromLanguageTag(context.get("language")) : Language.UNKNOWN; - this.contextValues = context; } /** Returns the local name of the field which will receive the converted value (or null when this is empty) */ @@ -46,9 +44,6 @@ public class ConversionContext { /** Returns the language, which is never null but may be UNKNOWN */ Language language() { return language; } - /** Returns a read-only map of context key-values which can be looked up during conversion. */ - Map<String,String> contextValues() { return contextValues; } - /** Returns an empty context */ public static ConversionContext empty() { return new ConversionContext(null, null, Embedder.throwsOnUse.asMap(), Map.of()); diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/types/TensorFieldType.java b/container-search/src/main/java/com/yahoo/search/query/profile/types/TensorFieldType.java index e16f8e7b0cd..cfadd79de8f 100644 --- a/container-search/src/main/java/com/yahoo/search/query/profile/types/TensorFieldType.java +++ b/container-search/src/main/java/com/yahoo/search/query/profile/types/TensorFieldType.java @@ -48,8 +48,7 @@ public class TensorFieldType extends FieldType { @Override public Object convertFrom(Object o, ConversionContext context) { if (o instanceof SubstituteString) return new SubstituteStringTensor((SubstituteString) o, type); - return new TensorConverter(context.embedders()).convertTo(type, context.destination(), o, - context.language(), context.contextValues()); + return new TensorConverter(context.embedders()).convertTo(type, context.destination(), o, context.language()); } public static TensorFieldType fromTypeString(String s) { diff --git a/container-search/src/main/java/com/yahoo/search/query/properties/RankProfileInputProperties.java b/container-search/src/main/java/com/yahoo/search/query/properties/RankProfileInputProperties.java index 25a5c277dce..c9f935e5f52 100644 --- a/container-search/src/main/java/com/yahoo/search/query/properties/RankProfileInputProperties.java +++ b/container-search/src/main/java/com/yahoo/search/query/properties/RankProfileInputProperties.java @@ -44,8 +44,7 @@ public class RankProfileInputProperties extends Properties { value = tensorConverter.convertTo(expectedType, name.last(), value, - query.getModel().getLanguage(), - context); + query.getModel().getLanguage()); } } catch (IllegalArgumentException e) { diff --git a/container-search/src/main/java/com/yahoo/search/schema/internal/TensorConverter.java b/container-search/src/main/java/com/yahoo/search/schema/internal/TensorConverter.java index 94f92c7fd48..6da53ae699c 100644 --- a/container-search/src/main/java/com/yahoo/search/schema/internal/TensorConverter.java +++ b/container-search/src/main/java/com/yahoo/search/schema/internal/TensorConverter.java @@ -19,8 +19,7 @@ import java.util.regex.Pattern; */ public class TensorConverter { - private static final Pattern embedderArgumentAndQuotedTextRegexp = Pattern.compile("^([A-Za-z0-9_@\\-.]+),\\s*([\"'].*[\"'])"); - private static final Pattern embedderArgumentAndReferenceRegexp = Pattern.compile("^([A-Za-z0-9_@\\-.]+),\\s*(@.*)"); + private static final Pattern embedderArgumentRegexp = Pattern.compile("^([A-Za-z0-9_\\-.]+),\\s*([\"'].*[\"'])"); private final Map<String, Embedder> embedders; @@ -28,9 +27,8 @@ public class TensorConverter { this.embedders = embedders; } - public Tensor convertTo(TensorType type, String key, Object value, Language language, - Map<String, String> contextValues) { - var context = new Embedder.Context(key).setLanguage(language).setContextValues(contextValues); + public Tensor convertTo(TensorType type, String key, Object value, Language language) { + var context = new Embedder.Context(key).setLanguage(language); Tensor tensor = toTensor(type, value, context); if (tensor == null) return null; if (! tensor.type().isAssignableTo(type)) @@ -57,16 +55,16 @@ public class TensorConverter { String embedderId; // Check if arguments specifies an embedder with the format embed(embedder, "text to encode") - Matcher matcher; - if (( matcher = embedderArgumentAndQuotedTextRegexp.matcher(argument)).matches()) { + Matcher matcher = embedderArgumentRegexp.matcher(argument); + if (matcher.matches()) { embedderId = matcher.group(1); - embedder = requireEmbedder(embedderId); argument = matcher.group(2); - } else if (( matcher = embedderArgumentAndReferenceRegexp.matcher(argument)).matches()) { - embedderId = matcher.group(1); - embedder = requireEmbedder(embedderId); - argument = matcher.group(2); - } else if (embedders.isEmpty()) { + if ( ! embedders.containsKey(embedderId)) { + throw new IllegalArgumentException("Can't find embedder '" + embedderId + "'. " + + "Valid embedders are " + validEmbedders(embedders)); + } + embedder = embedders.get(embedderId); + } else if (embedders.size() == 0) { throw new IllegalStateException("No embedders provided"); // should never happen } else if (embedders.size() > 1) { throw new IllegalArgumentException("Multiple embedders are provided but no embedder id is given. " + @@ -76,35 +74,19 @@ public class TensorConverter { embedderId = entry.getKey(); embedder = entry.getValue(); } - return embedder.embed(resolve(argument, embedderContext), embedderContext.copy().setEmbedderId(embedderId), type); + return embedder.embed(removeQuotes(argument), embedderContext.copy().setEmbedderId(embedderId), type); } - private Embedder requireEmbedder(String embedderId) { - if ( ! embedders.containsKey(embedderId)) - throw new IllegalArgumentException("Can't find embedder '" + embedderId + "'. " + - "Valid embedders are " + validEmbedders(embedders)); - return embedders.get(embedderId); - } - - private static String resolve(String s, Embedder.Context embedderContext) { - if (s.startsWith("'") && s.endsWith("'")) + private static String removeQuotes(String s) { + if (s.startsWith("'") && s.endsWith("'")) { return s.substring(1, s.length() - 1); - if (s.startsWith("\"") && s.endsWith("\"")) + } + if (s.startsWith("\"") && s.endsWith("\"")) { return s.substring(1, s.length() - 1); - if (s.startsWith("@")) - return resolveReference(s, embedderContext); + } return s; } - private static String resolveReference(String s, Embedder.Context embedderContext) { - String referenceKey = s.substring(1); - String referencedValue = embedderContext.getContextValues().get(referenceKey); - if (referencedValue == null) - throw new IllegalArgumentException("Could not resolve query parameter reference '" + referenceKey + - "' used in an embed() argument"); - return referencedValue; - } - private static String validEmbedders(Map<String, Embedder> embedders) { List<String> embedderIds = new ArrayList<>(); embedders.forEach((key, value) -> embedderIds.add(key)); diff --git a/container-search/src/test/java/com/yahoo/search/query/RankProfileInputTest.java b/container-search/src/test/java/com/yahoo/search/query/RankProfileInputTest.java index 429b8d1c6cb..90e21e5f3b0 100644 --- a/container-search/src/test/java/com/yahoo/search/query/RankProfileInputTest.java +++ b/container-search/src/test/java/com/yahoo/search/query/RankProfileInputTest.java @@ -185,21 +185,6 @@ public class RankProfileInputTest { assertEmbedQuery("embed(emb2, '" + text + "')", embedding2, embedders, Language.UNKNOWN.languageCode()); } - @Test - void testUnembeddedTensorRankFeatureInRequestReferencedFromAParameter() { - String text = "text to embed into a tensor"; - Tensor embedding1 = Tensor.from("tensor<float>(x[5]):[3,7,4,0,0]]"); - - Map<String, Embedder> embedders = Map.of( - "emb1", new MockEmbedder(text, Language.UNKNOWN, embedding1) - ); - assertEmbedQuery("embed(@param1)", embedding1, embedders, null, text); - assertEmbedQuery("embed(emb1, @param1)", embedding1, embedders, null, text); - assertEmbedQueryFails("embed(emb1, @noSuchParam)", embedding1, embedders, - "Could not resolve query parameter reference 'noSuchParam' " + - "used in an embed() argument"); - } - private Query createTensor1Query(String tensorString, String profile, String additionalParams) { return new Query.Builder() .setSchemaInfo(createSchemaInfo()) @@ -217,24 +202,18 @@ public class RankProfileInputTest { } private void assertEmbedQuery(String embed, Tensor expected, Map<String, Embedder> embedders) { - assertEmbedQuery(embed, expected, embedders, null, null); + assertEmbedQuery(embed, expected, embedders, null); } private void assertEmbedQuery(String embed, Tensor expected, Map<String, Embedder> embedders, String language) { - assertEmbedQuery(embed, expected, embedders, language, null); - } - private void assertEmbedQuery(String embed, Tensor expected, Map<String, Embedder> embedders, String language, String param1Value) { String languageParam = language == null ? "" : "&language=" + language; - String param1 = param1Value == null ? "" : "¶m1=" + urlEncode(param1Value); - String destination = "query(myTensor4)"; Query query = new Query.Builder().setRequest(HttpRequest.createTestRequest( "?" + urlEncode("ranking.features." + destination) + "=" + urlEncode(embed) + "&ranking=commonProfile" + - languageParam + - param1, + languageParam, com.yahoo.jdisc.http.HttpRequest.Method.GET)) .setSchemaInfo(createSchemaInfo()) .setQueryProfile(createQueryProfile()) @@ -251,7 +230,7 @@ public class RankProfileInputTest { if (t.getMessage().equals(errMsg)) return; t = t.getCause(); } - fail("Exception with message '" + errMsg + "' not thrown"); + fail("Error '" + errMsg + "' not thrown"); } private CompiledQueryProfile createQueryProfile() { diff --git a/linguistics/abi-spec.json b/linguistics/abi-spec.json index dc6a62cc463..1ffb879e57e 100644 --- a/linguistics/abi-spec.json +++ b/linguistics/abi-spec.json @@ -344,9 +344,7 @@ "public java.lang.String getDestination()", "public com.yahoo.language.process.Embedder$Context setDestination(java.lang.String)", "public java.lang.String getEmbedderId()", - "public com.yahoo.language.process.Embedder$Context setEmbedderId(java.lang.String)", - "public java.util.Map getContextValues()", - "public com.yahoo.language.process.Embedder$Context setContextValues(java.util.Map)" + "public com.yahoo.language.process.Embedder$Context setEmbedderId(java.lang.String)" ], "fields" : [ ] }, diff --git a/linguistics/src/main/java/com/yahoo/language/process/Embedder.java b/linguistics/src/main/java/com/yahoo/language/process/Embedder.java index d9d2256d0c1..fa141977d5d 100644 --- a/linguistics/src/main/java/com/yahoo/language/process/Embedder.java +++ b/linguistics/src/main/java/com/yahoo/language/process/Embedder.java @@ -88,7 +88,6 @@ public interface Embedder { private Language language = Language.UNKNOWN; private String destination; private String embedderId = "unknown"; - private Map<String, String> contextValues; public Context(String destination) { this.destination = destination; @@ -139,15 +138,6 @@ public interface Embedder { this.embedderId = embedderId; return this; } - - /** Returns a read-only map of context key-values which can be looked up during conversion. */ - public Map<String, String> getContextValues() { return contextValues; } - - public Context setContextValues(Map<String, String> contextValues) { - this.contextValues = Map.copyOf(contextValues); - return this; - } - } class FailingEmbedder implements Embedder { |