diff options
Diffstat (limited to 'container-search/src/main/java/com')
8 files changed, 50 insertions, 64 deletions
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/CompositeItem.java b/container-search/src/main/java/com/yahoo/prelude/query/CompositeItem.java index aaa4d33c6dc..d3fbeb020f8 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/CompositeItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/CompositeItem.java @@ -80,11 +80,6 @@ public abstract class CompositeItem extends Item { subitems.add(index, item); } - /** For NOT items, which may wish to insert nulls */ - void insertNullFirstItem() { - subitems.add(0, null); - } - /** * Returns a subitem * @@ -109,7 +104,7 @@ public abstract class CompositeItem extends Item { adding(item); Item old = subitems.set(index, item); - if (old!=item) + if (old != item) removing(old); return old; } @@ -188,9 +183,7 @@ public abstract class CompositeItem extends Item { return itemCount; } - /** - * Encodes just this item, not it's usual subitems, to the given buffer. - */ + /** Encodes just this item, not its regular subitems, to the given buffer. */ protected void encodeThis(ByteBuffer buffer) { super.encodeThis(buffer); IntegerCompressor.putCompressedPositiveNumber(encodingArity(), buffer); @@ -279,10 +272,7 @@ public abstract class CompositeItem extends Item { return code; } - /** - * Returns whether this item is of the same class and - * contains the same state as the given item - */ + /** Returns whether this item is of the same class and contains the same state as the given item. */ @Override public boolean equals(Object object) { if (!super.equals(object)) return false; @@ -303,17 +293,12 @@ public abstract class CompositeItem extends Item { @Override public int getTermCount() { int terms = 0; - for (Item item : subitems) { + for (Item item : subitems) terms += item.getTermCount(); - } return terms; } - /** - * Will return its single child if itself can safely be omitted. - * - * @return a valid Item or empty Optional if it can not be done - */ + /** Returns the single child of this, if this can be omitted without changes to recall semantics. */ public Optional<Item> extractSingleChild() { return getItemCount() == 1 ? Optional.of(getItem(0)) : Optional.empty(); } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/NotItem.java b/container-search/src/main/java/com/yahoo/prelude/query/NotItem.java index 833b8635f61..6985ed0913c 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/NotItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/NotItem.java @@ -7,11 +7,12 @@ import java.util.Objects; /** * A composite item where the first item is positive and the following - * items are negative items which should be excluded from the result. + * items are negative items where matches should exclude the document should from the result. + * The default positive item, if only negatives are added, is TrueItem: Meaning that all documents are matched + * except those matching the negative terms added. * * @author bratseth */ -// TODO: Handle nulls by creating nullItem or checking in encode/toString public class NotItem extends CompositeItem { @Override @@ -25,6 +26,7 @@ public class NotItem extends CompositeItem { } /** Adds an item. The first item is the positive, the rest are negative */ + @Override public void addItem(Item item) { super.addItem(item); } @@ -34,27 +36,25 @@ public class NotItem extends CompositeItem { * (position 0) if it is not already set. */ public void addNegativeItem(Item negative) { - if (getItemCount() == 0) { - insertNullFirstItem(); - } + if (getItemCount() == 0) + insertTrueFirstItem(); addItem(negative); } /** Returns the negative items of this: All child items except the first */ public List<Item> negativeItems() { return items().subList(1, getItemCount()); } - /** Returns the positive item (the first subitem), or null if no positive items has been added. */ + /** Returns the positive item (the first subitem), or TrueItem if no positive items has been added. */ public Item getPositiveItem() { - if (getItemCount() == 0) { - return null; - } + if (getItemCount() == 0) + return new TrueItem(); return getItem(0); } /** * Sets the positive item (the first item) * - * @return the old positive item, or null if there was none + * @return the old positive item, or TrueItem if there was none */ public Item setPositiveItem(Item item) { Objects.requireNonNull(item, () -> "Positive item of " + this); @@ -72,7 +72,7 @@ public class NotItem extends CompositeItem { * the positive item becomes an AndItem with the items added */ public void addPositiveItem(Item item) { - if (getPositiveItem() == null) { + if (getPositiveItem() instanceof TrueItem) { setPositiveItem(item); } else if (getPositiveItem() instanceof AndItem) { ((AndItem) getPositiveItem()).addItem(item); @@ -90,7 +90,7 @@ public class NotItem extends CompositeItem { boolean removed = super.removeItem(item); if (removed && removedIndex == 0) { - insertNullFirstItem(); + insertTrueFirstItem(); } return removed; } @@ -99,35 +99,35 @@ public class NotItem extends CompositeItem { Item removed = super.removeItem(index); if (index == 0) { // Don't make the first negative the positive - insertNullFirstItem(); + insertTrueFirstItem(); } return removed; } + private void insertTrueFirstItem() { + addItem(0, new TrueItem()); + } + /** Not items uses a empty heading instead of "NOT " */ protected void appendHeadingString(StringBuilder buffer) {} /** - * Overridden to tolerate nulls and to append "+" + * Overridden to skip the positive TrueItem and (otherwise) append "+" * to the first item and "-" to the rest */ + @Override protected void appendBodyString(StringBuilder buffer) { - boolean isFirstItem = true; - - for (Iterator<Item> i = getItemIterator(); i.hasNext();) { - Item item = i.next(); - - if (isFirstItem) { - buffer.append("+"); - } else { - buffer.append(" -"); - } - if (item == null) { - buffer.append("(null)"); - } else { - buffer.append(item.toString()); - } - isFirstItem = false; + if (items().isEmpty()) return; + if (items().size() == 1) { + buffer.append(items().get(0)); + return; + } + for (int i = 0; i < items().size(); i++) { + if (i == 0 && items().get(i) instanceof TrueItem) continue; // skip positive true + + buffer.append(i == 0 ? "+" : "-").append(items().get(i)); + if ( i < items().size() - 1) + buffer.append(" "); } } diff --git a/container-search/src/main/java/com/yahoo/prelude/query/QueryCanonicalizer.java b/container-search/src/main/java/com/yahoo/prelude/query/QueryCanonicalizer.java index 1f30833b3db..8c4c5c84a28 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/QueryCanonicalizer.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/QueryCanonicalizer.java @@ -77,11 +77,6 @@ public class QueryCanonicalizer { else if (composite instanceof RankItem) { makeDuplicatesCheap((RankItem)composite); } - else if (composite instanceof NotItem) { - if (((NotItem) composite).getPositiveItem() == null) - return CanonicalizationResult.error("Can not search for only negative items"); - } - if (composite.getItemCount() == 0) parentIterator.remove(); diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java index 087da13a937..020d93d951c 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/SimpleParser.java @@ -121,7 +121,7 @@ abstract class SimpleParser extends StructuredParser { return combineItems(topLevelItem, not.getPositiveItem()); } } - if (not != null && not.getPositiveItem() == null) { + if (not != null && not.getPositiveItem() instanceof TrueItem) { // Incomplete not, only negatives - if (topLevelItem != null && topLevelItem != not) { diff --git a/container-search/src/main/java/com/yahoo/prelude/querytransform/LiteralBoostSearcher.java b/container-search/src/main/java/com/yahoo/prelude/querytransform/LiteralBoostSearcher.java index e9e5818cefe..47a5213c041 100644 --- a/container-search/src/main/java/com/yahoo/prelude/querytransform/LiteralBoostSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/querytransform/LiteralBoostSearcher.java @@ -71,8 +71,6 @@ public class LiteralBoostSearcher extends Searcher { } private void addLiterals(RankItem rankTerms, Item item, IndexFacts.Session indexFacts) { - if (item == null) return; - if (item instanceof NotItem) { addLiterals(rankTerms, ((NotItem) item).getPositiveItem(), indexFacts); } diff --git a/container-search/src/main/java/com/yahoo/search/query/textserialize/item/AndNotRestConverter.java b/container-search/src/main/java/com/yahoo/search/query/textserialize/item/AndNotRestConverter.java index b64809e8071..795a78157c5 100644 --- a/container-search/src/main/java/com/yahoo/search/query/textserialize/item/AndNotRestConverter.java +++ b/container-search/src/main/java/com/yahoo/search/query/textserialize/item/AndNotRestConverter.java @@ -13,6 +13,7 @@ import static com.yahoo.search.query.textserialize.item.ListUtil.first; * @author Tony Vaagenes */ public class AndNotRestConverter extends CompositeConverter<NotItem> { + static final String andNotRest = "AND-NOT-REST"; public AndNotRestConverter() { @@ -51,4 +52,5 @@ public class AndNotRestConverter extends CompositeConverter<NotItem> { protected String getFormName(Item item) { return andNotRest; } + } diff --git a/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java b/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java index f81083221a8..94b7c140b0f 100644 --- a/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java +++ b/container-search/src/main/java/com/yahoo/search/searchers/ValidateNearestNeighborSearcher.java @@ -102,7 +102,7 @@ public class ValidateNearestNeighborSearcher extends Searcher { String queryFeatureName = "query(" + item.getQueryTensorName() + ")"; Optional<Tensor> queryTensor = query.getRanking().getFeatures().getTensor(queryFeatureName); if (queryTensor.isEmpty()) - return item + " requires a tensor rank feature " + queryFeatureName + " but this is not present"; + return item + " requires a tensor rank feature named '" + queryFeatureName + "' but this is not present"; if ( ! validAttributes.containsKey(item.getIndexName())) { return item + " field is not an attribute"; diff --git a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java index a81f90aa18e..c0c5b0ee0b0 100644 --- a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java +++ b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java @@ -356,6 +356,8 @@ public class YqlParser implements Parser { return buildFunctionCall(ast); case LITERAL: return buildLiteral(ast); + case NOT: + return buildNot(ast); default: throw newUnexpectedArgumentException(ast.getOperator(), ExpressionOperator.AND, ExpressionOperator.CALL, @@ -1096,17 +1098,21 @@ public class YqlParser implements Parser { AndItem andItem = new AndItem(); NotItem notItem = new NotItem(); convertVarArgsAnd(ast, 0, andItem, notItem); - Preconditions - .checkArgument(andItem.getItemCount() > 0, - "Vespa does not support AND with no logically positive branches."); if (notItem.getItemCount() == 0) { return andItem; } if (andItem.getItemCount() == 1) { notItem.setPositiveItem(andItem.getItem(0)); - } else { + } else if (andItem.getItemCount() > 1) { notItem.setPositiveItem(andItem); - } + } // else no positives, which is ok + return notItem; + } + + /** Build a "pure" not, without any positive terms. */ + private CompositeItem buildNot(OperatorNode<ExpressionOperator> ast) { + NotItem notItem = new NotItem(); + notItem.addNegativeItem(convertExpression(ast.getArgument(0))); return notItem; } |