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

Rephrase comment for deferred IVM case #576

Merged
merged 3 commits into from
Aug 25, 2024
Merged

Conversation

reshke
Copy link
Contributor

@reshke reshke commented Aug 16, 2024

Have been studying AQUMV feature internals today. Noticed the typo in comment:

...defered-fefresh...

I decided to rephrase it a little bit. This way it is more clear IMO.

Have been studying AQUMV feature internals today. Noticed the typo in comment, and decided to rephrase it a little bit. This way it is more clear IMO.
@reshke
Copy link
Contributor Author

reshke commented Aug 16, 2024

I omitted the fact that deferred IVM intend to only SERVERLESS mode. I think this should be state in doc, no need to duplicate this in comment. Am I right?

@avamingli
Copy link
Contributor

avamingli commented Aug 17, 2024

I omitted the fact that deferred IVM intend to only SERVERLESS mode. I think this should be state in doc, no need to duplicate this in comment. Am I right?

Hi,
Put it into doc is good.
We use same kernel codes in SERVERLESS mode, deferred refresh is implemented first in SERVERLESS mode.
Some codes may be cherry-picked to CBDB later(in fact, we are working on it) and CBDB could also have deferred IVM in the future, not intend to only SERVERLESS mode.
I keep it here to remind us to correct the codes for that.
Thanks for the typos.

@reshke
Copy link
Contributor Author

reshke commented Aug 17, 2024

I keep it here to remind us to correct the codes for that. Thanks for the typos.

Ok, reverted this, lets keep it there

@avamingli avamingli merged commit c7802dc into apache:main Aug 25, 2024
11 checks passed
@reshke reshke deleted the patch-6 branch August 29, 2024 14:33
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