Ticket #280: patch

tests/internaltest.cc

@@ -451 +451 @@
     for (query = queries.begin(); query != queries.end(); query++) {
         Xapian::Query::Internal * qint;
 
-        s = query->internal->serialise();
+        s = query->internalptr->get()->serialise();
         qint = Xapian::Query::Internal::unserialise(s);
 
         TEST(qint->serialise() == s);

include/xapian/query.h

@@ -33 +33 @@
 #include <xapian/termiterator.h>
 #include <xapian/visibility.h>
 
-// FIXME: sort this out so we avoid exposing Xapian::Query::Internal
-// - we need to at present so that the Xapian::Query's template ctors
-// compile.
-class LocalSubMatch;
-class MultiMatch;
-class QueryOptimiser;
-struct SortPosName;
-
 namespace Xapian {
 
 class PostingSource;
@@ -53 +45 @@
     public:
         /// Class holding details of the query
         class Internal;
-        /// @private @internal Reference counted internals.
-        Xapian::Internal::RefCntPtr<Internal> internal;
 
+        /// Refcount pointer to class holding details of the query.
+        struct InternalPtr;
+
+        /// @private @internal Pointer to reference counted internals.
+        InternalPtr * internalptr;
+
         /// Enum of possible query operations
         typedef enum {
             /// Return iff both subqueries are satisfied
@@ -252 +248 @@
 
 template <class Iterator>
 Query::Query(Query::op op_, Iterator qbegin, Iterator qend, termcount parameter)
-    : internal(0)
+    : internalptr(0)
 {
     try {
         start_construction(op_, parameter);
@@ -270 +266 @@
     }
 }
 
-#ifndef SWIG // SWIG has no interest in the internal class, so hide it completely.
-
-/// @internal Internal class, implementing most of Xapian::Query.
-class XAPIAN_VISIBILITY_DEFAULT Query::Internal : public Xapian::Internal::RefCntBase {
-    friend class ::LocalSubMatch;
-    friend class ::MultiMatch;
-    friend class ::QueryOptimiser;
-    friend struct ::SortPosName;
-    friend class Query;
-    public:
-        static const int OP_LEAF = -1;
-        static const int OP_EXTERNAL_SOURCE = -2;
-
-        /// The container type for storing pointers to subqueries
-        typedef std::vector<Internal *> subquery_list;
-
-        /// Type storing the operation
-        typedef int op_t;
-
-    private:
-        /// Operation to be performed at this node
-        Xapian::Query::Internal::op_t op;
-
-        /// Sub queries on which to perform operation
-        subquery_list subqs;
-
-        /** For NEAR or PHRASE, how close terms must be to match: all terms
-         *  within the operation must occur in a window of this size.
-         *
-         * For ELITE_SET, the number of terms to select from those specified.
-         *
-         * For RANGE, the value number to apply the range test to.
-         */
-        Xapian::termcount parameter;
-
-        /** Term that this node represents, or start of a range query.
-         *
-         *  For a leaf node, this holds the term name.  For an OP_VALUE_RANGE
-         *  query this holds the start of the range.  For an OP_VALUE_GE or
-         *  OP_VALUE_LE query this holds the value to compare against.
-         */
-        std::string tname;
-
-        /** Used to store the end of a range query. */
-        std::string str_parameter;
-
-        /// Position in query of this term - leaf node only
-        Xapian::termpos term_pos;
-
-        /// Within query frequency of this term - leaf node only
-        Xapian::termcount wqf;
-
-        /// External posting source.
-        Xapian::PostingSource * external_source;
-
-        /** swap the contents of this with another Xapian::Query::Internal,
-         *  in a way which is guaranteed not to throw.  This is
-         *  used with the assignment operator to make it exception
-         *  safe.
-         *  It's important to adjust swap with any addition of
-         *  member variables!
-         */
-        void swap(Query::Internal &other);
-
-        /// Copy another Xapian::Query::Internal into self.
-        void initialise_from_copy(const Query::Internal & copyme);
-
-        void accumulate_terms(
-            std::vector<std::pair<std::string, Xapian::termpos> > &terms) const;
-
-        /** Simplify the query.
-         *  For example, an AND query with only one subquery would become the
-         *  subquery itself.
-         */
-        Internal * simplify_query();
-
-        /** Perform checks that query is valid. (eg, has correct number of
-         *  sub queries.)  Throw an exception if not.  This is initially called
-         *  on the query before any simplifications have been made, and after
-         *  simplications.
-         */
-        void validate_query() const;
-
-        /** Simplify any matchnothing subqueries, either eliminating them,
-         *  or setting this query to matchnothing, depending on the query
-         *  operator.  Returns true if simplification resulted in a
-         *  matchnothing query.
-         */
-        bool simplify_matchnothing();
-
-        /** Get a string describing the given query type.
-         */
-        static std::string get_op_name(Xapian::Query::Internal::op_t op);
-
-        /** Collapse the subqueries together if appropriate.
-         */
-        void collapse_subqs();
-
-        /** Flatten a query structure, by changing, for example,
-         *  "A NEAR (B AND C)" to "(A NEAR B) AND (A NEAR C)"
-         */
-        void flatten_subqs();
-
-        /** Implementation of serialisation; called recursively.
-         */
-        std::string serialise(Xapian::termpos & curpos) const;
-
-    public:
-        /** Copy constructor. */
-        Internal(const Query::Internal & copyme);
-
-        /** Assignment. */
-        void operator=(const Query::Internal & copyme);
-
-        /** A query consisting of a single term. */
-        explicit Internal(const std::string & tname_, Xapian::termcount wqf_ = 1,
-                          Xapian::termpos term_pos_ = 0);
-
-        /** Create internals given only the operator and a parameter. */
-        Internal(op_t op_, Xapian::termcount parameter);
-
-        /** Construct a range query on a document value. */
-        Internal(op_t op_, Xapian::valueno valno,
-                 const std::string &begin, const std::string &end);
-
-        /** Construct a value greater-than-or-equal query on a document value.
-         */
-        Internal(op_t op_, Xapian::valueno valno, const std::string &value);
-
-        /// Construct an external source query.
-        explicit Internal(Xapian::PostingSource * external_source_);
-
-        /** Destructor. */
-        ~Internal();
-
-        static Xapian::Query::Internal * unserialise(const std::string &s);
-
-        /** Add a subquery.
-         */
-        void add_subquery(const Query::Internal * subq);
-
-        void set_dbl_parameter(double dbl_parameter_);
-
-        double get_dbl_parameter() const;
-
-        /** Finish off the construction.
-         */
-        Query::Internal * end_construction();
-
-        /** Return a string in an easily parsed form
-         *  which contains all the information in a query.
-         */
-        std::string serialise() const {
-            Xapian::termpos curpos = 1;
-            return serialise(curpos);
-        }
-
-        /// Return a string describing this object.
-        std::string get_description() const;
-
-        /** Get the numeric parameter used in this query.
-         *
-         *  This is used by the QueryParser to get the value number for
-         *  VALUE_RANGE queries.  It should be replaced by a public method on
-         *  the Query class at some point, but the API which should be used for
-         *  that is unclear, so this is a temporary workaround.
-         */
-        Xapian::termcount get_parameter() const { return parameter; }
-
-        /** Get the length of the query, used by some ranking formulae.
-         *  This value is calculated automatically - if you want to override
-         *  it you can pass a different value to Enquire::set_query().
-         */
-        Xapian::termcount get_length() const;
-
-        /** Return an iterator over all the terms in the query,
-         *  in order of termpos.  If multiple terms have the same term
-         *  position, their order is unspecified.  Duplicates (same term and
-         *  termpos) will be removed.
-         */
-        TermIterator get_terms() const;
-};
-
-#endif // SWIG
-
 }
 
 #endif /* XAPIAN_INCLUDED_QUERY_H */

common/omqueryinternal.h

@@ -24 +24 @@
 // because of the templated members of Xapian::Query.
 #include <xapian/query.h>
 
+class LocalSubMatch;
+class MultiMatch;
+class QueryOptimiser;
+struct SortPosName;
+
+namespace Xapian {
+
+/// @internal Internal class, implementing most of Xapian::Query.
+class XAPIAN_VISIBILITY_DEFAULT Query::Internal : public Xapian::Internal::RefCntBase {
+    friend class ::LocalSubMatch;
+    friend class ::MultiMatch;
+    friend class ::QueryOptimiser;
+    friend struct ::SortPosName;
+    friend class Query;
+    public:
+        static const int OP_LEAF = -1;
+        static const int OP_EXTERNAL_SOURCE = -2;
+
+        /// The container type for storing pointers to subqueries
+        typedef std::vector<Internal *> subquery_list;
+
+        /// Type storing the operation
+        typedef int op_t;
+
+    private:
+        /// Operation to be performed at this node
+        Xapian::Query::Internal::op_t op;
+
+        /// Sub queries on which to perform operation
+        subquery_list subqs;
+
+        /** For NEAR or PHRASE, how close terms must be to match: all terms
+         *  within the operation must occur in a window of this size.
+         *
+         * For ELITE_SET, the number of terms to select from those specified.
+         *
+         * For RANGE, the value number to apply the range test to.
+         */
+        Xapian::termcount parameter;
+
+        /** Term that this node represents, or start of a range query.
+         *
+         *  For a leaf node, this holds the term name.  For an OP_VALUE_RANGE
+         *  query this holds the start of the range.  For an OP_VALUE_GE or
+         *  OP_VALUE_LE query this holds the value to compare against.
+         */
+        std::string tname;
+
+        /** Used to store the end of a range query. */
+        std::string str_parameter;
+
+        /// Position in query of this term - leaf node only
+        Xapian::termpos term_pos;
+
+        /// Within query frequency of this term - leaf node only
+        Xapian::termcount wqf;
+
+        /// External posting source.
+        Xapian::PostingSource * external_source;
+
+        /** swap the contents of this with another Xapian::Query::Internal,
+         *  in a way which is guaranteed not to throw.  This is
+         *  used with the assignment operator to make it exception
+         *  safe.
+         *  It's important to adjust swap with any addition of
+         *  member variables!
+         */
+        void swap(Query::Internal &other);
+
+        /// Copy another Xapian::Query::Internal into self.
+        void initialise_from_copy(const Query::Internal & copyme);
+
+        void accumulate_terms(
+            std::vector<std::pair<std::string, Xapian::termpos> > &terms) const;
+
+        /** Simplify the query.
+         *  For example, an AND query with only one subquery would become the
+         *  subquery itself.
+         */
+        Internal * simplify_query();
+
+        /** Perform checks that query is valid. (eg, has correct number of
+         *  sub queries.)  Throw an exception if not.  This is initially called
+         *  on the query before any simplifications have been made, and after
+         *  simplications.
+         */
+        void validate_query() const;
+
+        /** Simplify any matchnothing subqueries, either eliminating them,
+         *  or setting this query to matchnothing, depending on the query
+         *  operator.  Returns true if simplification resulted in a
+         *  matchnothing query.
+         */
+        bool simplify_matchnothing();
+
+        /** Get a string describing the given query type.
+         */
+        static std::string get_op_name(Xapian::Query::Internal::op_t op);
+
+        /** Collapse the subqueries together if appropriate.
+         */
+        void collapse_subqs();
+
+        /** Flatten a query structure, by changing, for example,
+         *  "A NEAR (B AND C)" to "(A NEAR B) AND (A NEAR C)"
+         */
+        void flatten_subqs();
+
+        /** Implementation of serialisation; called recursively.
+         */
+        std::string serialise(Xapian::termpos & curpos) const;
+
+    public:
+        /** Copy constructor. */
+        Internal(const Query::Internal & copyme);
+
+        /** Assignment. */
+        void operator=(const Query::Internal & copyme);
+
+        /** A query consisting of a single term. */
+        explicit Internal(const std::string & tname_, Xapian::termcount wqf_ = 1,
+                          Xapian::termpos term_pos_ = 0);
+
+        /** Create internals given only the operator and a parameter. */
+        Internal(op_t op_, Xapian::termcount parameter);
+
+        /** Construct a range query on a document value. */
+        Internal(op_t op_, Xapian::valueno valno,
+                 const std::string &begin, const std::string &end);
+
+        /** Construct a value greater-than-or-equal query on a document value.
+         */
+        Internal(op_t op_, Xapian::valueno valno, const std::string &value);
+
+        /// Construct an external source query.
+        explicit Internal(Xapian::PostingSource * external_source_);
+
+        /** Destructor. */
+        ~Internal();
+
+        static Xapian::Query::Internal * unserialise(const std::string &s);
+
+        /** Add a subquery.
+         */
+        void add_subquery(const Query::Internal * subq);
+
+        void set_dbl_parameter(double dbl_parameter_);
+
+        double get_dbl_parameter() const;
+
+        /** Finish off the construction.
+         */
+        Query::Internal * end_construction();
+
+        /** Return a string in an easily parsed form
+         *  which contains all the information in a query.
+         */
+        std::string serialise() const {
+            Xapian::termpos curpos = 1;
+            return serialise(curpos);
+        }
+
+        /// Return a string describing this object.
+        std::string get_description() const;
+
+        /** Get the numeric parameter used in this query.
+         *
+         *  This is used by the QueryParser to get the value number for
+         *  VALUE_RANGE queries.  It should be replaced by a public method on
+         *  the Query class at some point, but the API which should be used for
+         *  that is unclear, so this is a temporary workaround.
+         */
+        Xapian::termcount get_parameter() const { return parameter; }
+
+        /** Get the length of the query, used by some ranking formulae.
+         *  This value is calculated automatically - if you want to override
+         *  it you can pass a different value to Enquire::set_query().
+         */
+        Xapian::termcount get_length() const;
+
+        /** Return an iterator over all the terms in the query,
+         *  in order of termpos.  If multiple terms have the same term
+         *  position, their order is unspecified.  Duplicates (same term and
+         *  termpos) will be removed.
+         */
+        TermIterator get_terms() const;
+};
+
+struct XAPIAN_VISIBILITY_DEFAULT Query::InternalPtr {
+    Xapian::Internal::RefCntPtr<Query::Internal> ptr;
+
+    InternalPtr() : ptr(0) {}
+    ~InternalPtr() { ptr = 0; }
+    Query::Internal * get() const { return ptr.get(); }
+};
+
+}
+
 #endif // XAPIAN_INCLUDED_OMQUERYINTERNAL_H

api/omquery.cc

@@ -45 +45 @@
 Query::add_subquery(const Query & subq)
 {
     DEBUGAPICALL(void, "Xapian::Query::add_subquery", subq);
-    Assert(internal.get());
-    internal->add_subquery(subq.internal.get());
+    Assert(internalptr);
+    Assert(internalptr->get());
+    internalptr->get()->add_subquery(subq.internalptr->get());
 }
 
 /// Add a subquery by pointer
@@ -57 +58 @@
     if (subq == 0) {
         throw InvalidArgumentError("Pointer to subquery may not be null");
     }
-    Assert(internal.get());
-    internal->add_subquery(subq->internal.get());
+    Assert(internalptr);
+    Assert(internalptr->get());
+    internalptr->get()->add_subquery(subq->internalptr->get());
 }
 
 /// Add a subquery which is a single term
@@ -66 +68 @@
 Query::add_subquery(const string & tname)
 {
     DEBUGAPICALL(void, "Xapian::Query::add_subquery", tname);
-    Assert(internal.get());
+    Assert(internalptr);
+    Assert(internalptr->get());
     Query::Internal subqint(tname);
-    internal->add_subquery(&subqint);
+    internalptr->ptr->add_subquery(&subqint);
 }
 
 /// Setup the internals for the query, with the appropriate operator.
@@ -76 +79 @@
 Query::start_construction(Query::op op_, termcount parameter)
 {
     DEBUGAPICALL(void, "Xapian::Query::start_construction", op_);
-    Assert(!internal.get());
-    internal = new Query::Internal(op_, parameter);
+    Assert(!internalptr);
+    internalptr = new Query::InternalPtr();
+    internalptr->ptr = new Query::Internal(op_, parameter);
 }
 
 /// Check that query has an appropriate number of arguments, etc,
@@ -85 +89 @@
 Query::end_construction()
 {
     DEBUGAPICALL(void, "Xapian::Query::end_construction", "");
-    Assert(internal.get());
-    internal = internal->end_construction();
+    Assert(internalptr);
+    Assert(internalptr->get());
+    internalptr->ptr = internalptr->get()->end_construction();
 }
 
 /// Abort construction of the query: delete internal.
@@ -94 +99 @@
 Query::abort_construction()
 {
     DEBUGAPICALL(void, "Xapian::Query::abort_construction", "");
-    Assert(internal.get());
-    internal = 0;
+    if (internalptr) {
+        internalptr->ptr = 0;
+    }
 }
 
 Query::Query(const string & tname_, termcount wqf_, termpos pos_)
-        : internal(new Query::Internal(tname_, wqf_, pos_))
+        : internalptr(new Query::InternalPtr())
 {
     DEBUGAPICALL(void, "Xapian::Query::Query",
                  tname_ << ", " << wqf_ << ", " << pos_);
+    internalptr->ptr = new Query::Internal(tname_, wqf_, pos_);
 }
 
 Query::Query(Query::op op_, const Query &left, const Query &right)
-        : internal(new Query::Internal(op_, 0u))
+        : internalptr(new Query::InternalPtr())
 {
     DEBUGAPICALL(void, "Xapian::Query::Query",
                  op_ << ", " << left << ", " << right);
+    internalptr->ptr = new Query::Internal(op_, 0u);
     try {
         add_subquery(left);
         add_subquery(right);
@@ -125 +133 @@
     DEBUGAPICALL(void, "Xapian::Query::Query",
                  op_ << ", " << q << ", " << parameter);
     if (op_ == OP_SCALE_WEIGHT) {
-        if (!q.internal.get() ||
-            q.internal->op == OP_VALUE_RANGE ||
-            q.internal->op == OP_VALUE_GE ||
-            q.internal->op == OP_VALUE_LE) {
+        if (!q.internalptr->get() ||
+            q.internalptr->get()->op == OP_VALUE_RANGE ||
+            q.internalptr->get()->op == OP_VALUE_GE ||
+            q.internalptr->get()->op == OP_VALUE_LE) {
             // Applying OP_SCALE_WEIGHT to Xapian::Query or OP_VALUE_*
             // has no effect as they're all pure-boolean.
-            internal = q.internal;
+            Assert(!internalptr);
+            internalptr = new Query::InternalPtr();
+            internalptr->ptr = q.internalptr->ptr;
             return;
         }
     }
     try {
         start_construction(op_, 0);
-        internal->set_dbl_parameter(parameter);
+        internalptr->get()->set_dbl_parameter(parameter);
         add_subquery(q);
         end_construction();
     } catch (...) {
@@ -148 +158 @@
 
 Query::Query(Query::op op_, Xapian::valueno valno,
              const string &begin, const string &end)
-    : internal(new Query::Internal(op_, valno, begin, end))
+        : internalptr(new Query::InternalPtr())
 {
     DEBUGAPICALL(void, "Xapian::Query::Query",
                  op_ << ", " << valno << ", " << begin << ", " << end);
+    internalptr->ptr = new Query::Internal(op_, valno, begin, end);
 }
 
 Query::Query(Query::op op_, Xapian::valueno valno, const std::string &value)
-    : internal(new Query::Internal(op_, valno, value))
+        : internalptr(new Query::InternalPtr())
 {
     DEBUGAPICALL(void, "Xapian::Query::Query",
                  op_ << ", " << valno << ", " << value);
+    internalptr->ptr = new Query::Internal(op_, valno, value);
 }
 
 Query::Query(PostingSource * external_source)
-        : internal(new Query::Internal(external_source))
+        : internalptr(new Query::InternalPtr())
 {
     DEBUGAPICALL(void, "Xapian::Query::Query", external_source);
+    internalptr->ptr = new Query::Internal(external_source);
 }
 
 // Copy constructor
 Query::Query(const Query & copyme)
-        : internal(copyme.internal)
+        : internalptr(new Query::InternalPtr())
 {
     DEBUGAPICALL(void, "Xapian::Query::Query", copyme);
+    internalptr->ptr = copyme.internalptr->ptr;
 }
 
 // Assignment
@@ -179 +193 @@
 Query::operator=(const Query & copyme)
 {
     DEBUGAPICALL(Xapian::Query &, "Xapian::Query::operator=", copyme);
-    internal = copyme.internal;
+    internalptr->ptr = copyme.internalptr->ptr;
     RETURN(*this);
 }
 
 // Default constructor
-Query::Query() : internal(0)
+Query::Query() : internalptr(new Query::InternalPtr())
 {
     DEBUGAPICALL(void, "Xapian::Query::Query", "");
 }
@@ -193 +207 @@
 Query::~Query()
 {
     DEBUGAPICALL(void, "Xapian::Query::~Query", "");
+    delete internalptr;
 }
 
 std::string
 Query::get_description() const
 {
     std::string res("Xapian::Query(");
-    if (internal.get()) res += internal->get_description();
+    if (internalptr->get()) res += internalptr->get()->get_description();
     res += ")";
     return res;
 }
@@ -207 +222 @@
 termcount Query::get_length() const
 {
     DEBUGAPICALL(Xapian::termcount, "Xapian::Query::get_length", "");
-    RETURN(internal.get() ? internal->get_length() : 0);
+    RETURN(internalptr->get() ? internalptr->get()->get_length() : 0);
 }
 
 TermIterator Query::get_terms_begin() const
 {
     DEBUGAPICALL(Xapian::TermIterator, "Xapian::Query::get_terms_begin", "");
-    if (!internal.get()) RETURN(TermIterator(NULL));
-    RETURN(internal->get_terms());
+    if (!internalptr->get()) RETURN(TermIterator(NULL));
+    RETURN(internalptr->get()->get_terms());
 }
 
 bool
 Query::empty() const
 {
     DEBUGAPICALL(void, "Xapian::Query::empty", "");
-    return internal.get() == 0;
+    return internalptr->get() == 0;
 }
 
 Query::Query(Query::op op_, const std::string & left, const std::string & right)
-    : internal(0)
+        : internalptr(0)
 {
     try {
         start_construction(op_, 0);

api/omenquire.cc

@@ -631 +631 @@
     }
 
     Stats stats;
-    ::MultiMatch match(db, query.internal.get(), qlen, rset, collapse_key,
+    ::MultiMatch match(db, query.internalptr->get(), qlen, rset, collapse_key,
                        percent_cutoff, weight_cutoff,
                        order, sort_key, sort_by, sort_value_forward, sorter,
                        errorhandler, stats, weight);