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

Enhance technical README for DDL propagation #7471

Merged
merged 1 commit into from
Jan 31, 2024
Merged

Conversation

onurctirtir
Copy link
Member

No description provided.

Copy link

codecov bot commented Jan 31, 2024

Codecov Report

Merging #7471 (4a63185) into main (5aedec4) will increase coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7471      +/-   ##
==========================================
+ Coverage   89.60%   89.61%   +0.01%     
==========================================
  Files         282      282              
  Lines       60312    60312              
  Branches     7512     7512              
==========================================
+ Hits        54042    54049       +7     
+ Misses       4115     4109       -6     
+ Partials     2155     2154       -1     

Finally, when adding support for propagation of a new DDL command, you also need to make sure that:
* Use `quote_identifier()` or `quote_literal_cstr()` for the fields that might need escaping some characters or bare quotes when deparsing a DDL command.
* The code is tolerant to nullable fields within given `Stmt *` object, i.e., the ones that Postgres allows not specifying at all.
* You register the object into `pg_dist_object` if it's a CREATE command and you delete the object from `pg_dist_object` if it's a DROP command.
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this bullet should mention markDistributed since we remove its previous mention now.

Copy link
Contributor

@JelteF JelteF left a comment

Choose a reason for hiding this comment

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

Looks like a good improvement overall, left one small comment.

@onurctirtir onurctirtir merged commit 6f43d5c into main Jan 31, 2024
126 checks passed
@onurctirtir onurctirtir deleted the update-tech-readme branch January 31, 2024 09:30
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.

None yet

2 participants