Skip to content

Conversation

@homersimpsons
Copy link
Collaborator

Previously there were a bug (reproduced in tests) where retrieving an already not-loaded object would not hydrate it and hence trigger a new query on access.

@homersimpsons
Copy link
Collaborator Author

(Note that I asked copilot for a review to test)

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug where retrieving an already instantiated but not-loaded object from the object storage would not hydrate it, causing subsequent property access to trigger unnecessary database queries. The fix ensures that when an object is retrieved from storage in a STATE_NOT_LOADED state, it gets hydrated immediately with the available data.

Key Changes:

  • Added hydration logic in InnerResultIterator to check if retrieved objects are not loaded and hydrate them
  • Added two test cases to verify the fix works for both direct iteration and reference access scenarios
  • Added helper method findByIds in TestCountryDao to support the new test cases

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/InnerResultIterator.php Added STATE_NOT_LOADED check and hydration call when retrieving objects from storage
tests/TDBMDaoGeneratorTest.php Added two test methods to verify objects are properly hydrated on retrieval
tests/Dao/TestCountryDao.php Added findByIds helper method for testing bulk retrieval by IDs

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@homersimpsons homersimpsons force-pushed the perf/hydrate-not-loaded-objects branch from 64eb424 to 1d5d199 Compare October 24, 2025 15:35
@homersimpsons homersimpsons changed the title ⚡ Hydrate not-loaded objects on retrieval ⚡ Hydrate not-loaded objects on retrieval, throw on inheritance lazyload Oct 24, 2025
@homersimpsons homersimpsons force-pushed the perf/hydrate-not-loaded-objects branch 2 times, most recently from aafa3d4 to 16ef0fc Compare October 26, 2025 08:50
@homersimpsons homersimpsons requested a review from Copilot October 26, 2025 08:53
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Previously there were a bug (reproduced in tests) where retrieving an already not-loaded object would not hydrate it and hence trigger a new query on access.
@homersimpsons homersimpsons force-pushed the perf/hydrate-not-loaded-objects branch from 16ef0fc to 790979a Compare October 26, 2025 08:57
@homersimpsons homersimpsons merged commit 81791c3 into master Oct 26, 2025
10 of 11 checks passed
@homersimpsons homersimpsons deleted the perf/hydrate-not-loaded-objects branch October 26, 2025 09:01
@homersimpsons homersimpsons changed the title ⚡ Hydrate not-loaded objects on retrieval, throw on inheritance lazyload ⚡ Hydrate not-loaded objects on retrieval Oct 26, 2025
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.

2 participants