Skip to content

sql: Allow zone configs on offline tables #40285

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

Merged
merged 1 commit into from
Aug 29, 2019

Conversation

pbardea
Copy link
Contributor

@pbardea pbardea commented Aug 28, 2019

Zone configs should be allowed to be set for offline tables. The specific use
case that this is targetting is to create a work-around for #39602. This is to
allow for tables to start being restored, having the restored job paused and
then making zone config changes on these offline tables.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@pbardea pbardea force-pushed the offline-zone-config branch 12 times, most recently from 831a11d to 18de819 Compare August 28, 2019 21:46
@pbardea pbardea requested a review from a team as a code owner August 28, 2019 21:46
@pbardea pbardea force-pushed the offline-zone-config branch 4 times, most recently from 37e3634 to 5c3d432 Compare August 28, 2019 22:01
@pbardea
Copy link
Contributor Author

pbardea commented Aug 28, 2019

Additional context: ObjectLookupFlags was moved to the sql/sem/tree package so LookupObject could use it as an argument while avoiding introducing a cyclic dependency between sql/sem/tree and sql

@pbardea pbardea force-pushed the offline-zone-config branch from 5c3d432 to e2047af Compare August 28, 2019 22:04
@pbardea pbardea requested a review from dt August 28, 2019 22:05
Copy link
Member

@dt dt left a comment

Choose a reason for hiding this comment

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

you were not kidding about it being "a bit more plumbing".

@pbardea pbardea changed the title [wip] sql: Allow zone configs on offline tables sql: Allow zone configs on offline tables Aug 29, 2019
@pbardea pbardea force-pushed the offline-zone-config branch 2 times, most recently from 17fd2fe to 8095dc5 Compare August 29, 2019 15:57
Zone configs should be allowed to be set for offline tables. The specific use
case that this is targetting is to create a work-around for cockroachdb#39602. This is to
allow for tables to start being restored, having the restored job paused and
then making zone config changes on these offline tables.

This change also refactors some methods to pass a ObjectLookupFlags struct
rather than a lot of individual flags down. This is so if in the future
another flag is added, the signature of the intermediary functions don't
explode in size. It would also ensure that future changes won't need to go
through this plumbing again.

Release note: None
@pbardea pbardea force-pushed the offline-zone-config branch from 8095dc5 to 0d61b25 Compare August 29, 2019 17:53
@pbardea
Copy link
Contributor Author

pbardea commented Aug 29, 2019

bors r=dt

craig bot pushed a commit that referenced this pull request Aug 29, 2019
40285: sql: Allow zone configs on offline tables r=dt a=pbardea

Zone configs should be allowed to be set for offline tables. The specific use
case that this is targetting is to create a work-around for #39602. This is to
allow for tables to start being restored, having the restored job paused and
then making zone config changes on these offline tables.

Release note: None

40344: exec: handle division by zero for decimal types r=rafiss a=rafiss

Previously, when a decimal operation had a division by zero, we would
return `errors.fundamental(division by zero)`. That results in the
correct error message being shown to the user, but it does not have the
appropriate PG error code.

Release note: None

Co-authored-by: Paul Bardea <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
@craig
Copy link
Contributor

craig bot commented Aug 29, 2019

Build succeeded

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