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 table lookup for drop index #40516

Merged
merged 1 commit into from
Sep 5, 2019

Conversation

pbardea
Copy link
Contributor

@pbardea pbardea commented Sep 5, 2019

Previously, when searching for the table relevant to a particular index
when dropping the index, we would fetch all object names and require that
all those tables exist. However, if a table was deleted in the same
transaction that table name would not be resolvable and we would error.
We already had a check to see if the table being looked up was nil, but
this check would not be used because the required flag was set to true.

This PR just sets the flag to false, and looks at moves on to the next
table if one of them no longer is resolvable.

Addresses #38768.

Release note (bug fix): Fix faulty error when trying to delete a table
and an unrelated index in the same transaction.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Previously, when searching for the table relevant to a particular index
when dropping the index, we would fetch all object names and require that
all those tables exist. However, if a table was deleted in the same
transaction that table name would not be resolvable and we would error.
We already had a check to see if the table being looked up was nil, but
this check would not be used because the `required` flag was set to true.

This PR just sets the flag to false, and looks at moves on to the next
table if one of them no longer is resolvable.

Release note (bug fix): Fix faulty error when trying to delete a table
and an unrelated index in the same transaction.
@pbardea pbardea changed the title [wip] sql: don't require a table to exist when searching sql: fix table lookup for drop index Sep 5, 2019
@pbardea pbardea requested a review from dt September 5, 2019 16:47
@pbardea
Copy link
Contributor Author

pbardea commented Sep 5, 2019

bors r+

craig bot pushed a commit that referenced this pull request Sep 5, 2019
40485: sql: Fix a bug with ordinal_position in information_schema.columns r=arulajmani a=arulajmani

When a column other than the last is dropped, ordinal_position in
information_schema.columns virtual table no longer matches attnum from
the pg_attribute table. This PR fixes this issue.

Fixes #39787

Release note (bug fix): ordinal_position in information_schema.columns
matches pg_attribute.attnum after a column is dropped.

40511: exec: fix explain(vec) for queries with subqueries r=jordanlewis a=jordanlewis

Also add logic tests that show the explain(vec) plans for all of the
tpch queries.

Closes #40484.

Release note: None

40516: sql: fix table lookup for drop index  r=pbardea a=pbardea

Previously, when searching for the table relevant to a particular index
when dropping the index, we would fetch all object names and require that
all those tables exist. However, if a table was deleted in the same
transaction that table name would not be resolvable and we would error.
We already had a check to see if the table being looked up was nil, but
this check would not be used because the `required` flag was set to true.

This PR just sets the flag to false, and looks at moves on to the next
table if one of them no longer is resolvable.

Addresses #38768.

Release note (bug fix): Fix faulty error when trying to delete a table
and an unrelated index in the same transaction.

Co-authored-by: Arul Ajmani <arula@cockroachlabs.com>
Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
Co-authored-by: Paul Bardea <pbardea@gmail.com>
@craig
Copy link
Contributor

craig bot commented Sep 5, 2019

Build succeeded

@craig craig bot merged commit c224caf into cockroachdb:master Sep 5, 2019
@pbardea pbardea deleted the index-search-required branch November 22, 2019 21:03
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.

3 participants