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] Dataset query to get only the latest facet for each version #2859

Merged

Conversation

sophiely
Copy link
Contributor

@sophiely sophiely commented Jul 25, 2024

Problem

Closes: #2860

Solution

Since the same facet type is replicated a lot of times, we can rank the facet partition by dataset version and facet name ands so as we can take only the most recent facet for each dataset uuid and type.
The UI seems to display only one facet per type (facet name) and dataset version anyway so we don't need to query as much facet (which are just duplicates anyway).

Checklist

  • You've signed-off your work
  • [] Your changes are accompanied by tests (if relevant)
  • Your change contains a small diff and is self-contained
  • You've updated any relevant documentation (if relevant)
  • You've included a one-line summary of your change for the CHANGELOG.md (Depending on the change, this may not be necessary).
  • You've versioned your .sql database schema migration according to Flyway's naming convention (if relevant)
  • You've included a header in any source code files (if relevant)

@boring-cyborg boring-cyborg bot added the api API layer changes label Jul 25, 2024
Copy link

netlify bot commented Jul 25, 2024

Deploy Preview for peppy-sprite-186812 canceled.

Name Link
🔨 Latest commit 75160fe
🔍 Latest deploy log https://app.netlify.com/sites/peppy-sprite-186812/deploys/66a28c291668a40007d0014c

@sophiely sophiely force-pushed the feat/fix-find-all-dataset-query branch from 4470982 to 676800d Compare July 25, 2024 09:07
sophiely added 2 commits July 25, 2024 11:15
Signed-off-by: sophiely <ly.sophie200@gmail.com>
Signed-off-by: sophiely <ly.sophie200@gmail.com>
@sophiely sophiely force-pushed the feat/fix-find-all-dataset-query branch from 3e05cdd to 2e76688 Compare July 25, 2024 09:15
Copy link

codecov bot commented Jul 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.75%. Comparing base (879031a) to head (2e76688).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2859   +/-   ##
=========================================
  Coverage     84.75%   84.75%           
  Complexity     1456     1456           
=========================================
  Files           253      253           
  Lines          6566     6566           
  Branches        305      305           
=========================================
  Hits           5565     5565           
  Misses          850      850           
  Partials        151      151           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@phixMe phixMe left a comment

Choose a reason for hiding this comment

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

Amazing, thanks, do we have a similar problem elsewhere? Or is this the only instance of this problem that you have seen.

df.facet,
df."name",
df.created_at,
rank() OVER (PARTITION BY df.dataset_version_uuid, "name"
Copy link
Member

Choose a reason for hiding this comment

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

❤️

Copy link
Member

@wslulciuc wslulciuc left a comment

Choose a reason for hiding this comment

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

LGTM 💯 🚀 🔥

@wslulciuc wslulciuc enabled auto-merge (squash) July 25, 2024 17:32
@wslulciuc wslulciuc merged commit f73f115 into MarquezProject:main Jul 25, 2024
15 checks passed
@dkt-sophie-ly
Copy link

Amazing, thanks, do we have a similar problem elsewhere? Or is this the only instance of this problem that you have seen.

For now i don't think so but ofc I will let you know if I notice similar issues :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api API layer changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PERF] DatasetDAO findAll query fail if there are too much facet
4 participants