Now using DISTINCT in ivoid-selecting registry subqueries.

This is not only more logical (let's filter out duplicates as early as possible)but experimentally also is enough to keep the planner going off to follies of the type encountered in bug #571.
astropy · Jul 2, 2024 · 7db0cfb · 7db0cfb
1 parent e9c370f
commit 7db0cfb
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 27 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -27,7 +27,7 @@ Enhancements and Fixes
   whose structure corresponds to the classes of the mapped models. [#497]
 
 - RegTAP constraints involving tables other than rr.resource are now
-  done via subqueries for less duplication of interfaces. [#562]
+  done via subqueries for less duplication of interfaces. [#562, #572]
 
 
 Deprecations and Removals

diff --git a/pyvo/registry/rtcons.py b/pyvo/registry/rtcons.py
@@ -233,7 +233,7 @@ def get_search_condition(self, service):
             raise NotImplementedError("{} is an abstract Constraint"
                                       .format(self.__class__.__name__))
 
-        return ("ivoid IN (SELECT ivoid FROM {subquery_table}"
+        return ("ivoid IN (SELECT DISTINCT ivoid FROM {subquery_table}"
             " WHERE {condition})".format(
                 subquery_table=self._subquery_table,
                 condition=self._condition.format(**self._get_sql_literals())))
@@ -274,11 +274,11 @@ def get_search_condition(self, service):
 
     def _get_union_condition(self, service):
         base_queries = [
-            "SELECT ivoid FROM rr.resource WHERE"
+            "SELECT DISTINCT ivoid FROM rr.resource WHERE"
             " 1=ivo_hasword(res_description, {{{parname}}})",
-            "SELECT ivoid FROM rr.resource WHERE"
+            "SELECT DISTINCT ivoid FROM rr.resource WHERE"
             " 1=ivo_hasword(res_title, {{{parname}}})",
-            "SELECT ivoid FROM rr.res_subject WHERE"
+            "SELECT DISTINCT ivoid FROM rr.res_subject WHERE"
             " rr.res_subject.res_subject ILIKE {{{parpatname}}}"]
         self._fillers, subqueries = {}, []
 
@@ -288,7 +288,7 @@ def _get_union_condition(self, service):
             self._fillers[parname] = word
             self._fillers[parpatname] = '%' + word + '%'
             args = locals()
-            subqueries.append(" UNION ".join(
+            subqueries.append(" UNION ALL ".join(
                 q.format(**args) for q in base_queries))
 
         self._condition = " AND ".join(

diff --git a/pyvo/registry/tests/test_regtap.py b/pyvo/registry/tests/test_regtap.py
@@ -349,14 +349,14 @@ def get_regtap_results(**kwargs):
 def test_spatial():
     assert (rtcons.keywords_to_constraints({
             "spatial": (23, -40)})[0].get_search_condition(FAKE_GAVO)
-            == "ivoid IN (SELECT ivoid FROM rr.stc_spatial"
+            == "ivoid IN (SELECT DISTINCT ivoid FROM rr.stc_spatial"
             " WHERE 1 = CONTAINS(MOC(6, POINT(23, -40)), coverage))")
 
 
 def test_spectral():
     assert (rtcons.keywords_to_constraints({
             "spectral": (1e-17, 2e-17)})[0].get_search_condition(FAKE_GAVO)
-            == "ivoid IN (SELECT ivoid FROM rr.stc_spectral WHERE"
+            == "ivoid IN (SELECT DISTINCT ivoid FROM rr.stc_spectral WHERE"
             " 1 = ivo_interval_overlaps(spectral_start, spectral_end, 1e-17, 2e-17))")
 
 

diff --git a/pyvo/registry/tests/test_rtcons.py b/pyvo/registry/tests/test_rtcons.py
@@ -31,7 +31,7 @@ def _make_subquery(table, condition):
     """returns how condition would show up in something produced by
     SubqueriedConstraint.
     """
-    return f"ivoid IN (SELECT ivoid FROM {table} WHERE {condition})"
+    return f"ivoid IN (SELECT DISTINCT ivoid FROM {table} WHERE {condition})"
 
 
 class TestAbstractConstraint:
@@ -83,26 +83,26 @@ def test_odd_type_rejected(self):
 class TestFreetextConstraint:
     def test_basic(self):
         assert rtcons.Freetext("star").get_search_condition(FAKE_GAVO) == (
-            "ivoid IN (SELECT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_description, 'star') "
-            "UNION SELECT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_title, 'star') "
-            "UNION SELECT ivoid FROM rr.res_subject WHERE rr.res_subject.res_subject ILIKE '%star%')")
+            "ivoid IN (SELECT DISTINCT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_description, 'star') "
+            "UNION ALL SELECT DISTINCT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_title, 'star') "
+            "UNION ALL SELECT DISTINCT ivoid FROM rr.res_subject WHERE rr.res_subject.res_subject ILIKE '%star%')")
 
     def test_interesting_literal(self):
         assert rtcons.Freetext("α Cen's planets").get_search_condition(FAKE_GAVO) == (
-            "ivoid IN (SELECT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_description, 'α Cen''s planets')"
-            " UNION SELECT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_title, 'α Cen''s planets')"
-            " UNION SELECT ivoid FROM rr.res_subject WHERE rr.res_subject.res_subject"
+            "ivoid IN (SELECT DISTINCT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_description, 'α Cen''s planets')"
+            " UNION ALL SELECT DISTINCT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_title, 'α Cen''s planets')"
+            " UNION ALL SELECT DISTINCT ivoid FROM rr.res_subject WHERE rr.res_subject.res_subject"
             " ILIKE '%α Cen''s planets%')")
 
     def test_multipleLiterals(self):
         assert rtcons.Freetext("term1", "term2").get_search_condition(FAKE_GAVO) == (
-            "ivoid IN (SELECT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_description, 'term1')"
-            " UNION SELECT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_title, 'term1')"
-            " UNION SELECT ivoid FROM rr.res_subject WHERE rr.res_subject.res_subject ILIKE '%term1%')"
+            "ivoid IN (SELECT DISTINCT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_description, 'term1')"
+            " UNION ALL SELECT DISTINCT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_title, 'term1')"
+            " UNION ALL SELECT DISTINCT ivoid FROM rr.res_subject WHERE rr.res_subject.res_subject ILIKE '%term1%')"
             " AND "
-            "ivoid IN (SELECT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_description, 'term2')"
-            " UNION SELECT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_title, 'term2')"
-            " UNION SELECT ivoid FROM rr.res_subject WHERE rr.res_subject.res_subject ILIKE '%term2%')")
+            "ivoid IN (SELECT DISTINCT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_description, 'term2')"
+            " UNION ALL SELECT DISTINCT ivoid FROM rr.resource WHERE 1=ivo_hasword(res_title, 'term2')"
+            " UNION ALL SELECT DISTINCT ivoid FROM rr.res_subject WHERE rr.res_subject.res_subject ILIKE '%term2%')")
 
     def test_adaption_to_service(self):
         assert rtcons.Freetext("term1", "term2").get_search_condition(FAKE_PLAIN) == (
@@ -460,13 +460,13 @@ def test_with_legacy_keyword(self):
         assert self.where_clause_for(
             "plain", "string"
         ) == (
-            '(ivoid IN (SELECT ivoid FROM rr.resource WHERE '
-            "1=ivo_hasword(res_description, 'plain') UNION SELECT ivoid FROM rr.resource "
-            "WHERE 1=ivo_hasword(res_title, 'plain') UNION SELECT ivoid FROM "
+            '(ivoid IN (SELECT DISTINCT ivoid FROM rr.resource WHERE '
+            "1=ivo_hasword(res_description, 'plain') UNION ALL SELECT DISTINCT ivoid FROM rr.resource "
+            "WHERE 1=ivo_hasword(res_title, 'plain') UNION ALL SELECT DISTINCT ivoid FROM "
             "rr.res_subject WHERE rr.res_subject.res_subject ILIKE '%plain%'))\n"
-            '  AND (ivoid IN (SELECT ivoid FROM rr.resource WHERE '
-            "1=ivo_hasword(res_description, 'string') UNION SELECT ivoid FROM rr.resource "
-            "WHERE 1=ivo_hasword(res_title, 'string') UNION SELECT ivoid FROM "
+            '  AND (ivoid IN (SELECT DISTINCT ivoid FROM rr.resource WHERE '
+            "1=ivo_hasword(res_description, 'string') UNION ALL SELECT DISTINCT ivoid FROM rr.resource "
+            "WHERE 1=ivo_hasword(res_title, 'string') UNION ALL SELECT DISTINCT ivoid FROM "
             "rr.res_subject WHERE rr.res_subject.res_subject ILIKE '%string%'))")