diff options
author | Jon Bratseth <bratseth@gmail.com> | 2022-01-06 11:35:01 +0100 |
---|---|---|
committer | Jon Bratseth <bratseth@gmail.com> | 2022-01-06 11:35:01 +0100 |
commit | 1366eb01c678345c2879ea092ab8737d0f087a89 (patch) | |
tree | 0f2d0040c1c2506068cf5ae6d2af658c630d3e1b /container-search | |
parent | c6972162154fa3e580e4a3311739f22a609f534e (diff) |
Default positive in NotItem to TRUE instead of null
Diffstat (limited to 'container-search')
13 files changed, 43 insertions, 56 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..2dc1d09e129 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 @@ -25,6 +25,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,20 +35,18 @@ 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); } @@ -72,7 +71,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 +89,7 @@ public class NotItem extends CompositeItem { boolean removed = super.removeItem(item); if (removed && removedIndex == 0) { - insertNullFirstItem(); + insertTrueFirstItem(); } return removed; } @@ -99,11 +98,15 @@ 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) {} 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/test/java/com/yahoo/prelude/query/test/QueryCanonicalizerTestCase.java b/container-search/src/test/java/com/yahoo/prelude/query/test/QueryCanonicalizerTestCase.java index 1d2f92063fe..17d5b4d23c3 100644 --- a/container-search/src/test/java/com/yahoo/prelude/query/test/QueryCanonicalizerTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/query/test/QueryCanonicalizerTestCase.java @@ -291,22 +291,22 @@ public class QueryCanonicalizerTestCase { } @Test - public void testNegativeMustHavePositive() { + public void testNegative() { NotItem root = new NotItem(); root.addNegativeItem(new WordItem("negative")); - assertCanonicalized("+(null) -negative","Can not search for only negative items", root); + assertCanonicalized("+TRUE -negative",null, root); } @Test - public void testNegativeMustHavePositiveNested() { + public void testNegativeOnly() { CompositeItem root = new AndItem(); NotItem not = new NotItem(); root.addItem(not); root.addItem(new WordItem("word")); not.addNegativeItem(new WordItem("negative")); - assertCanonicalized("AND (+(null) -negative) word","Can not search for only negative items", root); + assertCanonicalized("AND (+TRUE -negative) word",null, root); } @Test diff --git a/container-search/src/test/java/com/yahoo/prelude/semantics/test/ExactMatchTrickTestCase.java b/container-search/src/test/java/com/yahoo/prelude/semantics/test/ExactMatchTrickTestCase.java index a2ff6ae5b82..c566b05405d 100644 --- a/container-search/src/test/java/com/yahoo/prelude/semantics/test/ExactMatchTrickTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/semantics/test/ExactMatchTrickTestCase.java @@ -21,13 +21,13 @@ public class ExactMatchTrickTestCase extends RuleBaseAbstractTestCase { @Test public void testCompleteMatchWithNegative() { // Notice ordering bug - assertSemantics("+(AND default:primetime default:in default:time default:no) -regionexcl:us", + assertSemantics("+(AND default:primetime default:in default:time default:no TRUE) -regionexcl:us", new Query(QueryTestCase.httpEncode("?query=primetime ANDNOT regionexcl:us&type=adv"))); } @Test public void testCompleteMatchWithFilterAndNegative() { - assertSemantics("AND (+(AND default:primetime default:in default:time default:no) -regionexcl:us) |lang:en", + assertSemantics("AND (+(AND default:primetime default:in default:time default:no TRUE) -regionexcl:us) |lang:en", new Query(QueryTestCase.httpEncode("?query=primetime ANDNOT regionexcl:us&type=adv&filter=+lang:en"))); } diff --git a/container-search/src/test/java/com/yahoo/prelude/semantics/test/NoStemmingTestCase.java b/container-search/src/test/java/com/yahoo/prelude/semantics/test/NoStemmingTestCase.java index 7acecdcf00b..fbdd72fe6ac 100644 --- a/container-search/src/test/java/com/yahoo/prelude/semantics/test/NoStemmingTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/semantics/test/NoStemmingTestCase.java @@ -17,13 +17,13 @@ public class NoStemmingTestCase extends RuleBaseAbstractTestCase { /** Should rewrite correctly */ @Test public void testCorrectRewriting1() { - assertSemantics("+(AND i:arts i:sciences) -i:b","i:as -i:b"); + assertSemantics("+(AND i:arts i:sciences TRUE) -i:b","i:as -i:b"); } /** Should rewrite correctly too */ @Test public void testCorrectRewriting2() { - assertSemantics("+(AND i:arts i:sciences i:crafts) -i:b","i:asc -i:b"); + assertSemantics("+(AND i:arts i:sciences i:crafts TRUE) -i:b","i:asc -i:b"); } /** Should not rewrite */ diff --git a/container-search/src/test/java/com/yahoo/prelude/semantics/test/StemmingTestCase.java b/container-search/src/test/java/com/yahoo/prelude/semantics/test/StemmingTestCase.java index 136381df552..6702a1ca1d9 100644 --- a/container-search/src/test/java/com/yahoo/prelude/semantics/test/StemmingTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/semantics/test/StemmingTestCase.java @@ -16,17 +16,17 @@ public class StemmingTestCase extends RuleBaseAbstractTestCase { @Test public void testRewritingDueToStemmingInQuery() { - assertSemantics("+i:vehicle -i:s","i:cars -i:s"); + assertSemantics("+(AND i:vehicle TRUE) -i:s","i:cars -i:s"); } @Test public void testRewritingDueToStemmingInRule() { - assertSemantics("+i:animal -i:s","i:horse -i:s"); + assertSemantics("+(AND i:animal TRUE) -i:s","i:horse -i:s"); } @Test public void testRewritingDueToExactMatch() { - assertSemantics("+(AND i:arts i:sciences) -i:s","i:as -i:s"); + assertSemantics("+(AND i:arts i:sciences TRUE) -i:s","i:as -i:s"); } @Test diff --git a/container-search/src/test/java/com/yahoo/search/grouping/vespa/GroupingExecutorTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/vespa/GroupingExecutorTestCase.java index 2fda56c7454..4a7c6179db7 100644 --- a/container-search/src/test/java/com/yahoo/search/grouping/vespa/GroupingExecutorTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/grouping/vespa/GroupingExecutorTestCase.java @@ -9,6 +9,7 @@ import com.yahoo.document.GlobalId; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.prelude.fastsearch.GroupingListHit; import com.yahoo.prelude.query.NotItem; +import com.yahoo.prelude.query.NullItem; import com.yahoo.prelude.query.WordItem; import com.yahoo.search.Query; import com.yahoo.search.Result; @@ -535,18 +536,14 @@ public class GroupingExecutorTestCase { Execution exc = newExecution(new GroupingExecutor()); Query query = new Query(); - NotItem notItem = new NotItem(); - - notItem.addNegativeItem(new WordItem("negative")); - query.getModel().getQueryTree().setRoot(notItem); + query.getModel().getQueryTree().setRoot(new NullItem()); Result result = exc.search(query); com.yahoo.search.result.ErrorMessage message = result.hits().getError(); assertNotNull("Got error", message); assertEquals("Illegal query", message.getMessage()); - assertEquals("Can not search for only negative items", - message.getDetailedMessage()); + assertEquals("No query", message.getDetailedMessage()); assertEquals(3, message.getCode()); } diff --git a/container-search/src/test/java/com/yahoo/search/query/textserialize/item/test/ParseItemTestCase.java b/container-search/src/test/java/com/yahoo/search/query/textserialize/item/test/ParseItemTestCase.java index aa0d692ec92..b351bfb927a 100644 --- a/container-search/src/test/java/com/yahoo/search/query/textserialize/item/test/ParseItemTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/query/textserialize/item/test/ParseItemTestCase.java @@ -13,6 +13,7 @@ import com.yahoo.prelude.query.PrefixItem; import com.yahoo.prelude.query.RankItem; import com.yahoo.prelude.query.SubstringItem; import com.yahoo.prelude.query.SuffixItem; +import com.yahoo.prelude.query.TrueItem; import com.yahoo.prelude.query.WordItem; import com.yahoo.search.query.textserialize.item.ItemContext; import com.yahoo.search.query.textserialize.item.ItemFormHandler; @@ -71,7 +72,7 @@ public class ParseItemTestCase { @Test public void parse_and_not_rest_with_only_negated_children() throws ParseException { NotItem notItem = (NotItem) parse("(AND-NOT-REST null (WORD 'negated-item'))"); - assertNull(notItem.getPositiveItem()); + assertTrue(notItem.getPositiveItem() instanceof TrueItem); assertTrue(notItem.getItem(1) instanceof WordItem); } diff --git a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java index 55fb53b4460..351cf75b1b0 100644 --- a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java @@ -193,6 +193,12 @@ public class YqlParserTestCase { } @Test + public void testNot() { + assertParse("select foo from bar where title contains \"madonna\" and !(title contains \"saint\")", + "+title:madonna -title:saint"); + } + + @Test public void testLessThan() { assertParse("select foo from bar where price < 500", "price:<500"); assertParse("select foo from bar where 500 < price", "price:>500"); |