-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
sql: change when txn is checked refresh materialized views #87271
Conversation
@@ -80,6 +77,10 @@ func (n *refreshMaterializedViewNode) startExec(params runParams) error { | |||
// results of the view query into the new set of indexes, and then change the | |||
// set of indexes over to the new set of indexes atomically. | |||
|
|||
if !params.p.extendedEvalCtx.TxnIsSingleStmt { | |||
return pgerror.Newf(pgcode.InvalidTransactionState, "cannot refresh view in a multi-statement transaction") | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a test in materialized_view_test.go
right below
} |
you can add
// Verify that refreshing with a prepared statement works.
preparedStmt, err := sqlDB.Prepare(`REFRESH MATERIALIZED VIEW t.v;`)
if err != nil {
t.Fatal(err)
}
if _, err := preparedStmt.Exec(); err != nil {
t.Fatal(err)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)
pkg/sql/refresh_materialized_view.go
line 82 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
can you add a test in materialized_view_test.go
right below
} you can add
// Verify that refreshing with a prepared statement works. preparedStmt, err := sqlDB.Prepare(`REFRESH MATERIALIZED VIEW t.v;`) if err != nil { t.Fatal(err) } if _, err := preparedStmt.Exec(); err != nil { t.Fatal(err) }
Done.
743176a
to
64ec09b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
let's make sure this gets backported |
materialized views Due to the transaction being checked during planning and not during execution, refresh materialized views would fail from the client. Now the transaction is checked during execution . Release justification: Bug fix for refresh materialized views Release note: None
64ec09b
to
fcf22cd
Compare
bors r=rafiss |
Build failed (retrying...): |
Build succeeded: |
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from fcf22cd to blathers/backport-release-22.1-87271: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
Due to the transaction being checked during planning
and not during execution, refresh materialized views
would fail from the client. Now the transaction is checked
during execution .
Release justification: Bug fix for refresh materialized views
Release note: None