From bd7ca70ee269f115a81ec5147c55b8884edb8183 Mon Sep 17 00:00:00 2001 From: Andy Salnikov Date: Tue, 30 Jul 2024 10:56:02 -0700 Subject: [PATCH 1/4] Avoid building queries that return no results. When QueryBuilder.joinDatset returns False there is no point in trying to build and execuite the query. --- .../lsst/daf/butler/registry/sql_registry.py | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/python/lsst/daf/butler/registry/sql_registry.py b/python/lsst/daf/butler/registry/sql_registry.py index 08ff18519c..6e427e6fb2 100644 --- a/python/lsst/daf/butler/registry/sql_registry.py +++ b/python/lsst/daf/butler/registry/sql_registry.py @@ -2126,17 +2126,24 @@ def queryDatasets( # only if we need to findFirst. Note that if any of the # collections are actually wildcard expressions, and # findFirst=True, this will raise TypeError for us. - builder.joinDataset( + any_records = builder.joinDataset( resolved_dataset_type, collection_wildcard, isResult=True, findFirst=findFirst ) - query = builder.finish() - parent_results.append(queries.DatabaseParentDatasetQueryResults(query, resolved_dataset_type)) + if any_records: + query = builder.finish() + parent_results.append(queries.DatabaseParentDatasetQueryResults(query, resolved_dataset_type)) + else: + doomed_by.append( + f"No datasets of type {resolved_dataset_type.name} " + f"in collections {collection_wildcard!r}." + ) if not parent_results: - doomed_by.extend( - f"No registered dataset type matching {t!r} found, so no matching datasets can " - "exist in any collection." - for t in ensure_iterable(datasetType) - ) + if not doomed_by: + doomed_by.extend( + f"No registered dataset type matching {t!r} found, so no matching datasets can " + "exist in any collection." + for t in ensure_iterable(datasetType) + ) return queries.ChainedDatasetQueryResults([], doomed_by=doomed_by) elif len(parent_results) == 1: return parent_results[0] From 75a25d082ebd66ab57c848931b4490ec806073be Mon Sep 17 00:00:00 2001 From: Andy Salnikov Date: Tue, 30 Jul 2024 13:45:02 -0700 Subject: [PATCH 2/4] Update logic for checking overlap dimensions in relations (DM-45119) New logic only checks for required elements instead of full set of dimensions. This fixes special case of `healpix11` dataset types in dc2 repo. There is no unit test for this special case, but I verified that query-datasets and query-data-ids now work correctly. --- python/lsst/daf/butler/registry/dimensions/static.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/lsst/daf/butler/registry/dimensions/static.py b/python/lsst/daf/butler/registry/dimensions/static.py index 6e6178fa57..07d71bb750 100644 --- a/python/lsst/daf/butler/registry/dimensions/static.py +++ b/python/lsst/daf/butler/registry/dimensions/static.py @@ -385,9 +385,9 @@ def make_spatial_join_relation( ) -> tuple[Relation, bool]: # Docstring inherited. overlap_relationship = frozenset( - self.universe[element1].dimensions.names | self.universe[element2].dimensions.names + self.universe[element1].required.names | self.universe[element2].required.names ) - if overlap_relationship in existing_relationships: + if any(overlap_relationship.issubset(rel) for rel in existing_relationships): return context.preferred_engine.make_join_identity_relation(), False overlaps: Relation | None = None needs_refinement: bool = False From fccccbde56ce740c4399d29891191425a56606de Mon Sep 17 00:00:00 2001 From: Andy Salnikov Date: Tue, 30 Jul 2024 15:31:44 -0700 Subject: [PATCH 3/4] Add news fragment --- doc/changes/DM-45119.bugfix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 doc/changes/DM-45119.bugfix.md diff --git a/doc/changes/DM-45119.bugfix.md b/doc/changes/DM-45119.bugfix.md new file mode 100644 index 0000000000..87773f572e --- /dev/null +++ b/doc/changes/DM-45119.bugfix.md @@ -0,0 +1 @@ +Fix for handling of dataset types that use `healpix11` dimensions, previously they caused exception in many query operations. From 2111e21ed6a61ffecedd81c4763d49456c346a01 Mon Sep 17 00:00:00 2001 From: Andy Salnikov Date: Wed, 31 Jul 2024 10:23:41 -0700 Subject: [PATCH 4/4] Apply review suggestions --- .../lsst/daf/butler/registry/dimensions/static.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/python/lsst/daf/butler/registry/dimensions/static.py b/python/lsst/daf/butler/registry/dimensions/static.py index 07d71bb750..94ba4e0e96 100644 --- a/python/lsst/daf/butler/registry/dimensions/static.py +++ b/python/lsst/daf/butler/registry/dimensions/static.py @@ -384,11 +384,18 @@ def make_spatial_join_relation( existing_relationships: Set[frozenset[str]] = frozenset(), ) -> tuple[Relation, bool]: # Docstring inherited. - overlap_relationship = frozenset( - self.universe[element1].required.names | self.universe[element2].required.names - ) - if any(overlap_relationship.issubset(rel) for rel in existing_relationships): + group1 = self.universe[element1].minimal_group + group2 = self.universe[element2].minimal_group + overlap_relationships = { + frozenset(a | b) + for a, b in itertools.product( + [group1.names, group1.required], + [group2.names, group2.required], + ) + } + if not overlap_relationships.isdisjoint(existing_relationships): return context.preferred_engine.make_join_identity_relation(), False + overlaps: Relation | None = None needs_refinement: bool = False if element1 == self.universe.commonSkyPix.name: