Skip to content

Rephrase comment for deferred IVM case#576

Merged
avamingli merged 3 commits into
apache:mainfrom
reshke:patch-6
Aug 25, 2024
Merged

Rephrase comment for deferred IVM case#576
avamingli merged 3 commits into
apache:mainfrom
reshke:patch-6

Conversation

@reshke

@reshke reshke commented Aug 16, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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.

@reshke
Rephrase comment for deferred IVM case
28d4de1 
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 reshke mentioned this pull request Aug 16, 2024
Closed
@reshke

reshke commented Aug 16, 2024

Copy link
Copy Markdown
Contributor Author

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

avamingli commented Aug 17, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

reshke commented Aug 17, 2024

Copy link
Copy Markdown
Contributor Author

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
@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

Reviewers

@avamingli avamingli avamingli approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@reshke @avamingli