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-34247: simplify dataset subquery logic and fix edge-case bugs #670

Merged
merged 10 commits into from
Apr 11, 2022

Commits on Apr 9, 2022

  1. Refactor dataset subquery logic.

    This used to be one massive method; now it's two medium-large methods
    and two very simple ones, and it's much easier to follow.
    
    This shouldn't change the query behavior at all, but it does collapse
    some UNION ALL constructs by merging their IN clauses when they are
    otherwise the same - the original code had an unnecessary distinction
    between CALIBRATION and other collection types (as well as some
    necessary distinctions, which this preserves).
    TallJimbo committed Apr 9, 2022
    Configuration menu
    Copy the full SHA
    149c754 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    f70ba48 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    b1a3d43 View commit details
    Browse the repository at this point in the history
  4. Belated changelog entry for DM-34328.

    ...which I just merged without a changelog entry.
    TallJimbo committed Apr 9, 2022
    Configuration menu
    Copy the full SHA
    1e1b904 View commit details
    Browse the repository at this point in the history
  5. Add changelog entry.

    TallJimbo committed Apr 9, 2022
    Configuration menu
    Copy the full SHA
    4c3fdf3 View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    ec5e5a5 View commit details
    Browse the repository at this point in the history
  7. Join in a dummy dataset subquery even when the query is doomed.

    This prevents query construction from failing when someone queries for
    a skypix-dimension dataset type in collections where none exist.  When
    the datasets do exist, we rely on the dataset table to bring in the
    skypix dimension, since those don't have tables of their own, and we
    need to do the same when the query is doomed to avoid triggering an
    exception about skypix dimensions not being directly joinable.
    TallJimbo committed Apr 9, 2022
    Configuration menu
    Copy the full SHA
    e6d6bd5 View commit details
    Browse the repository at this point in the history

Commits on Apr 11, 2022

  1. Configuration menu
    Copy the full SHA
    8e403ca View commit details
    Browse the repository at this point in the history
  2. Move collection-type dispatch down to DatasetRecordStorage.select.

    This fixes an edge-case bug involving mixed CALIBRATION/non-CALIBRATION
    searches introduced in the previous refactoring commit.
    
    This involves a small internal API change for DatasetRecordStorage -
    it now returns a SQLAlchemy object directly instead of one of our
    SimpleQuery instances, since it sometimes now returns a UNION.
    TallJimbo committed Apr 11, 2022
    Configuration menu
    Copy the full SHA
    fde3c66 View commit details
    Browse the repository at this point in the history
  3. Minor typing fixes for Database.

    These were revealed by installing the sqlalchemy-stubs package locally.
    That's something we should consider doing more broadly (e.g. in CI), as
    it revealed a lot more than just these, but it seems tangled up with
    migrating more fully to SQLAlchemy 2.0 APIs (there is a different stubs
    package for SQLAlchemy 2), and since that's not even fully released yet
    doing much more seems premature.
    TallJimbo committed Apr 11, 2022
    Configuration menu
    Copy the full SHA
    d168fa6 View commit details
    Browse the repository at this point in the history