BIND 10 #1688: duplicate RR suprression in auth::Query

[BIND 10 Development]

#1688: duplicate RR suprression in auth::Query
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  UnAssigned
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120320
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  b10-auth                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 I have some preliminary design level comments:

 - Is it really a better choice to use a std::set?  While it'd be more
   efficient for the search itself, it needs to allocate new resource
   every time we insert an entry (and we need to release the resource
   on destruction).  On the other hand, if we perform duplicate check
   directly in the vectors, it'd be slower for search, but no new
   resource should be allocated (and the resources for the vectors are
   reused), and iterating over the vector would be quite fast because
   they are essentially an array.  In many cases the number of RRsets
   in the response should be quite small, so I at least don't think the
   tradeoff is so obvious.

 - If we really conclude the set is the right choice, we should remove
   (Abstract)RRset::isSameKind().  Realistically, the only reason we
   add this to the RRset class even if functionality wise it doesn't
   have to be a member function is because we wanted to optimize the
   duplicate RRset detection for in-memory data source.  If the only
   reason is gone, then we should keep the class simpler with the
   cleanup.

 - And, if we continue using the set and "lthan", I think we should
   reconsider the naming of the method.  "lthan" (or less than) sounds
   quite generic and sounds like a predicate based on the comparison of
   the entire RRset (the awkward name of isSameKind() was to avoid such
   confusion).

 A couple of minor things I just happened to notice (I've not yet
 reviewed all of the branch):
 - I made a few minor editorial fixes.
 - This comment in rrset.h seems to be incomplete:
 {{{#!c++
     /// check for equality.  It only needs to do a "less than" check on C.
     /// equality.  It only needs to do one check on C,
 }}}

-- 
Ticket URL: <http://bind10.isc.org/ticket/1688#comment:10>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development