Operators |= and &= with Xapian::Query{}

[Дилян Палаузов]

I do expect, that Xapian::Query{} constructs an empty query, that is the neutral element towards AND and OR operations. Thus in:

std::vector<std::string> v {"A", "B"};
Xapian::Query q1, q2, qOR {Xapian::Query::OP_OR, v.begin(), v.end()}, qAND {Xapian::Query::OP_OR, v.begin(), v.end()}

q1 |= qOR → q1.get_type() will be OR and the q1 terms will be A and B

q2 &= qAND → q2.get_type() will be AND and the q2 terms will be A and B

Instead q1.get_type() == q2.get_type() == LEAF_MATCH_NOTHING and contain no terms.

Reproducer:

#include <stdio.h>
#include <string>
#include <xapian.h>
void print_terms(const Xapian::Query& f, std::string s) {
  printf("%s: ", s.c_str());
  for (Xapian::TermIterator it = f.get_terms_begin();  it != f.get_terms_end(); ++it)
    printf("%s ", (*it).c_str());
  printf(" get_type=%i\n", f.get_type());
}

int main() {
  std::vector<std::string> v {"abc", "def", "ghi"};
  Xapian::Query t;
  print_terms(t, "t");
  Xapian::Query q { Xapian::Query::OP_OR, v.begin(), v.end()};
  print_terms(q, "q");
  t &= q;
  print_terms(t, "t2");
  return 0;
}

-- actual result
t:  get_type=103
q: abc def ghi  get_type=1
t2:  get_type=103

-- expected result
t:  get_type=103
q: abc def ghi  get_type=1
t2: abc def ghi get_type=1

[Olly Betts]

I do expect, that Xapian::Query{} constructs an empty query, that is the neutral element towards AND and OR operations

Long, long ago (probably 15-20 years) we tried making a default-constructed query work in the context-sensitive way you're expecting, but concluded it was more confusing than helpful.

Now Xapian::Query{} works just like int{} (which is 0) in bitwise logical operations or bool{} (which is false) in boolean logical operations. So Xapian::Query{} gives you a query which matches nothing (and Xapian::Query::MatchNothing is just a static instance of this).

It's easy to handle the case of accumulating onto a query with AND:

if (t.empty()) {
    t = q;
} else {
    t &= q;
}

(And t.empty() inlines to a NULL pointer test so the extra check is low overhead.)

q1 |= qOR → q1.get_type() will be OR and the q1 terms will be A and B q2 &= qAND → q2.get_type() will be AND and the q2 terms will be A and B

Instead q1.get_type() == q2.get_type() == LEAF_MATCH_NOTHING and contain no terms.

I tested with current RELEASE/1.4 branch (this code hasn't changed since 1.4.11) and I can't reproduce q1.get_type() == LEAF_MATCH_NOTHING - instead I get q1.get_type() == OP_OR (which is what I'd expect).

The query operators do simple folding at construction time around Query{}, so for q2 the result is simplified to Query() rather than giving you an OP_AND query, and q1 is actually qOR.

The API documentation for the default Query constructor just says "Default constructor." which is uselessly vague. I'll improve that for the next release.

[Дилян Палаузов]

#include <stdio.h>
#include <xapian.h>

int main() {
  std::vector<std::string> v;
  printf("%i\n", Xapian::Query {Xapian::Query::OP_OR, v.begin(), v.end()}.get_type());
}

prints 103 = LEAF_MATCH_NOTHING, which is problematic if the query is later appended with OP_AND to another query. If v is empty, that query will never match.

[Olly Betts]

Xapian::Query{Xapian::Query::OP_OR, v.begin(), v.end()} is a query for any documents which match any of the terms in v. If v.empty() then no documents match any of the terms in v, so this constructs a MatchNothing query.

If you want to filter a query by OR of terms in v but handle empty v as meaning "don't apply a filter" you want something like:

if (!v.empty()) {
    q = Xapian::Query{Xapian::Query::OP_FILTER, q, Xapian::Query{Xapian::Query::OP_OR, v.begin(), v.end()}};
}

[Olly Betts]

I've made a lot of improvements to the Query class API docs in [00f69cf3928b44a756ddafcde248610f72babf62] (backported for 1.4.12 in [976ea09888503b9c2dc188c9b8111f928b3c19d4]).

I think that addresses all the issues raised here, so I'm going to close this. It wouldn't be feasible to change how Query() or a query constructed from an empty iterator pair works at this point as it would force an incompatible change on existing code using the API.

We could perhaps make Query(OP_INVALID) behave like you expected Query() to - if you or somebody else thinks that would be beneficial, we could try it and see how it looks in various situations, and whether it's actually helpful or just confusing.