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.

Sign up for GitHub

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

Jump to bottom

ud-gl-remove duplicates #52

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ud-gl-remove duplicates #52

wants to merge 1 commit into from

Conversation

aurreco-uga
Copy link
Member

No description provided.

ryanrdoherty
ryanrdoherty requested changes Dec 19, 2024
View reviewed changes
Copy link
Member

@ryanrdoherty ryanrdoherty left a comment

Choose a reason for hiding this comment

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

I think this will work but requesting changes because I think we should discuss more.

Model/lib/wdk/model/questions/queries/geneQueries.xml
@@ -115,10 +115,12 @@
<column name="project_id"/>
<sql>
<![CDATA[
select source_id,gene_source_id,project_id,matched_result from (
Copy link
Member

Choose a reason for hiding this comment

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

Why use group by here instead of distinct? I would typically only use group by if I am extracting info from the group like min/max/count. Do you have reason to believe the group by would be faster?

Copy link
Member

Choose a reason for hiding this comment

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

Also I wonder if we should make sure we're not adding duplicates in the genelist installer. We could still put distinct here to make sure, but in Bob's case we have to process 2x the results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanrdoherty ryanrdoherty ryanrdoherty requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aurreco-uga @ryanrdoherty