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.

Sign up for GitHub

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

Jump to bottom

Only REFRESH CONCURRENTLY if view populated? #407

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

Only REFRESH CONCURRENTLY if view populated? #407

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/scenic/adapters/postgres.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

raise_unless_concurrent_refresh_supported
execute "REFRESH MATERIALIZED VIEW CONCURRENTLY #{quote_table_name(name)};"
else
Expand Down