Merge pull request astropy#572 from msdemlei/make-extra-table-constra…

…ints-subqueried Now using DISTINCT in ivoid-selecting registry subqueries.
chmmao · Jul 5, 2024 · 246a209 · 246a209
2 parents 0a7d2fb + c290c03
commit 246a209
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 27 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -38,7 +38,7 @@ Enhancements and Fixes
   objects are made more consistent [#551]
 
 - 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]
 
 - Where datalink records are made from table rows, the table row is
   now accessible as datalinks.original_row. []

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,38 @@ 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 +472,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%'))")