backupccl: stop swallowing error in restoreResumer.OnFailOrCancel()#43218
Conversation
|
@arulajmani feel free to indicate an alternative victim to review this one-liner. While |
arulajmani
left a comment
There was a problem hiding this comment.
LGTM, minor comment about error wrapping.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @arulajmani)
pkg/ccl/backupccl/restore.go, line 1793 at r1 (raw file):
err := sqlbase.RemovePublicTableNamespaceEntry(ctx, txn, tbl.ParentID, tbl.Name) if err != nil { return err
nit: Maybe return errors.Wrap(err, "dropping tables")? That seemed to be the error wrapping before I made the change that broke this.
In `restoreResumer.OnFailOrCancel()` we'd return nil when encountering a non-nil error in a call to `sqlbase.RemovePublicTableNamespaceEntry()`. Error handling in `jobs.Resumer.OnFailOrCancel()` is a tricky proposition but this behavior is certainly wrong. Release note: None
d6d13e5 to
db30a84
Compare
ajwerner
left a comment
There was a problem hiding this comment.
TFTR!
bors r+
Reviewable status:
complete! 0 of 0 LGTMs obtained
pkg/ccl/backupccl/restore.go, line 1793 at r1 (raw file):
Previously, arulajmani (Arul Ajmani) wrote…
nit: Maybe
return errors.Wrap(err, "dropping tables")? That seemed to be the error wrapping before I made the change that broke this.
Done.
43194: opt: use transitive equalities to infer filters r=rytaft a=rytaft Previously, the optimizer was not inferring all possible filters based on equality conditions. For example, consider a query such as: ``` SELECT * FROM t JOIN u ON t.a = u.a JOIN v ON u.a = v.a JOIN w ON v.a = w.a WHERE t.a = 5; ``` Ideally, the optimizer should infer additional filter conditions `u.a = 5 AND v.a = 5 AND w.a = 5` based on the equality conditions. However, this was not occurring because the optimizer only considered equality conditions that were at the same level in the query tree as the filter to be mapped. Now the optimizer also considers equality conditions that are further down in the tree, so that additional filters can be inferred based on transitive equalities. Informs #43039 Release note (performance improvement): The optimizer can now infer additional filter conditions in some cases based on transitive equalities between columns. 43218: backupccl: stop swallowing error in restoreResumer.OnFailOrCancel() r=ajwerner a=ajwerner In `restoreResumer.OnFailOrCancel()` we'd return nil when encountering a non-nil error in a call to `sqlbase.RemovePublicTableNamespaceEntry()`. Error handling in `jobs.Resumer.OnFailOrCancel()` is a tricky proposition but this behavior is certainly wrong. Release note: None Co-authored-by: Rebecca Taft <becca@cockroachlabs.com> Co-authored-by: Andrew Werner <ajwerner@cockroachlabs.com>
|
Flaked on #43141 |
Build succeeded |
In
restoreResumer.OnFailOrCancel()we'd return nil when encountering anon-nil error in a call to
sqlbase.RemovePublicTableNamespaceEntry().Error handling in
jobs.Resumer.OnFailOrCancel()is a tricky proposition butthis behavior is certainly wrong.
Release note: None