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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: query perf degradation from deep_copy util #1673

Merged
merged 3 commits into from
Jun 10, 2024

Conversation

tranhl
Copy link
Contributor

@tranhl tranhl commented Apr 14, 2024

Summary:

I believe this fixes #1260.

See this comment for context & details

GitHub linked issue:

#1260

Type (select 1):

  • Bug fix
  • Feature implementation
  • Documentation improvement
  • Testing improvement
  • Test added to report bug (GitHub issue #---- @---)
  • Something not listed here

Is this a breaking change? (select 1):

  • 馃毃 YES 馃毃
  • No
  • I'm not sure

Is this ready to be merged into Dynamoose? (select 1):

  • Yes
  • No

Are all the tests currently passing on this PR? (select 1):

  • Yes
  • No

Other:

  • I have read through and followed the Contributing Guidelines
  • I have searched through the GitHub pull requests to ensure this PR has not already been submitted
  • I have updated the Dynamoose documentation (if required) given the changes I made
  • I have added/updated the Dynamoose test cases (if required) given the changes I made
  • I agree that all changes made in this pull request may be distributed and are made available in accordance with the Dynamoose license
  • All of my commits and commit messages are detailed, explain what changes were made, and are easy to follow and understand
  • I have filled out all fields above

Copy link
Contributor

This pull request is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 3 days.

@tranhl
Copy link
Contributor Author

tranhl commented Apr 25, 2024

@fishcharlie I've added a test to confirm that this change works. It checks:

  • When a nested obj property references the top-level object
  • When a nested obj property references another nested property
  • When a nested obj property references itself

I believe these are all the cases we need to be worried about. Let me know if I've missed any cases and I'll add them in. Keen to have this shipped! It cut down one of my team's query from 20s to <1s.

@fishcharlie fishcharlie merged commit 1f7e433 into dynamoose:main Jun 10, 2024
14 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Slow scan operations
2 participants