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

feat: use select_related to fetch types in CSVLoader #4518

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

Ali-D-Akbar
Copy link
Contributor

@Ali-D-Akbar Ali-D-Akbar commented Dec 18, 2024

PROD-4237
Adds select_related to Eager load CourseType and CourseRunType for Courses and CourseRuns respectively in CSVLoader

@AfaqShuaib09
Copy link
Contributor

nit: commit type can be perf instead of feat

Copy link
Contributor

@DawoudSheraz DawoudSheraz left a comment

Choose a reason for hiding this comment

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

do we have stats on how does this improve the overall filtering?

@Ali-D-Akbar
Copy link
Contributor Author

do we have stats on how does this improve the overall filtering?

>>> # Without select_related
>>> start_time = time.time()
>>> course = Course.objects.filter_drafts(key='edX+DemoX').first()
>>> if course:
...     uuid = str(course.type.uuid)  # This triggers an additional query
... 
>>> end_time = time.time()
>>> print(f"Without select_related: {end_time - start_time:.4f} seconds")
Without select_related: 0.0395 seconds
>>> 
>>> # With select_related
>>> start_time = time.time()
>>> course = Course.objects.filter_drafts(key='edX+DemoX').select_related('type').first()
>>> if course:
...     uuid = str(course.type.uuid)  # No additional query
... 
>>> end_time = time.time()
>>> print(f"With select_related: {end_time - start_time:.4f} seconds")
With select_related: 0.0058 seconds

Copy link
Contributor

@DawoudSheraz DawoudSheraz left a comment

Choose a reason for hiding this comment

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

While it does not achieve much individually but it might prove a bit helpful when running loader against large set of data.

@Ali-D-Akbar Ali-D-Akbar merged commit 771da54 into master Dec 19, 2024
14 checks passed
@Ali-D-Akbar Ali-D-Akbar deleted the aakbar/PROD-4237 branch December 19, 2024 03:02
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.

3 participants