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

Add a method to detach the context used by TableReaderExecutor #53336

Closed
YangKeao opened this issue May 16, 2024 · 0 comments · Fixed by #54456
Closed

Add a method to detach the context used by TableReaderExecutor #53336

YangKeao opened this issue May 16, 2024 · 0 comments · Fixed by #54456
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@YangKeao
Copy link
Member

Enhancement

We have prepared enough utilities to detach the TableReacerExecutor's context from the session context now! Let's finish the last step before starting to develop a new cursor fetch!

It's not that convenient to copy these contexts again (especially for WarnHandler, which is shared in almost every context). To solve this complicated construction issue, a DI framework is usually used. However, I think it over complicates the logic. A better solution (IMO) is to just leave these fields. Re-think that these fields cannot be used across multiple statements because two reasons:

  1. They are shared and can be modified by the next statement, so it's not safe to keep using it.
  2. The memory is cleared before executing the next statement, so keeping a reference to it is not safe.

For the first situation, we have to copy the internal value. For the second situation, we can just leave it! And allocate/create new one for each statement, so the previous one doesn't need to be cleared (and the GC will help us to free these memory later). Using this strategy, it'll be much simpler to refactor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
1 participant