[1771-2] remove seen_ns&other check in getRRsets.

on a closer look, it just doesn't seem to be necessary if we are okay
with just "hiding" records under a delegation NS for normal (ie. non
GLUE_OK) lookups, instead of throwing an exception.

Essentially only one existing test case (which expects an exception
in that setup) should be updated (and the "brokenns2" case just seems
to be redundant and so was removed), and the disabled tests
in zone_finder_context_unittest now pass.

src/lib/datasrc/database.cc

@@ -206,7 +206,6 @@ DatabaseClient::Finder::getRRsets(const string& name, const WantedTypes& types,
     bool seen_cname(false);
     bool seen_ds(false);
     bool seen_other(false);
-    bool seen_ns(false);
 
     while (context->getNext(columns)) {
         // The domain is not empty
@@ -249,8 +248,6 @@ DatabaseClient::Finder::getRRsets(const string& name, const WantedTypes& types,
 
             if (cur_type == RRType::CNAME()) {
                 seen_cname = true;
-            } else if (cur_type == RRType::NS()) {
-                seen_ns = true;
             } else if (cur_type == RRType::DS()) {
                 seen_ds = true;
             } else if (cur_type != RRType::RRSIG() &&
@@ -278,11 +275,11 @@ DatabaseClient::Finder::getRRsets(const string& name, const WantedTypes& types,
                       RDATA_COLUMN]);
         }
     }
-    if (seen_cname && (seen_other || seen_ns || seen_ds)) {
+    if (seen_cname && (seen_other || seen_ds)) {
         isc_throw(DataSourceError, "CNAME shares domain " << name <<
                   " with something else");
     }
-    if (check_ns && seen_ns && seen_other) {
+    if (check_ns && /*seen_ns*/ false && seen_other) {
         isc_throw(DataSourceError, "NS shares domain " << name <<
                   " with something else");
     }

src/lib/datasrc/tests/database_unittest.cc

@@ -168,13 +168,16 @@ const char* const TEST_RECORDS[][5] = {
     {"child.insecdelegation.example.org.", "DS", "3600", "", "DS 5 3 3600 "
      "20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE"},
 
-    // Broken NS
+    // Delegation NS and other ordinary type of RR coexist at the same
+    // name.  This is deviant (except for some special cases like the other
+    // RR could be used for addressing the NS name), but as long as the
+    // other records are hidden behind the delegation for normal queries
+    // it's not necessarily harmful. (so "broken" may be too strong, but we
+    // keep the name since it could be in a chain of sorted names for DNSSEC
+    // processing and renaming them may have other bad effects for tests).
     {"brokenns1.example.org.", "A", "3600", "", "192.0.2.1"},
     {"brokenns1.example.org.", "NS", "3600", "", "ns.example.com."},
 
-    {"brokenns2.example.org.", "NS", "3600", "", "ns.example.com."},
-    {"brokenns2.example.org.", "A", "3600", "", "192.0.2.1"},
-
     // Now double DNAME, to test failure mode
     {"baddname.example.org.", "DNAME", "3600", "", "dname1.example.com."},
     {"baddname.example.org.", "DNAME", "3600", "", "dname2.example.com."},
@@ -2202,15 +2205,23 @@ TYPED_TEST(DatabaseClientTest, findDelegation) {
                               ZoneFinder::FIND_DEFAULT),
                  DataSourceError);
 
-    // Broken NS - it lives together with something else
-    EXPECT_THROW(finder->find(isc::dns::Name("brokenns1.example.org."),
-                              this->qtype_,
-                              ZoneFinder::FIND_DEFAULT),
-                 DataSourceError);
-    EXPECT_THROW(finder->find(isc::dns::Name("brokenns2.example.org."),
-                              this->qtype_,
-                              ZoneFinder::FIND_DEFAULT),
-                 DataSourceError);
+    // NS and other type coexist: deviant and not necessarily harmful.
+    // It should normally just result in DELEGATION; if GLUE_OK is specified,
+    // the other RR should be visible.
+    this->expected_rdatas_.clear();
+    this->expected_rdatas_.push_back("ns.example.com");
+    doFindTest(*finder, Name("brokenns1.example.org"), this->qtype_,
+               RRType::NS(), this->rrttl_, ZoneFinder::DELEGATION,
+               this->expected_rdatas_, this->empty_rdatas_,
+               ZoneFinder::RESULT_DEFAULT);
+
+    this->expected_rdatas_.clear();
+    this->expected_rdatas_.push_back("192.0.2.1");
+    doFindTest(*finder, Name("brokenns1.example.org"), this->qtype_,
+               this->qtype_, this->rrttl_, ZoneFinder::SUCCESS,
+               this->expected_rdatas_, this->empty_rdatas_,
+               ZoneFinder::RESULT_DEFAULT, Name("brokenns1.example.org"),
+               ZoneFinder::FIND_GLUE_OK);
 }
 
 TYPED_TEST(DatabaseClientTest, findDS) {

src/lib/datasrc/tests/zone_finder_context_unittest.cc

@@ -208,13 +208,6 @@ TEST_P(ZoneFinderContextTest, getAdditionalDelegation) {
 TEST_P(ZoneFinderContextTest, getAdditionalDelegationAtZoneCut) {
     // Similar to the previous case, but one of the NS addresses is at the
     // zone cut.
-
-    // XXX: the current database-based data source incorrectly rejects this
-    // setup (see #1771)
-    if (GetParam() == createSQLite3Client) {
-        return;
-    }
-
     ZoneFinderContextPtr ctx = finder_->find(Name("www.b.example.org"),
                                              RRType::SOA());
     EXPECT_EQ(ZoneFinder::DELEGATION, ctx->code);
@@ -316,12 +309,6 @@ TEST_P(ZoneFinderContextTest, getAdditionalMX) {
 }
 
 TEST_P(ZoneFinderContextTest, getAdditionalMXAtZoneCut) {
-    // XXX: the current database-based data source incorrectly rejects this
-    // setup (see #1771)
-    if (GetParam() == createSQLite3Client) {
-        return;
-    }
-
     ZoneFinderContextPtr ctx = finder_->find(Name("mxatcut.example.org."),
                                              RRType::MX());
     EXPECT_EQ(ZoneFinder::SUCCESS, ctx->code);