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

Introduce alter view propagation #5914

Merged
merged 1 commit into from
May 13, 2022
Merged

Conversation

velioglu
Copy link
Contributor

@velioglu velioglu commented Apr 26, 2022

DESCRIPTION: Adds support for propagating ALTER VIEW commands

Introduce alter view propagation

Adds support for propagating alter view commands (except setting/resetting default values as Citus doesn't support update/delete on views). Note that both ALTER VIEW and ALTER TABLE supports have been added, since PG supports both of those syntax to alter view.

@velioglu velioglu force-pushed the velioglu/alter_view_propagation branch 9 times, most recently from abb431c to 7be079e Compare April 27, 2022 15:06
@velioglu velioglu changed the title [WIP] Introduce alter view propagation Introduce alter view propagation Apr 28, 2022
@velioglu velioglu marked this pull request as ready for review April 28, 2022 00:48
Copy link
Member

@onderkalaci onderkalaci left a comment

Choose a reason for hiding this comment

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

Looks good overall, I need to test this a bit more. For now, few comments:

  1. Can we avoid overriding the object types in the statements, but let the code handle both views and tables where necessary?
  2. Can we send the exact commands that are executed on the coordinator (e.g., do not change ALTER TABLE to ALTER VIEW on the deparser
  3. Lets add the comments that we discussed earlier where we do the process redirection

Copy link
Member

@onderkalaci onderkalaci left a comment

Choose a reason for hiding this comment

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

Mostly minor comments, now testing the changes (and add comments if I find any issues) .

src/backend/distributed/commands/table.c Show resolved Hide resolved
src/backend/distributed/commands/table.c Show resolved Hide resolved
src/backend/distributed/commands/table.c Outdated Show resolved Hide resolved
src/backend/distributed/commands/table.c Outdated Show resolved Hide resolved
src/test/regress/expected/view_propagation.out Outdated Show resolved Hide resolved
src/test/regress/expected/view_propagation.out Outdated Show resolved Hide resolved
src/test/regress/expected/view_propagation.out Outdated Show resolved Hide resolved
@velioglu velioglu force-pushed the velioglu/view_propagation branch 14 times, most recently from 8b95cfd to 2413c09 Compare May 9, 2022 12:21
@velioglu velioglu force-pushed the velioglu/alter_view_propagation branch 21 times, most recently from 9c4638f to eadf2a4 Compare May 11, 2022 23:07
Copy link
Member

@onderkalaci onderkalaci left a comment

Choose a reason for hiding this comment

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

after considering minor style notes, seems ready to go

@velioglu velioglu force-pushed the velioglu/alter_view_propagation branch from eadf2a4 to 0c02a51 Compare May 13, 2022 10:17
Adds support for propagation ALTER VIEW commands to
- Change owner of view
- SET/RESET option
- Rename view and view's column name
- Change schema of the view

Since PG also supports targeting views with ALTER TABLE
commands, related code also added to direct such ALTER TABLE
commands to ALTER VIEW commands while sending them to workers.
@velioglu velioglu force-pushed the velioglu/alter_view_propagation branch from 0c02a51 to 1875516 Compare May 13, 2022 10:24
@codecov
Copy link

codecov bot commented May 13, 2022

Codecov Report

Merging #5914 (1875516) into master (1f17fa8) will increase coverage by 0.01%.
The diff coverage is 96.58%.

@@            Coverage Diff             @@
##           master    #5914      +/-   ##
==========================================
+ Coverage   92.67%   92.69%   +0.01%     
==========================================
  Files         236      236              
  Lines       48203    48422     +219     
==========================================
+ Hits        44672    44884     +212     
- Misses       3531     3538       +7     

@velioglu velioglu merged commit 544e6c7 into master May 13, 2022
@velioglu velioglu deleted the velioglu/alter_view_propagation branch May 13, 2022 10:34
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.

2 participants