Changing the data model constraint to subquery style, too.

CHANGES.rst

@@ -26,6 +26,9 @@ Enhancements and Fixes
   any model serialized in VO-DML. This package dynamically generates python objects
   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]
+
 
 Deprecations and Removals
 -------------------------

pyvo/registry/regtap.py

@@ -784,7 +784,6 @@ def get_interface(self, *,
                           if ((not std_only) or intf.is_standard)
                           and not intf.is_vosi
                           and ((not service_type) or intf.supports(service_type))]
-
         if not candidates:
             raise ValueError("No matching interface.")
 

pyvo/registry/rtcons.py

@@ -162,6 +162,10 @@ class Constraint:
     sequences of constraints.  Constraints that want to the all
     arguments in the constructor can set takes_sequence to True.
     """
+    # TODO: _extra_tables is only used in the legacy leg of
+    # the fulltext constraint any more, and there it's wrong, too.
+    # Let's do away with extra_tables and tell people to use
+    # SubqueriedConstraint whenever they think they need it.
     _extra_tables = []
     _condition = None
     _fillers = None
@@ -231,8 +235,8 @@ def get_search_condition(self, service):
 
         return ("ivoid IN (SELECT ivoid FROM {subquery_table}"
             " WHERE {condition})".format(
-            subquery_table=self._subquery_table,
-            condition=self._condition.format(**self._get_sql_literals())))
+                subquery_table=self._subquery_table,
+                condition=self._condition.format(**self._get_sql_literals())))
 
 
 class Freetext(Constraint):
@@ -481,7 +485,7 @@ def __init__(self, *bands):
             for band in self.bands)
 
 
-class Datamodel(Constraint):
+class Datamodel(SubqueriedConstraint):
     """
     A constraint on the adherence to a data model.
 
@@ -516,30 +520,30 @@ def __init__(self, dmname):
         if dmname not in self._known_dms:
             raise dalq.DALQueryError("Unknown data model id {}.  Known are: {}."
                                      .format(dmname, ", ".join(sorted(self._known_dms))))
-        self._condition = getattr(self, f"_make_{dmname}_constraint")()
+        self._subquery_table, self._condition = getattr(
+            self, f"_make_{dmname}_constraint")()
 
     def _make_obscore_constraint(self):
         # There was a bit of chaos with the DM ids for Obscore.
         # Be lenient here
-        self._extra_tables = ["rr.res_detail"]
         obscore_pat = 'ivo://ivoa.net/std/obscore%'
-        return ("detail_xpath = '/capability/dataModel/@ivo-id'"
-                f" AND 1 = ivo_nocasematch(detail_value, '{obscore_pat}')")
+        return "rr.res_detail", (
+            "detail_xpath = '/capability/dataModel/@ivo-id'"
+            f" AND 1 = ivo_nocasematch(detail_value, '{obscore_pat}')")
 
     def _make_epntap_constraint(self):
-        self._extra_tables = ["rr.res_table"]
         # we include legacy, pre-IVOA utypes for matches; lowercase
         # any new identifiers (utypes case-fold).
-        return " OR ".join(
+        return "rr.res_table", " OR ".join(
             f"table_utype LIKE '{pat}'" for pat in
             ['ivo://vopdc.obspm/std/epncore#schema-2.%',
              'ivo://ivoa.net/std/epntap#table-2.%'])
 
     def _make_regtap_constraint(self):
-        self._extra_tables = ["rr.res_detail"]
         regtap_pat = 'ivo://ivoa.net/std/RegTAP#1.%'
-        return ("detail_xpath = '/capability/dataModel/@ivo-id'"
-                f" AND 1 = ivo_nocasematch(detail_value, '{regtap_pat}')")
+        return "rr.res_detail", (
+            "detail_xpath = '/capability/dataModel/@ivo-id'"
+            f" AND 1 = ivo_nocasematch(detail_value, '{regtap_pat}')")
 
 
 class Ivoid(Constraint):

pyvo/registry/tests/test_regtap.py

@@ -146,7 +146,7 @@ def datamodeltest_callback(request, content):
         query = data['QUERY']
 
         assert (
-            "(detail_xpath = '/capability/dataModel/@ivo-id'" in query)
+            "detail_xpath = '/capability/dataModel/@ivo-id'" in query)
 
         assert (
             "ivo_nocasematch(detail_value, 'ivo://ivoa.net/std/obscore%'))"
@@ -349,13 +349,15 @@ def get_regtap_results(**kwargs):
 def test_spatial():
     assert (rtcons.keywords_to_constraints({
             "spatial": (23, -40)})[0].get_search_condition(FAKE_GAVO)
-            == "1 = CONTAINS(MOC(6, POINT(23, -40)), coverage)")
+            == "ivoid IN (SELECT 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)
-            == "1 = ivo_interval_overlaps(spectral_start, spectral_end, 1e-17, 2e-17)")
+            == "ivoid IN (SELECT ivoid FROM rr.stc_spectral WHERE"
+            " 1 = ivo_interval_overlaps(spectral_start, spectral_end, 1e-17, 2e-17))")
 
 
 def test_to_table(multi_interface_fixture, capabilities):

pyvo/registry/tests/test_rtcons.py

@@ -196,25 +196,31 @@ def test_junk_rejected(self):
     def test_obscore(self):
         cons = rtcons.Datamodel("ObsCore")
         assert (cons.get_search_condition(FAKE_GAVO)
-                == "detail_xpath = '/capability/dataModel/@ivo-id'"
-                " AND 1 = ivo_nocasematch(detail_value,"
-                " 'ivo://ivoa.net/std/obscore%')")
-        assert (cons._extra_tables == ["rr.res_detail"])
+                == _make_subquery(
+                    "rr.res_detail",
+                    "detail_xpath = '/capability/dataModel/@ivo-id'"
+                    " AND 1 = ivo_nocasematch(detail_value,"
+                    " 'ivo://ivoa.net/std/obscore%')"))
+        assert cons._extra_tables == []
 
     def test_epntap(self):
         cons = rtcons.Datamodel("epntap")
         assert (cons.get_search_condition(FAKE_GAVO)
-                == "table_utype LIKE 'ivo://vopdc.obspm/std/epncore#schema-2.%'"
-                " OR table_utype LIKE 'ivo://ivoa.net/std/epntap#table-2.%'")
-        assert (cons._extra_tables == ["rr.res_table"])
+                == _make_subquery(
+                    "rr.res_table",
+                    "table_utype LIKE 'ivo://vopdc.obspm/std/epncore#schema-2.%'"
+                    " OR table_utype LIKE 'ivo://ivoa.net/std/epntap#table-2.%'"))
+        assert cons._extra_tables == []
 
     def test_regtap(self):
         cons = rtcons.Datamodel("regtap")
         assert (cons.get_search_condition(FAKE_GAVO)
-                == "detail_xpath = '/capability/dataModel/@ivo-id'"
-                " AND 1 = ivo_nocasematch(detail_value,"
-                " 'ivo://ivoa.net/std/RegTAP#1.%')")
-        assert (cons._extra_tables == ["rr.res_detail"])
+                == _make_subquery(
+                    "rr.res_detail",
+                    "detail_xpath = '/capability/dataModel/@ivo-id'"
+                    " AND 1 = ivo_nocasematch(detail_value,"
+                    " 'ivo://ivoa.net/std/RegTAP#1.%')"))
+        assert cons._extra_tables == []
 
 
 class TestIvoidConstraint:
@@ -236,7 +242,9 @@ class TestUCDConstraint:
     def test_basic(self):
         cons = rtcons.UCD("phot.mag;em.opt.%", "phot.mag;em.ir.%")
         assert (cons.get_search_condition(FAKE_GAVO)
-                == _make_subquery("rr.table_column", "ucd LIKE 'phot.mag;em.opt.%' OR ucd LIKE 'phot.mag;em.ir.%'"))
+                == _make_subquery(
+                    "rr.table_column",
+                    "ucd LIKE 'phot.mag;em.opt.%' OR ucd LIKE 'phot.mag;em.ir.%'"))
 
 
 class TestSpatialConstraint:
@@ -354,7 +362,8 @@ def test_frequency_interval(self):
         assert (cons.get_search_condition(FAKE_GAVO)
                 == _make_subquery(
                     "rr.stc_spectral",
-                    "1 = ivo_interval_overlaps(spectral_start, spectral_end, 5.830941732e-26, 6.758591553e-26)"))
+                    "1 = ivo_interval_overlaps("
+                    "spectral_start, spectral_end, 5.830941732e-26, 6.758591553e-26)"))
 
     def test_no_spectral(self):
         cons = registry.Spectral((88 * u.MHz, 102 * u.MHz))
@@ -410,8 +419,8 @@ def where_clause_for(*args, **kwargs):
     def test_from_constraints(self):
         assert self.where_clause_for(
             rtcons.Waveband("EUV"),
-            rtcons.Author("%Hubble%")
-        ) == ("(1 = ivo_hashlist_has(rr.resource.waveband, 'euv'))\n"
+            rtcons.Author("%Hubble%"))\
+            == ("(1 = ivo_hashlist_has(rr.resource.waveband, 'euv'))\n"
               "  AND ({})".format(
                 _make_subquery(
                     "rr.res_role",
@@ -421,16 +430,16 @@ def test_from_constraints(self):
     def test_from_keywords(self):
         assert self.where_clause_for(
             waveband="EUV",
-            author="%Hubble%"
-        ) == ("(1 = ivo_hashlist_has(rr.resource.waveband, 'euv'))\n  AND ({})".format(
-              _make_subquery("rr.res_role", "role_name LIKE '%Hubble%' AND base_role='creator'")))
+            author="%Hubble%")\
+            == ("(1 = ivo_hashlist_has(rr.resource.waveband, 'euv'))\n  AND ({})".format(
+                _make_subquery("rr.res_role", "role_name LIKE '%Hubble%' AND base_role='creator'")))
 
     @pytest.mark.usefixtures('messenger_vocabulary')
     def test_mixed(self):
         assert self.where_clause_for(
             rtcons.Waveband("EUV"),
-            author="%Hubble%"
-        ) == ("(1 = ivo_hashlist_has(rr.resource.waveband, 'euv'))\n"
+            author="%Hubble%")\
+            == ("(1 = ivo_hashlist_has(rr.resource.waveband, 'euv'))\n"
               "  AND ({})".format(
                 _make_subquery("rr.res_role", "role_name LIKE '%Hubble%' AND base_role='creator'")))