diff --git a/CHANGES.rst b/CHANGES.rst index 036ee35c..6cc43a8e 100644 --- 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 index 2282a282..914fde9d 100644 --- 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 index 6051447d..3ea11f45 100644 --- 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 index 1a7c7f60..c378138c 100644 --- 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%'))")