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

fix query to get most recent actor catalog fetch event for some sources #21726

Merged
merged 5 commits into from
Jan 23, 2023

Conversation

mfsiega-airbyte
Copy link
Contributor

What

Fix the query to get the most recent actor catalog fetch event for some sources.

This query is both inefficient -- fails to apply the source id filter -- and possibly incorrect -- if two rows exist with the same actor_id, actor_catalog_id, created_at values, it will crash. This is unlikely since created_at is a timestamp, but the frontend actually ends up issuing two simultaneous discovery workflows, so it ends up happening more often than you'd expect. See #21208.

How

  • Use row_number() instead of rank(), which handles duplicates.
  • Applies sourceId filter.

@mfsiega-airbyte mfsiega-airbyte requested a review from a team as a code owner January 23, 2023 17:25
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets January 23, 2023 17:27 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets January 23, 2023 17:27 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte enabled auto-merge (squash) January 23, 2023 17:37
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets January 23, 2023 17:41 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets January 23, 2023 17:41 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets January 23, 2023 18:01 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets January 23, 2023 18:01 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets January 23, 2023 18:19 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets January 23, 2023 18:19 — with GitHub Actions Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Jan 23, 2023

Airbyte Code Coverage

File Coverage [81.63%] 🍏
ConfigRepository.java 81.63% 🍏
Total Project Coverage 26.69% 🍏

@mfsiega-airbyte
Copy link
Contributor Author

@benmoriceau just FYI - I noticed that the query was choking on an empty list of source ids. Short-circuiting seemed simplest, do you mind taking another look?

@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets January 23, 2023 19:33 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets January 23, 2023 19:33 — with GitHub Actions Inactive
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.

2 participants