Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-38943: fix query-data-ids CLI handling of empty queries #833

Merged
merged 4 commits into from Apr 29, 2023

Conversation

TallJimbo
Copy link
Member

@TallJimbo TallJimbo commented Apr 28, 2023

Checklist

  • ran Jenkins
  • added a release note for user-visible changes to doc/changes

Copy link
Contributor

@andy-slac andy-slac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (59f50c0) 87.73% compared to head (1506d19) 87.74%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #833   +/-   ##
=======================================
  Coverage   87.73%   87.74%           
=======================================
  Files         268      268           
  Lines       35136    35154   +18     
  Branches     7397     7401    +4     
=======================================
+ Hits        30828    30846   +18     
  Misses       3155     3155           
  Partials     1153     1153           
Impacted Files Coverage Δ
python/lsst/daf/butler/script/queryDataIds.py 85.71% <100.00%> (+1.09%) ⬆️
tests/test_cliCmdQueryDataIds.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Previous code was inefficient most of the time (count() was used when
any() would have been adequate), and failed when a pure SQL count
was impossible since some filtering happened post-query in Python.

The new version ignores that post-query filtering (via exact=False),
but we catch that case later when we actually execute the real query.
@TallJimbo TallJimbo merged commit 87c2bcf into main Apr 29, 2023
13 checks passed
@TallJimbo TallJimbo deleted the tickets/DM-38943 branch April 29, 2023 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants