Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

packages: fix package ref listing query with limit#47442

Merged
Strum355 merged 2 commits intomainfrom
nsc/packages-list-fix
Feb 8, 2023
Merged

packages: fix package ref listing query with limit#47442
Strum355 merged 2 commits intomainfrom
nsc/packages-list-fix

Conversation

@Strum355
Copy link
Copy Markdown
Contributor

@Strum355 Strum355 commented Feb 7, 2023

The list package repo refs query accidentally worked under the assumption of what the data should ideally look like (unique (scheme,name) tuples). This is not true in stage 1 of the migration, where there is still one (scheme,name) tuple for every version of that tuple.
Continuing: in step 1 of the migration, we copy for every row in lsif_dependency_repos into package_repo_versions, using the smallest ID for every (scheme,name) as the "canonical" package repo entry ID (this is so when we collapse n into 1, every entry in package_repo_versions still points to an existing package repo reference).
Thus when joining the two tables, every entry besides the first for every (scheme,name) would not have an accompanying entry in the latter table, thus not being in the final result set. This happens after the limit factor, so we would end up with <= limit.

There was also a bug with the count section that was fixed here, because the reduce happens in-application instead of in SQL :^)

Test plan

Added extra part in existing unit test and tested the extra part with before and after

@Strum355 Strum355 added team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) team/language-platform rfc-698 Packages https://docs.google.com/document/d/1aGTzOKXJkf29FXUaDb3uevmIXmX09hYS11UKSp6j2SA/edit package-repos labels Feb 7, 2023
@Strum355 Strum355 requested a review from efritz February 7, 2023 17:55
@Strum355 Strum355 self-assigned this Feb 7, 2023
@cla-bot cla-bot bot added the cla-signed label Feb 7, 2023
@efritz
Copy link
Copy Markdown
Contributor

efritz commented Feb 8, 2023

I tend to use 'rank' for row number (not a bit, just a fun fact)

@Strum355 Strum355 merged commit 0b2dca3 into main Feb 8, 2023
@Strum355 Strum355 deleted the nsc/packages-list-fix branch February 8, 2023 13:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed package-repos rfc-698 Packages https://docs.google.com/document/d/1aGTzOKXJkf29FXUaDb3uevmIXmX09hYS11UKSp6j2SA/edit team/graph Graph Team (previously Code Intel/Language Tools/Language Platform)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants