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

Only REFRESH CONCURRENTLY if view populated? #407

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

drnic
Copy link
Contributor

@drnic drnic commented Feb 28, 2024

No description provided.

@derekprior
Copy link
Contributor

Does this fail if the view isn't yet populated?

In any event, I'm not sure silently falling back to a non-concurrent refresh would be best. Thoughts @calebhearth?

@drnic
Copy link
Contributor Author

drnic commented Mar 26, 2024

You can refresh a view that’s not yet populated. Probably the first thing you’d do with the newly created but not yet populated view :)

Copy link
Contributor

@calebhearth calebhearth left a comment

Choose a reason for hiding this comment

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

This looks like a bug fix to me. +1

@@ -214,7 +214,7 @@ def refresh_materialized_view(name, concurrently: false, cascade: false)
refresh_dependencies_for(name, concurrently: concurrently)
end

if concurrently
if concurrently && populated?(name)
Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to update the docs above to mention that even if the value of concurrently is true, we won't refresh concurrently if a view is unpopulated.

Here's a reproduction of the problem being solved here, which would be nice to have a test added for:

caleb=# CREATE MATERIALIZED VIEW testing AS (SELECT unnest('{1, 2}'::int[])) WITH NO DATA;
CREATE MATERIALIZED VIEW
Time: 17.382 ms
caleb=# select * from testing;
ERROR:  materialized view "testing" has not been populated
HINT:  Use the REFRESH MATERIALIZED VIEW command.
Time: 4.597 ms
caleb=# REFRESH MATERIALIZED VIEW CONCURRENTLY testing;
ERROR:  CONCURRENTLY cannot be used when the materialized view is not populated
caleb=# REFRESH MATERIALIZED VIEW testing;
REFRESH MATERIALIZED VIEW
Time: 8.110 ms
caleb=# SELECT * FROM testing;
 unnest 
--------
      1
      2
(2 rows)

Time: 1.345 ms

Copy link
Contributor

Choose a reason for hiding this comment

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

caleb=# REFRESH MATERIALIZED VIEW CONCURRENTLY testing;
ERROR:  cannot refresh materialized view "public.testing" concurrently
HINT:  Create a unique index with no WHERE clause on one or more columns of the materialized view.
Time: 1.029 ms

silently falling back to a non-concurrent refresh

In the case that the view is already populated, we're avoiding raising an error but agreed that it might actually be desirable to raise that error in this case, to avoid long queries blocking a table unexpectedly

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.

None yet

3 participants