sql: fix getPlanColumns for hookFnNode #40616
Conversation
|
This change is |
|
I haven't been able to reproduce but this can definitely cause that race (and it's very likely we're dealing with a |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @justinj and @RaduBerinde)
pkg/sql/plan_columns.go, line 121 at r1 (raw file):
return n.getColumns(mut, sqlbase.ExportColumns) case *hookFnNode:
[nit] does the blank line mean this node is in a different category? If so, maybe deserves a comment
There was a problem hiding this comment.
Reviewable status:
`getPlanColumns` allows the caller to directly mutate the columns of a `hookFnNode`; but these columns can be defined globally and should not be modified in place. Fixes cockroachdb#40585. Release note: None
RaduBerinde
force-pushed
the
fix-opaque-race
branch
from
b3d8cf7 to
bdb9b87
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/sql/plan_columns.go, line 121 at r1 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
[nit] does the blank line mean this node is in a different category? If so, maybe deserves a comment
Done.
|
bors r+ |
40616: sql: fix getPlanColumns for hookFnNode r=RaduBerinde a=RaduBerinde `getPlanColumns` allows the caller to directly mutate the columns of a `hookFnNode`; but these columns can be defined globally and should not be modified in place. Fixes #40585. Release note: None Co-authored-by: Radu Berinde <[email protected]>
Build succeeded |
getPlanColumnsallows the caller to directly mutate the columns of ahookFnNode; but these columns can be defined globally and should notbe modified in place.
Fixes #40585.
Release note: None