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

sql: fix data race in ResolvableFunctionReference #45919

Closed

Conversation

jordanlewis
Copy link
Member

Previously, ResolvableFunctionReference was the site of a data race in
which statements like SHOW QUERIES, which need to format an AST
concurrently with query planning and running, raced with the query
planner which as a side effect of planning mutates
ResolvableFunctionReference. Ideally, ResolvableFunctionReference would
be immutable like a good little AST node.

This commit separates ResolvableFunctionReference into two fields, one
immutable (the UnresolvedName that it starts with from the parser) and
one mutable (the resolved function itself). The formatter only reads the
immutable part, so the planner is free to mutate the resolved part as it
wishes.

Fixes #28033

@RaduBerinde I wonder if we could change things up a bit during planning to get rid of this problem entirely, but I think this is a good fix for now.

Release note: None

@jordanlewis jordanlewis requested review from otan and RaduBerinde March 10, 2020 00:24
@jordanlewis jordanlewis requested a review from a team as a code owner March 10, 2020 00:24
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@otan otan left a comment

Choose a reason for hiding this comment

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

o_o interest set of test failures. Wonder if help relies on resolved fns

Previously, ResolvableFunctionReference was the site of a data race in
which statements like SHOW QUERIES, which need to format an AST
concurrently with query planning and running, raced with the query
planner which as a side effect of planning mutates
ResolvableFunctionReference. Ideally, ResolvableFunctionReference would
be immutable like a good little AST node.

This commit separates ResolvableFunctionReference into two fields, one
immutable (the UnresolvedName that it starts with from the parser) and
one mutable (the resolved function itself). The formatter only reads the
immutable part, so the planner is free to mutate the resolved part as it
wishes.

Release note: None
@jordanlewis jordanlewis force-pushed the show-query-data-race branch from b120cd6 to 7316e7a Compare March 10, 2020 05:04
@bobvawter
Copy link
Contributor

bobvawter commented Mar 10, 2020

CLA assistant check
All committers have signed the CLA.

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.

sql: race in CANCEL QUERIES
4 participants